docs(skills): fix four reference-doc traps in the loop and dispatch surfaces (#170-#173) - #174
Conversation
…, in-lane worker routing, and the incomplete spawn model list Four reference-doc traps reported as #170-#173, all fixed against the shipped source rather than from the prose. - durable-goalplan.md: add the missing `cxc loop add-criterion` line with its real flags, show `init --criterion` repetition, state that criterion ids are generated as c-1, c-2, ... (max existing c-N + 1) and that meet-criterion --id takes those, and give criteria[] its missing `surface` field plus per-field provenance so nobody plans on an --expected-evidence flag that does not exist. (#170, #171) - dispatch-surfaces.md / loop SKILL.md: state the composition rule inside DISPATCH-SURFACE-01 instead of only in the closing section, and route the in-lane worker case in DISPATCH-ROUTE-01. A lane is thread work; a worker inside a lane is subagent work. (#172) - delegation.md: DELEGATE-MODEL-LIST-01 — the advertised spawn_agent override list is a hint, not an allowlist. Pass a user-named model through; only a real spawn rejection proves unavailability. (#173) - structure/20_pabcd_dispatch_doctrine.md: mirror the worker/lane row into the translation table. Docs only; no runtime or CLI behavior changed. Local suite/typecheck/build NOT RUN.
|
✅ Target branch corrected This pull request now targets The |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7036f9b8fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `scenario` is the `--criterion` text and `surface` comes from `--surface` (default | ||
| `logic`); `id` is auto-assigned and `status` is derived. `expectedEvidence` has no |
There was a problem hiding this comment.
Preserve the surface on init-time criteria
When criteria are seeded through the newly documented cxc loop init --criterion path, this blanket statement suggests that --surface web|tui classifies them. However, cxc loop --help advertises --surface only for add-criterion, and the init branch in goalplan-cli.ts calls buildGoalplan with { scenario } only, ignoring the parsed surface; consequently, loop init ... --criterion ... --surface web exits successfully but stores surface: "logic". Because computeQaRequired only recognizes web and tui, this can silently omit the required QA receipt. Either propagate args.surface during init or scope this statement explicitly to add-criterion and document how visual criteria must be registered.
Useful? React with 👍 / 👎.
| A **lane** is thread work; a **worker inside a lane** is subagent work. N lanes | ||
| means N worktree threads, and the workers inside each lane are that lane's | ||
| subagents — they cannot collide across lanes because the worktrees differ. |
There was a problem hiding this comment.
Do not make the word “lane” force thread routing
This absolute definition contradicts the same document's earlier statement that the word “lane” does not select a surface and its later instruction to route by ownership rather than parallelism. For requests such as “fan out three read-only lanes” or bounded non-overlapping slices in the current checkout, these lines require three user-visible worktree threads while DISPATCH-ROUTE-01 requires subagents, leaving agents with incompatible STRICT instructions. Restrict this definition to branch/worktree lanes, or keep “lane” neutral until the ownership requirements select a surface.
Useful? React with 👍 / 👎.
LOOP-DOCS-FIRST-01: the delivery roadmap is written to diff level before the first merge or version edit, not discovered phase by phase. - 000_plan.md — objective, constraints, measured starting state, the four work-phase map, and the delete_branch_on_merge hazard that already cost this repository its dev branch once. - 010 — landing #174 on dev, and why the dev push is a separate CI generation: the release gate reads conclusions by SHA, and WSL never runs on pull requests. - 020 — the exact thirteen version surfaces collectSurfaces() enumerates, which two may carry +codex build metadata, the two declared workspaces (gui, cli) that it does not read, and the hosted tests=3150 measurement the inventory regeneration must use. - 030 — the promotion PR's enforce-target exemption, ruleset protect-main with its six required contexts and no bypass actors, the immovable v* tag ruleset, and the dev restore step the promotion merge will require. Docs only. Local suite/typecheck/build NOT RUN.
Four reference-doc traps reported as #170-#173 and landed on dev as #174: the missing add-criterion CLI surface, generated c-N criterion ids, the in-lane worker routing that DISPATCH-SURFACE-01 buried, and DELEGATE-MODEL-LIST-01 for the incomplete spawn_agent override list. Version surfaces: every surface check-versions.mjs enumerates moves 0.2.27 -> 0.2.28, plus the two declared workspaces it does not read (gui, cli) and the thirteen workspace self-versions in package-lock.json. The plugin manifest and inventory carry the regenerated +codex.20260914090142 build metadata, which never affects precedence. Inventory regenerated with the measured total from hosted CI, not a local run: run 34790437740 job 103813520900 reported "tests 3150" at head 7036f9b. The published badge already read 3,150, so it does not move and the READMEs are untouched — only the inventory's version fields changed. Local suite/typecheck/build/install NOT RUN. Proof for this head is the hosted dev CI generation; the release gate reads conclusions by SHA.
Four reference-doc traps reported as #170-#173. Each fix was checked against the shipped source, not the surrounding prose.
What was wrong
#170 / #171 —
skills/loop/references/durable-goalplan.md. The file makes plan registration mandatory (an init-only plan failscxc loop validateE8, andupdate_goal {status:"complete"}is hook-denied behind GOAL-COMPLETE-GATE-01), documents thecriteria[]field names, and then omits the only command that writes them. Reading the schema line, the natural first call isadd-criterion --id c1 --scenario ... --expected-evidence ..., which fails with--criterion "<scenario>" is required. Separately,meet-criterion --id <id>reads as if the caller picks the id the wayadd-work-phase --id wp1does; it does not, so an agent that assumedc1at registration later records no evidence for a criterion it believes it met.#172 —
skills/pabcd/references/dispatch-surfaces.mdand the DISPATCH-SURFACE-01 bullet inskills/loop/SKILL.md. Substance was correct, order was not. Four STRICT blocks all push toward threads, and the correcting sentence about in-lane subagents was the last paragraph of the file. Given "orchestrators are model A and B, workers are C and D at a 2:3 ratio, dispatched into new worktrees", routing came out as lanes-are-threads (right) and workers-are-also-threads (wrong), needing two user corrections.#173 —
skills/pabcd/references/delegation.md.spawn_agentadvertises five model overrides. A user asked fordevin/swe-2; its absence from that list was read as unavailability and the requested ratio was silently re-planned around threads. The list is incomplete, not authoritative.What changed
durable-goalplan.md: added the realadd-criterionsignature to the CLI surface, showedinit [--criterion <text>]..., stated that ids are generated asc-1,c-2, ... (max existingc-N+ 1, in registration order) and thatmeet-criterion --idconsumes those, noted the duplicate-scenario rejection, and gavecriteria[]its missingsurfacefield with per-field provenance —scenario/surfaceare CLI-settable,idandstatusare derived,expectedEvidencehas noadd-criterionflag,capturedEvidencecomes frommeet-criterion --evidence.dispatch-surfaces.md: the composition rule now sits inside DISPATCH-SURFACE-01 — a lane is thread work, a worker inside a lane is subagent work, and in-lane subagents cannot collide across lanes because the worktrees differ. DISPATCH-ROUTE-01 gained the matching route line. The closing section stays as the expanded form.loop/SKILL.md: the DISPATCH-SURFACE-01 bullet mirrors one clause of the same rule, so summary and reference agree.delegation.md: new DELEGATE-MODEL-LIST-01 (STRICT) next to the V1 spawn table — the advertised override list is a hint, not an allowlist; pass a user-named model through as given; only a real spawn rejection is evidence of unavailability; report a genuine failure instead of substituting.structure/20_pabcd_dispatch_doctrine.md: mirrored the worker-inside-a-lane row into the translation table.Verification
Flags, id generation, duplicate handling and field defaults were read out of
plugins/codexclaw/components/pabcd-state/src/goalplan-cli.ts(renderGoalplanHelp,runAddOp),steering.ts(applyOps,c-${maxId + 1}) andgoalplan.ts(buildGoalplan,c-${i + 1}).DELEGATE-MODEL-LIST-01 is backed by a live measurement taken while preparing this PR: three
spawn_agent({ model: "devin/swe-2" })calls succeeded and produced the durable-goalplan, dispatch-surface and delegation edits respectively, while the host description still omitted that model. Those subagents also landed their edits in this checkout on this branch, which is a second confirmation of DISPATCH-SHARED-TREE-01.Docs only; no runtime or CLI behavior changed. Local suite, typecheck, build and install: NOT RUN.
Closes #170
Closes #171
Closes #172
Closes #173