Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 25 additions & 12 deletions skills/ship/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ creation, every `chatgpt-review` invocation, and the merge.
code-review loop's fix-accepted-findings agent) uses `sonnet`. Planning/plan-authoring
work uses `fable` at `effort: "high"` in the default planner mode. With
`--planner chatgpt`, ChatGPT owns every plan draft and revision while Fable/high owns
the read-only approval decision; Sonnet still verifies every substantive finding.
the read-only approval decision. ChatGPT verifies Fable's own findings itself while
revising — it is the plan's sole author and reviser, and it already performs this kind
of live check unprompted (e.g. confirming an exact npm package version) — so this loop
has no separate Sonnet fact-check pass on Fable's findings the way the default plan
loop and the code review loop do on theirs.
This split is wired into `references/plan-review-loop.workflow.mjs`,
`references/chatgpt-plan-author-loop.workflow.mjs`, and
`references/code-review-pass.workflow.mjs`; keep it there when editing those scripts.
Expand All @@ -89,10 +93,16 @@ This split is wired into `references/plan-review-loop.workflow.mjs`,
authoring protocol plus schema-constrained Fable verdicts. Every loop's pass still
counts against its cap.
- **Every substantive finding is verified against the real repository before it is
trusted** — the loop workflows fan out one read-only verifier per finding; neither
ChatGPT nor Fable is a source of truth. Every finding ends in exactly one state:
accepted-and-fixed, rejected-with-reason, or unresolved. Never silently drop one —
the workflows return `accepted` and `rejected` lists; record them.
trusted, by whichever party owns the plan/PR being revised** — never the critic who
raised it. In the default plan loop and the code review loop that's a separate
read-only Sonnet verifier per finding, since Fable there is the reviewer of ChatGPT's
own PR/plan review; in the ChatGPT-author loop, ChatGPT itself (the plan's sole
author/reviser) verifies each Fable finding while revising, since Fable is read-only
and cannot confirm exact repository state. Either way, every finding ends in exactly
one state — accepted-and-fixed, rejected-with-reason, or unresolved — never silently
dropped: the default/code loops return `accepted` and `rejected` lists to record; the
ChatGPT-author loop's rejections land as `## Review responses` entries in the plan
itself, and a `needs_human` outcome there carries the round's raw `findings`.

### Output capture

Expand Down Expand Up @@ -250,18 +260,21 @@ mid-loop (footguns).
```

ChatGPT privately authors a complete standalone plan through `plan-author`; Fable
at high effort reviews it read-only against the actual repository. Sonnet read-only
agents verify every substantive Fable finding. Accepted findings and evidence-backed
rebuttals are passed back to ChatGPT, which atomically replaces the canonical plan
with a complete revision in the same conversation. The workflow performs at most
five Fable review passes. ChatGPT alone owns drafts and revisions; Fable/high alone
owns approval.
at high effort reviews it read-only against the actual repository. Fable's raw
findings — labelled unverified, since Fable cannot confirm exact repository state —
are passed straight back to ChatGPT, which verifies each one itself (it already does
this kind of live check unprompted) and atomically replaces the canonical plan with
a complete revision in the same conversation, recording anything it rejects under
`## Review responses`. The workflow performs at most five Fable review passes.
ChatGPT alone owns drafts, revisions, and verifying findings against them; Fable/high
alone owns approval.
3. `status: "approved"` → record the pass count and conversation URL for the ship log;
proceed to 2.3.
4. `status: "blocked"` → skip the unit and report the concrete missing decision; do
not guess and do not treat this as a review-loop exhaustion.
5. `status: "needs_human"` → **FULL STOP — human decision needed.** Present the latest
plan, the returned `contested` findings, and the conversation URL, and ask the
plan, the last pass's returned findings (`contested` from the default loop,
`findings` from the ChatGPT-author loop), and the conversation URL, and ask the
human: approve the latest plan, redirect, or skip the unit. Write no code for this
unit before that decision. (`status: "error"` → read the workflow journal, then
re-invoke or stop.)
Expand Down
31 changes: 13 additions & 18 deletions skills/ship/references/chatgpt-plan-author-loop.workflow.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const meta = {
phases: [
{ title: 'Author', detail: 'ChatGPT writes or replaces the canonical plan privately' },
{ title: 'Review', detail: 'Fable/high reviews the plan read-only against the repository' },
{ title: 'Verify', detail: 'one Sonnet read-only verifier per substantive finding' },
{ title: 'Prepare', detail: 'fold Fable\'s raw findings into revision context for ChatGPT to verify and incorporate itself' },
],
}

