Skip to content

chore(ship): default to chatgpt planner, fix #630 tooling friction - #654

Merged
BorisTyshkevich merged 1 commit into
mainfrom
chore/ship-skill-improvements
Aug 9, 2026
Merged

chore(ship): default to chatgpt planner, fix #630 tooling friction#654
BorisTyshkevich merged 1 commit into
mainfrom
chore/ship-skill-improvements

Conversation

@BorisTyshkevich

Copy link
Copy Markdown
Collaborator

Summary

Reviewed the /ship skill against the full 8-phase history of issue #630 (the @altinity/clickhouse-http extraction) and made the fixes that history pointed at, plus flipped the default planner per explicit request.

  • Default planner flipped to chatgpt (--planner fable opts back into the original Fable-authors/ChatGPT-reviews mode). Updated every default/opt-in reference across SKILL.md, per-issue-cycle.md, and review-loops.md.
  • Session pass-budget visibility (code-review-pass.workflow.mjs): the review-runner's own internal retry logic (up to 4 attempts per /ship-labelled pass) makes real chatgpt-review pr CLI calls that count against the tool's hard 3-total-passes-per-session cap — invisibly to the coordinator's own pass count. Observed live on PR feat(#630): make @altinity/clickhouse-http standalone-buildable, retire client-web vendor spike (phase 8) #653: "pass 1" silently spent 2 of 3 slots on an internal retry, so "pass 3" was rejected outright as CLI call 4. Added attemptsUsed + a dedicated sessionExhausted/session-cap-exhausted status so this is detected immediately rather than discovered as a bare rejection.
  • Written recovery procedure for a recurring ChatGPT-side stall (review-loops.md): a live tool call hanging mid-turn for 5-10+ minutes hit every plan-authoring session from Phase 3 onward — distinct from the completion-detection bug already fixed in fix(chatgpt-review): plan-author early-exit once a complete protocol appears #650. Previously handled ad hoc each time; now an explicit procedure (stop + nudge in the same conversation) to try before treating needs_human as SKILL.md's FULL STOP.
  • Local main drift check+repair (SKILL.md 2.7): a /ship run never advances local main (every unit branches off origin/main directly), so gh pr merge --delete-branch's local fast-forward step can fail even when the remote merge succeeds — happened on PR feat(#630): make @altinity/clickhouse-http standalone-buildable, retire client-web vendor spike (phase 8) #653, the run's very last merge. Added an explicit check+repair step.
  • Reviewer checklist + footgun entries: the escape-unaware-textual-prefilter and node_modules-workspace-symlink-bypass bug classes each recurred multiple times against build/check-boundaries.mjs across Epic: extract the Fetch-native ClickHouse client into a reusable package #630 (twice on the same guard within Phase 8 alone) — added explicit checklist coverage so a reviewer checks every sibling instance, not just the one flagged.
  • Diagnostic capture (chatgpt-plan-author-loop.workflow.mjs): Phase 8 hit one needs_human that never got root-caused (a complete plan seemingly not recognized). Now captures the last response text on an incomplete result so a recurrence is easier to diagnose.

Known pre-existing issue (not touched here)

skills/ship/tests/workflow-contract.test.mjs has one pre-existing failing assertion (a stale drift-bind regex against decompose-and-implement-loop.workflow.mjs that no longer matches its content since PR #651's wave-based rewrite) — confirmed via git stash that it fails identically on origin/main, unrelated to this change. Left alone as a separate concern.

Test plan

  • node --test skills/ship/tests/*.test.mjs — 7/8 pass; the 1 failure is the pre-existing, unrelated issue above
  • All three edited .workflow.mjs files syntax-checked (wrapped to match the Workflow runtime's actual execution shape, since they use top-level return/agent/log)
  • invocation.test.mjs updated and passing for the new default + explicit opt-out
  • Confirmed no cross-references from the main repo's tests//build/ trees into .claude/skills or skills/ship — this change cannot affect the main gate

🤖 Generated with Claude Code

https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz

Learned across #630's 8 phases:

- --planner now defaults to chatgpt (ChatGPT authors/revises, Fable/high approves);
  --planner fable opts back into the original mode. Flipped every default/opt-in
  reference in SKILL.md, per-issue-cycle.md, and review-loops.md accordingly.
- code-review-pass.workflow.mjs's own up-to-4-attempt retry logic makes real
  chatgpt-review CLI calls that count against the tool's hard 3-total-passes-per-
  session cap, invisibly to the coordinator's own pass count -- observed on PR #653
  where pass 1 silently spent 2 of 3 slots. Added attemptsUsed + a dedicated
  sessionExhausted/session-cap-exhausted status so this is detected immediately
  instead of discovered as a bare CLI rejection.
- Documented a recurring, previously undocumented ChatGPT-side failure (a live tool
  call hanging mid-turn, distinct from the completion-detection bug fixed in #650)
  and its recovery (stop + nudge in the same conversation) as an explicit
  review-loops.md procedure to try before treating needs_human as SKILL.md's FULL
  STOP.
- SKILL.md step 2.7: gh pr merge --delete-branch's local main fast-forward can fail
  even when the remote merge succeeds, since a /ship run never advances local main
  itself (every unit branches off origin/main directly) -- added an explicit
  check+repair step.
- per-issue-cycle.md reviewer prompt + repo-footguns.md: the escape-unaware-prefilter
  and node_modules-symlink-bypass bug classes recurred repeatedly against
  build/check-boundaries.mjs across #630 -- added explicit checklist coverage.
- chatgpt-plan-author-loop.workflow.mjs: capture lastResponsePreview on an incomplete
  authoring result, for diagnosing a future recurrence of #630 phase 8's one
  never-root-caused needs_human.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz
@BorisTyshkevich
BorisTyshkevich merged commit 2584deb into main Aug 9, 2026
8 checks passed
@BorisTyshkevich
BorisTyshkevich deleted the chore/ship-skill-improvements branch August 9, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant