fix: keep ship review retries inside one invocation

setup

@@ -18,8 +18,8 @@ INSTALL_GSTACK_DIR="$(cd "$(dirname "$0")" && pwd)"
 SOURCE_GSTACK_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 INSTALL_SKILLS_DIR="$(dirname "$INSTALL_GSTACK_DIR")"
 BROWSE_BIN="$SOURCE_GSTACK_DIR/browse/dist/browse"
-CODEX_SKILLS="$HOME/.codex/skills"
-CODEX_GSTACK="$CODEX_SKILLS/gstack"
+CODEX_SKILLS="$HOME/.agents/skills"
+CODEX_GSTACK="$HOME/.codex/skills/gstack"
 FACTORY_SKILLS="$HOME/.factory/skills"
 FACTORY_GSTACK="$FACTORY_SKILLS/gstack"
 OPENCODE_SKILLS="$HOME/.config/opencode/skills"
@@ -225,9 +225,14 @@ fi
 migrate_direct_codex_install() {
   local gstack_dir="$1"
   local codex_gstack="$2"
+  local agents_gstack="$3"
   local migrated_dir="$HOME/.gstack/repos/gstack"
 
-  [ "$gstack_dir" = "$codex_gstack" ] || return 0
+  if [ "$gstack_dir" = "$codex_gstack" ] || [ "$gstack_dir" = "$agents_gstack" ]; then
+    : # Direct install: migrate below so Codex does not recursively discover source skills.
+  else
+    return 0
+  fi
   [ -L "$gstack_dir" ] && return 0
 
   mkdir -p "$(dirname "$migrated_dir")"
@@ -246,7 +251,7 @@ migrate_direct_codex_install() {
 }
 
 if [ "$INSTALL_CODEX" -eq 1 ]; then
-  migrate_direct_codex_install "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK"
+  migrate_direct_codex_install "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK" "$CODEX_SKILLS/gstack"
 fi
 
 ensure_playwright_browser() {
@@ -446,12 +451,12 @@ fi
 AGENTS_DIR="$SOURCE_GSTACK_DIR/.agents/skills"
 NEEDS_AGENTS_GEN=1
 
-if [ "$NEEDS_AGENTS_GEN" -eq 1 ] && [ "$NEEDS_BUILD" -eq 0 ]; then
+if [ "$NEEDS_AGENTS_GEN" -eq 1 ]; then
   log "Generating .agents/ skill docs..."
   (
     cd "$SOURCE_GSTACK_DIR"
     bun_cmd install --frozen-lockfile 2>/dev/null || bun_cmd install
-    bun_cmd run gen:skill-docs --host codex
+    bun_cmd run gen:skill-docs --host codex --model gpt
   )
 fi
 
@@ -711,11 +716,11 @@ link_codex_skill_dirs() {
 
   if [ ! -d "$agents_dir" ]; then
     echo "  Generating .agents/ skill docs..."
-    ( cd "$gstack_dir" && bun run gen:skill-docs --host codex )
+    ( cd "$gstack_dir" && bun run gen:skill-docs --host codex --model gpt )
   fi
 
   if [ ! -d "$agents_dir" ]; then
-    echo "  warning: .agents/skills/ generation failed — run 'bun run gen:skill-docs --host codex' manually" >&2
+    echo "  warning: .agents/skills/ generation failed — run 'bun run gen:skill-docs --host codex --model gpt' manually" >&2
     return 1
   fi
 
@@ -976,8 +981,13 @@ link_opencode_skill_dirs() {
 SKILLS_BASENAME="$(basename "$INSTALL_SKILLS_DIR")"
 SKILLS_PARENT_BASENAME="$(basename "$(dirname "$INSTALL_SKILLS_DIR")")"
 CODEX_REPO_LOCAL=0
+CODEX_GLOBAL_AGENTS=0
 if [ "$SKILLS_BASENAME" = "skills" ] && [ "$SKILLS_PARENT_BASENAME" = ".agents" ]; then
-  CODEX_REPO_LOCAL=1
+  if [ "$INSTALL_SKILLS_DIR" = "$HOME/.agents/skills" ]; then
+    CODEX_GLOBAL_AGENTS=1
+  else
+    CODEX_REPO_LOCAL=1
+  fi
 fi
 
 if [ "$INSTALL_CLAUDE" -eq 1 ]; then
@@ -1087,6 +1097,8 @@ if [ "$INSTALL_CODEX" -eq 1 ]; then
   if [ "$CODEX_REPO_LOCAL" -eq 1 ]; then
     CODEX_SKILLS="$INSTALL_SKILLS_DIR"
     CODEX_GSTACK="$INSTALL_GSTACK_DIR"
+  elif [ "$CODEX_GLOBAL_AGENTS" -eq 1 ]; then
+    CODEX_SKILLS="$INSTALL_SKILLS_DIR"
   fi
   mkdir -p "$CODEX_SKILLS"
 

ship/sections/review-army.md

@@ -383,8 +383,8 @@ Output a summary header: `Pre-Landing Review: N issues (X critical, Y informatio
    - If 3 or fewer ASK items, you may use individual AskUserQuestion calls instead
 
 7. **After all fixes (auto + user-approved):**
-   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then **STOP** and tell the user to run `/ship` again to re-test.
-   - If no fixes applied (all ASK items skipped, or no issues found): continue to Step 12.
+   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then rerun the required tests and pre-landing review against the new immutable HEAD inside the same `/ship` invocation. Repeat until clean or a genuine ASK/hard failure requires human input. Do not return control to the user merely because fixes changed HEAD.
+   - When no fixes remain, continue the ship workflow.
 
 8. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped)`
 

ship/sections/review-army.md.tmpl

@@ -33,8 +33,8 @@ Review the diff for structural issues that tests don't catch.
    - If 3 or fewer ASK items, you may use individual AskUserQuestion calls instead
 
 7. **After all fixes (auto + user-approved):**
-   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then **STOP** and tell the user to run `/ship` again to re-test.
-   - If no fixes applied (all ASK items skipped, or no issues found): continue to Step 12.
+   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then rerun the required tests and pre-landing review against the new immutable HEAD inside the same `/ship` invocation. Repeat until clean or a genuine ASK/hard failure requires human input. Do not return control to the user merely because fixes changed HEAD.
+   - When no fixes remain, continue the ship workflow.
 
 8. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped)`
 

test/fixtures/golden/codex-ship-SKILL.md

@@ -2061,8 +2061,8 @@ Output a summary header: `Pre-Landing Review: N issues (X critical, Y informatio
    - If 3 or fewer ASK items, you may use individual AskUserQuestion calls instead
 
 7. **After all fixes (auto + user-approved):**
-   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then **STOP** and tell the user to run `/ship` again to re-test.
-   - If no fixes applied (all ASK items skipped, or no issues found): continue to Step 12.
+   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then rerun the required tests and pre-landing review against the new immutable HEAD inside the same `/ship` invocation. Repeat until clean or a genuine ASK/hard failure requires human input. Do not return control to the user merely because fixes changed HEAD.
+   - When no fixes remain, continue the ship workflow.
 
 8. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped)`
 

test/fixtures/golden/factory-ship-SKILL.md

@@ -2315,8 +2315,8 @@ Output a summary header: `Pre-Landing Review: N issues (X critical, Y informatio
    - If 3 or fewer ASK items, you may use individual AskUserQuestion calls instead
 
 7. **After all fixes (auto + user-approved):**
-   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then **STOP** and tell the user to run `/ship` again to re-test.
-   - If no fixes applied (all ASK items skipped, or no issues found): continue to Step 12.
+   - If ANY fixes were applied: commit fixed files by name (`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`), then rerun the required tests and pre-landing review against the new immutable HEAD inside the same `/ship` invocation. Repeat until clean or a genuine ASK/hard failure requires human input. Do not return control to the user merely because fixes changed HEAD.
+   - When no fixes remain, continue the ship workflow.
 
 8. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped)`
 

test/gen-skill-docs.test.ts

@@ -1797,6 +1797,30 @@ describe('Codex generation (--host codex)', () => {
     expect(reviewContent).not.toContain('CODEX_REVIEWS');
   });
 
+  test('ship keeps mechanical review fixes inside the same invocation through PR creation', () => {
+    const shipContent = fs.readFileSync(path.join(AGENTS_DIR, 'gstack-ship', 'SKILL.md'), 'utf-8');
+    const postFixStart = shipContent.indexOf('7. **After all fixes (auto + user-approved):**');
+    const postFixEnd = shipContent.indexOf('8. Output summary:', postFixStart);
+    const postFixBlock = shipContent.slice(postFixStart, postFixEnd);
+
+    expect(postFixStart).toBeGreaterThan(-1);
+    expect(postFixEnd).toBeGreaterThan(postFixStart);
+    expect(postFixBlock).toContain('same `/ship` invocation');
+    expect(postFixBlock).toContain('Repeat until clean');
+    expect(postFixBlock).not.toContain('**STOP**');
+    expect(postFixBlock).not.toContain('tell the user to run');
+    expect(shipContent.indexOf('## Step 19: Create PR/MR')).toBeGreaterThan(postFixEnd);
+  });
+
+  test('ship automation contract does not contradict its post-fix instructions', () => {
+    const shipContent = fs.readFileSync(path.join(AGENTS_DIR, 'gstack-ship', 'SKILL.md'), 'utf-8');
+
+    expect(shipContent).toContain('Auto-fixable review findings');
+    expect(shipContent).not.toMatch(
+      /If ANY fixes were applied:[\s\S]{0,500}\*\*STOP\*\*[\s\S]{0,200}run `\/ship` again/,
+    );
+  });
+
   test('--host codex --dry-run freshness', () => {
     const result = Bun.spawnSync(['bun', 'run', 'scripts/gen-skill-docs.ts', '--host', 'codex', '--dry-run'], {
       cwd: ROOT,
@@ -2296,6 +2320,20 @@ describe('setup script validation', () => {
     expect(setupContent).toContain('CODEX_SKILLS="$INSTALL_SKILLS_DIR"');
   });
 
+  test('global .agents installs keep generated skills there and create the Codex runtime root', () => {
+    expect(setupContent).toContain('CODEX_GLOBAL_AGENTS=0');
+    expect(setupContent).toContain('[ "$INSTALL_SKILLS_DIR" = "$HOME/.agents/skills" ]');
+    expect(setupContent).toContain('CODEX_GLOBAL_AGENTS=1');
+
+    const codexSection = setupContent.slice(
+      setupContent.indexOf('# 5. Install for Codex'),
+      setupContent.indexOf('# 6. Create'),
+    );
+    expect(codexSection).toContain('elif [ "$CODEX_GLOBAL_AGENTS" -eq 1 ]; then');
+    expect(codexSection).toContain('CODEX_SKILLS="$INSTALL_SKILLS_DIR"');
+    expect(codexSection).toContain('create_codex_runtime_root "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK"');
+  });
+
   test('setup separates install path from source path for symlinked repo-local installs', () => {
     expect(setupContent).toContain('INSTALL_GSTACK_DIR=');
     expect(setupContent).toContain('SOURCE_GSTACK_DIR=');
@@ -2384,6 +2422,17 @@ describe('setup script validation', () => {
     expect(setupContent).toContain('command -v opencode');
   });
 
+  test('setup generates Codex skills with the GPT model overlay', () => {
+    const codexGenerationCommands = setupContent.match(
+      /bun(?:_cmd)? run gen:skill-docs --host codex[^\n]*/g,
+    ) ?? [];
+
+    expect(codexGenerationCommands.length).toBeGreaterThan(0);
+    for (const command of codexGenerationCommands) {
+      expect(command).toContain('--model gpt');
+    }
+  });
+
   // T1: Sidecar skip guard — prevents .agents/skills/gstack from being linked as a skill
   test('link_codex_skill_dirs skips the gstack sidecar directory', () => {
     const fnStart = setupContent.indexOf('link_codex_skill_dirs()');
@@ -2456,6 +2505,17 @@ describe('setup script validation', () => {
     expect(setupContent).toContain('avoid duplicate skill discovery');
   });
 
+  test('global Codex discovery uses ~/.agents and migrates the source repo out of it', () => {
+    expect(setupContent).toContain('CODEX_SKILLS="$HOME/.agents/skills"');
+
+    const migrationStart = setupContent.indexOf('migrate_direct_codex_install()');
+    const migrationEnd = setupContent.indexOf('ensure_playwright_browser()', migrationStart);
+    const migrationBody = setupContent.slice(migrationStart, migrationEnd);
+    expect(migrationBody).toContain('local agents_gstack="$3"');
+    expect(migrationBody).toContain('[ "$gstack_dir" = "$agents_gstack" ]');
+    expect(migrationBody).toContain('migrate_direct_codex_install "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK" "$CODEX_SKILLS/gstack"');
+  });
+
   // --- Symlink prefix tests (PR #503) ---
 
   test('link_claude_skill_dirs applies gstack- prefix by default', () => {