Expand Down Expand Up @@ -37,10 +37,6 @@ const REVIEW_SCHEMA = {
} } },
},
}
const VERIFY_SCHEMA = {
type: 'object', additionalProperties: false, required: ['accepted', 'reason'],
properties: { accepted: { type: 'boolean' }, reason: { type: 'string' } },
}
const CONTEXT_SCHEMA = {
type: 'object', additionalProperties: false, required: ['written'],
properties: { written: { type: 'boolean' } },
Expand All @@ -51,7 +47,7 @@ const label = runArgs.unitLabel ?? runArgs.issueUrl
let session = runArgs.session ?? null
let conversationUrl = runArgs.conversationUrl ?? null
let authorContextFile = runArgs.contextFile
let lastContested = { accepted: [], rejected: [] }
let lastFindings = []

for (let pass = 1; pass <= 5; pass++) {
log(`ChatGPT plan authoring / Fable review pass ${pass}/5 — ${label}`)
Expand Down Expand Up @@ -80,26 +76,25 @@ for (let pass = 1; pass <= 5; pass++) {
}
if (review.verdict === 'APPROVED') return { status: 'approved', passes: pass, session, conversationUrl }

const verified = (await parallel(review.findings.map((finding, index) => () =>
agent(
`Adversarially verify this Fable plan-review finding against the ACTUAL repository and canonical plan.\nFinding: ${finding.claim}\nTarget: ${finding.where}\nPlan: ${runArgs.planFile}\nContext: ${runArgs.contextFile}\naccepted=true only when concrete repository, contract, or plan evidence supports it; cite file:line or plan-section evidence either way. ${READ_ONLY}`,
{ label: `verify finding ${index + 1}`, phase: 'Verify', schema: VERIFY_SCHEMA, model: 'sonnet' },
).then(result => ({ finding, accepted: result.accepted, reason: result.reason })),
))).filter(Boolean)
const accepted = verified.filter(item => item.accepted)
const rejected = verified.filter(item => !item.accepted)
lastContested = { accepted, rejected }
lastFindings = review.findings
if (pass === 5) break

// Fable is read-only and does not have live access to confirm exact repository state,
// registry contents, or line numbers — its findings are unverified claims, not settled
// fact. ChatGPT is the plan's sole author and reviser; rather than a separate Sonnet
// pass fact-checking Fable before handing ChatGPT a pre-filtered accept/reject list,
// ChatGPT verifies each finding itself (it already does this kind of live check when
// revising — e.g. looking up an exact npm package version) and decides whether to fold
// it in or reject it, recording either outcome so nothing is silently dropped.
const nextContext = `${runArgs.contextFile}.chatgpt-revision-${pass + 1}.md`
const contextWrite = await agent(
`Create ${nextContext} as a complete revision context. Copy the full original delivery contract from ${runArgs.contextFile}, then append a section "Fable review pass ${pass}" containing these accepted findings to incorporate: ${JSON.stringify(accepted)} and these evidence-backed rebuttals to rejected findings: ${JSON.stringify(rejected)}. If neither list has entries, explicitly require a complete reassessment with concrete findings. Mutation boundary: Write ${nextContext} only; no other file, git, gh, task, memory, or chatgpt-review mutation.`,
{ label: `prepare revision context ${pass + 1}`, phase: 'Verify', schema: CONTEXT_SCHEMA, model: 'sonnet' },
`Create ${nextContext} as a complete revision context. Copy the full original delivery contract from ${runArgs.contextFile}, then append a section "Fable review pass ${pass} — unverified findings" containing these raw findings from an independent read-only reviewer: ${JSON.stringify(review.findings)}. Precede them with this instruction verbatim: "These are UNVERIFIED claims from a reviewer with no live access to confirm exact repository state, registry contents, or line numbers. Before incorporating any finding, verify it yourself against the actual issue, the real repository, and current external sources (e.g. package registries) as needed. Fold in only what you confirm is correct and material. For any finding you determine is wrong, outdated, or already addressed, do not incorporate it — instead add a one-or-two-line entry under a '##' + ' Review responses' section at the end of the plan explaining why, citing your own verification evidence." Mutation boundary: Write ${nextContext} only; no other file, git, gh, task, memory, or chatgpt-review mutation.`,
{ label: `prepare revision context ${pass + 1}`, phase: 'Prepare', schema: CONTEXT_SCHEMA, model: 'sonnet' },
)
if (!contextWrite?.written) return { status: 'error', reason: 'could not prepare ChatGPT revision context', pass, session, conversationUrl }
authorContextFile = nextContext
}

return { status: 'needs_human', reason: 'no Fable APPROVED verdict after 5 passes', passes: 5, session, conversationUrl, contested: lastContested }
return { status: 'needs_human', reason: 'no Fable APPROVED verdict after 5 passes', passes: 5, session, conversationUrl, findings: lastFindings }

function shellQuote(value) { return `'${String(value).replaceAll("'", "'\\''")}'` }
30 changes: 21 additions & 9 deletions skills/ship/references/review-loops.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,14 @@ its task notification; do not poll and do not start other review work meanwhile.
PR review contexts include their `VERDICT:` protocol. ChatGPT authoring context
instead carries the complete delivery contract; the CLI supplies its strict
READY/BLOCKED protocol.
- **Findings are never silently dropped.** Every return carries `accepted` and
`rejected` (with per-finding evidence); the coordinator records them in the ship log
and final report. `rejected` entries become rebuttals, not deletions.
- **Findings are never silently dropped.** In the default plan loop and the code
review loop, every return carries `accepted` and `rejected` (with per-finding
evidence) from an independent Sonnet fact-check; the coordinator records them in the
ship log and final report, and `rejected` entries become rebuttals, not deletions. In
the ChatGPT-author plan loop there is no separate fact-check pass — ChatGPT verifies
Fable's findings itself while revising, and a rejected one must still land as a `##
Review responses` entry in the plan (not a return field); a `needs_human` outcome
there carries the last round's raw `findings` for the coordinator to present.
- **Verify the tree after every workflow** (`git diff`, `git log`, `gh pr list`) — the
fix and revise agents carry stated mutation boundaries, but a prompt is not an
enforced restriction.
Expand Down Expand Up @@ -74,12 +79,19 @@ Workflow {

The canonical plan path never changes. Each pass calls private `plan-author`, has
Fable/high review the resulting complete plan read-only against the repository, and
fans out one Sonnet read-only verifier per substantive finding. Before the next pass,
accepted findings and evidence-backed rebuttals are placed in revision context for
ChatGPT, which returns a complete atomic replacement. The loop stops at the first
Fable `APPROVED`, skips the unit on a concrete `BLOCKED`, and returns `needs_human`
after five non-approved Fable passes or an authoring response that remains incomplete
after bounded same-session retries.
passes Fable's raw findings — explicitly labelled unverified — straight into the next
revision's context. Unlike the default loop, there is no separate Sonnet fact-check
pass here: ChatGPT is the plan's sole author and reviser, and it verifies each finding
itself against the actual issue/repository/external sources (it already does this kind
of live check unprompted, e.g. looking up an exact npm package version) before folding
it in. A finding it determines is wrong or already addressed gets rejected the same way
the default loop's revise agent does — a one-or-two-line entry under `## Review
responses` at the end of the plan, not a silent drop; that section is where a rejected
finding's evidence lives for this loop, since there is no `rejected` return value. The
loop stops at the first Fable `APPROVED`, skips the unit on a concrete `BLOCKED`, and
returns `needs_human` (with the last round's raw `findings`, not an accepted/rejected
split) after five non-approved Fable passes or an authoring response that remains
incomplete after bounded same-session retries.

## Code review pass — `code-review-pass.workflow.mjs`

Expand Down
10 changes: 7 additions & 3 deletions skills/ship/tests/workflow-contract.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@ import { fileURLToPath } from 'node:url';

const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');

test('ChatGPT planner workflow enforces ownership, verification, and the five-pass stop', async () => {
test('ChatGPT planner workflow enforces ownership, ChatGPT-self-verification, and the five-pass stop', async () => {
const source = await fs.readFile(path.join(root, 'references/chatgpt-plan-author-loop.workflow.mjs'), 'utf8');
assert.doesNotThrow(() => new Function(`return async function workflowSyntaxCheck() {\n${source.replace('export const meta', 'const meta')}\n}`));
assert.match(source, /for \(let pass = 1; pass <= 5; pass\+\+\)/);
assert.match(source, /plan-author/);
assert.match(source, /model: 'fable', effort: 'high'/);
assert.match(source, /model: 'sonnet'/);
assert.match(source, /accepted = verified\.filter/);
assert.match(source, /rejected = verified\.filter/);
// Fable's findings go to ChatGPT unverified by a separate pass — ChatGPT (the plan's
// sole author/reviser) checks them itself against the real repository before folding
// any in, per the caller instruction embedded in the revision context.
assert.match(source, /UNVERIFIED claims/);
assert.match(source, /lastFindings = review\.findings/);
assert.doesNotMatch(source, /verified\.filter/);
assert.match(source, /no Fable APPROVED verdict after 5 passes/);
assert.match(source, /status: 'blocked'/);
assert.match(source, /status: 'needs_human'/);
Expand Down