feat(ce-compound): add headless mode:autofix with depth selector#662
feat(ce-compound): add headless mode:autofix with depth selector#662benwilson wants to merge 9 commits intoEveryInc:mainfrom
Conversation
…ded documentation ce-compound had no headless affordance. Its Execution Strategy enforces four mandatory blocking prompts (Full-vs-Lightweight mode selection, session-history opt-in, Discoverability Check consent, post-completion "What's next?" menu), each using the platform's blocking question tool with "Never silently skip" guarantees. This blocked programmatic callers (post-merge hooks, CI, orchestrator pipelines) — the blocking question tool errors in headless sessions and the documented fallback is "present in chat," with no user to respond. The inconsistency was visible against sibling skills in the same plugin: ce-compound-refresh ships mode:autofix, ce-code-review ships mode:autofix/mode:headless/mode:report-only, ce-doc-review ships mode:headless. ce-compound was the lone holdout. This PR adds two mode tokens: - mode:autofix — autofix + Lightweight execution (default headless). Low-cost single-pass capture; overlap drift caught later by ce-compound-refresh. - mode:autofix-full — autofix + Full execution (opt-in thorough). Phase 1 research subagents, overlap-update, Phase 3 specialized reviewers; session-history defaults to off. Mutually exclusive with a conflict error mirroring ce-code-review. All four interactive prompts are guarded in both modes. Discoverability Check becomes a recommendation in the output rather than an instruction- file edit. "What's next?" menu is suppressed. No-op output shape explains which precondition blocked when preconditions aren't met. Learning doc added at docs/solutions/skill-design/compound-skill-improvements.md covers the design decisions (explicit opt-in, Lightweight as default depth, two distinct mode tokens vs nested colons, tokenization rule for the mode:autofix-full prefix ambiguity). Tests: compound-specific (compound-support-files, pipeline-review-contract) 51/51 pass. release:validate clean. Full test suite: same 896/9 as main (the 9 failures are pre-existing in tests/skills/ce-polish-beta-project-type.test.ts, verified by stashing edits and rerunning).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9afa056dbe
ℹ️ 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".
Phase 2.5's multi-candidate branch unconditionally asked the user whether to run a targeted refresh, which would block unattended mode:autofix-full runs (mode:autofix already dodged it via the lightweight rule, but the guard was implicit). Add an explicit autofix guard: recommend ce-compound-refresh via the Follow-up line instead of prompting, and enumerate this prompt in the shared autofix skip list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tmchow
left a comment
tmchow
left a comment
There was a problem hiding this comment.
Thanks for putting this together. The goal is right and I want to land this, but the execution is roughly five times the size of the sibling pattern it claims to follow, and it crosses a few of the plugin's own authoring rules in plugins/compound-engineering/AGENTS.md. I've flagged the specifics inline.
Headline asks:
- Collapse Mode Detection to the shape used by
ce-compound-refreshandce-code-review. One table, one rules block, state each rule once. - Reconsider the two-token design. Orthogonal tokens (
mode:autofixplusdepth:full) or reusingmode:headlesswould sidestep the prefix tokenization rule and remove the parallel Use-when blocks. - Unify the four Autofix output templates into one parameterized block, or extract them to
references/. Four near-duplicate blocks loaded on every invocation is exactly the case the AGENTS.md extraction rule was written for. - Cut runtime-neutral rationale: the Use-when bullets, the subagent-safety paragraph, the
Note:inside the output template, and the caller-parsing instructions. - Drop or radically shrink
docs/solutions/skill-design/compound-skill-improvements.md.ce-compound-refreshdid not ship a parallel learning doc when it addedmode:autofixand I don't want the precedent.
Happy to iterate. Once the size comes down I expect the rest of the review to move quickly.
…KILL.md Responds to @tmchow's PR 662 review. Replaces `mode:autofix` / `mode:autofix-full` with orthogonal `mode:autofix` + optional `depth:lightweight|full|thorough` (default `lightweight`). Collapses Mode Detection to sibling shape (one table + one rules block, 23 lines). Consolidates four ~90-line output templates into one parameterized success template + one short no-op template. Cuts runtime-neutral rationale flagged in review (per-variant Use-when bullets, subagent- safety paragraph, Note: in the output template, parser-contract paragraph). Moves the parser-facing contract to a new reference file not loaded at runtime. Deletes docs/solutions/skill-design/compound- skill-improvements.md — it does not meet the docs/solutions/ institutional-memory bar. BREAKING CHANGE: removes `mode:autofix-full`. Callers must use `mode:autofix depth:full` (full path, session history off) or `mode:autofix depth:thorough` (full path, session history on). No alias or migration shim — the token was introduced in this PR and has no merged users. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review @tmchow — all five asks addressed in 1988594:
Net: -209 / +91 lines in the commit; SKILL.md dropped from 686 to 630. One pre-authorized divergence from your line-44 suggestion (follow-up to #discussion_r3140321365): went with three depth presets ( PR body refreshed to match the new grammar and carries the output contract table. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1988594053
ℹ️ 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".
|
|
||
| ### Autofix mode rules | ||
|
|
||
| - **Skip all user questions.** Never pause — the mode-selection prompt, Phase 2.5 selective-refresh prompt (multi-candidate case), session-history opt-in, Discoverability Check consent, and "What's next?" menu are all suppressed. |
There was a problem hiding this comment.
Autofix mode is documented here as suppressing all user questions, but the Success Output section still has an unconditional "present the What's next? options ... do not continue ... without the user's selection" instruction. In unattended invocations (/ce-compound mode:autofix ...), that conflicting requirement can still trigger a blocking question after writing docs and hang the caller. Add an explicit autofix guard at the post-output routing step so headless runs always terminate without waiting for input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a8cf13a.
You were right — the Mode Detection suppression alone wasn't strong enough to override the "present the options ... do not end the turn without the user's selection" imperative sitting at the prompt site. A headless claude -p --permission-mode bypassPermissions "/ce-compound mode:autofix depth:thorough ..." run before the fix showed the agent emitting the full numbered menu with a confused inline disclaimer ("Autofix mode was specified, so I'm stopping here rather than prompting"), which is exactly the conflicting-requirements case you flagged.
Added a seven-word guard at the prompt site: "In autofix mode, skip this step." Same test run after the fix returns no "What's next?" header, no options, no prompt — clean termination.
Context: my earlier U2 refactor (commit 1988594) deleted a longer version of this guard that used to sit below the prompt instruction, judging it redundant with the Autofix mode rules block. That was too aggressive — the rule is load-bearing at the point of action even when stated in Mode Detection.
|
|
||
| ## Mode Detection | ||
|
|
||
| Parse `$ARGUMENTS` for `mode:autofix` and an optional `depth:<value>` token. Strip matched tokens; treat the remainder as the brief context hint. |
There was a problem hiding this comment.
These parsing rules treat any depth:<value> token as control input and pair that with a hard failure for unknown values, even when mode:autofix is absent. That means normal interactive calls can fail if the free-form context includes a depth: token (for example from pasted logs or prose) that is not one of the three presets. To avoid regressing interactive usage, only interpret/validate depth: when mode:autofix is present; otherwise leave it in the context hint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3d032e1. Two minimal edits:
-
Parse instruction now conditional:
Parse $ARGUMENTS for mode:autofix. If present, also parse an optional depth:<value> token. Strip matched tokens; the remainder is the context hint.— depth: is only parsed when autofix is present. -
Halt rule scoped:
In autofix mode, invalid depth: values halt before subagent dispatch and emit ...— invalid-depth error no longer fires outside autofix.
Verified headless: /ce-compound depth:bogus (no mode:autofix) now enters Interactive mode instead of emitting the halt envelope. The autofix-path halt still fires correctly on /ce-compound mode:autofix depth:bogus.
Net change: 2 insertions / 2 deletions — zero line growth, since both fixes fit on the existing single lines.
…ofix
Phase 1 step 4 was written for interactive mode only ("only if the user
opted in"); it never had an autofix branch. In `mode:autofix
depth:thorough` — which the Mode Detection contract promises dispatches
the Session Historian — the executing agent correctly followed the
literal Phase 1 text and skipped dispatch.
Rewrites the step 4 header and lead bullets to explicitly list both
dispatch and skip conditions with their mode triggers, so the rule at
the dispatch site matches the contract in Mode Detection.
Caught by a headless `claude -p` test run against a scratch repo, which
showed "Session Historian — mode:autofix arg implies skip" even though
depth:thorough was passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prior commit (6f409f8) restated the dispatch rule at the Phase 1 step 4 site, which was necessary to fix the skipped-dispatch bug, but included parenthetical rationale that just repeated what the condition already said: "(autofix mode — depth:thorough is the one autofix path that dispatches Session Historian)" and "(the depth:lightweight default and depth:full both skip Session Historian)". Per AGENTS.md rationale discipline, rationale that would not change runtime behavior if removed belongs elsewhere, not in SKILL.md. Also collapses the paired Dispatch/Skip bullets into one: the Skip condition was the logical inverse of Dispatch, so two bullets doubled text without adding runtime-actionable information. Net: 3 lines of dispatch guidance down to 1; same runtime effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Success Output section still carried an unconditional "present the 'What's next?' options ... Do not continue the workflow or end the turn without the user's selection" instruction at the prompt site. The Autofix mode rules block in Mode Detection lists the "What's next?" menu as suppressed, but that was too weak to override the direct imperative next to the prompt template, and a headless test run confirmed agents were emitting the menu anyway. Adds a four-word guard at the prompt site: "In autofix mode, skip this step." Load-bearing, no rationale, at the exact location where the drift was happening. Addresses EveryInc#662 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prior text treated any depth:<value> token as control input regardless of mode, and the invalid-depth halt rule applied unconditionally. An interactive call whose free-form context happened to include a depth: token (pasted logs, prose, error messages) would hard-fail with an autofix-shaped error envelope even though the user never opted into autofix routing. Scopes both parsing and the halt rule to mode:autofix being present. When mode:autofix is absent, depth: is not parsed, not stripped, not validated — it stays in the context hint. Verified headless: /ce-compound depth:bogus (no mode:autofix) now enters Interactive mode instead of emitting the halt envelope. Addresses EveryInc#662 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Follow-ups since 1988594:
Known drift worth flagging before re-review: in headless |
tmchow
changed the title
tmchow
left a comment
tmchow
left a comment
There was a problem hiding this comment.
@benwilson I updated the PR description to be better as your agent was weirdly making it into a changelog which wasn't necessary.
I'd like to change the mode values to lightweight, standard and deep to match what we do in other skills (vs lightweight, full, and thorough)
Renames `depth:full` → `depth:standard` and `depth:thorough` → `depth:deep` to match the naming convention tmchow set for depth values across skills. Also renames the prose "Full Mode" / "Full" interactive picker option to "Standard Mode" / "Standard" so the prose and tokens stay aligned. Per-PR feedback (EveryInc#662) from @tmchow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@benwilson reconcile teh PR description to ensure it's accurate given the changes please |
|
Done — PR description reconciled with the current branch. It now uses |
There was a problem hiding this comment.
On the headless drift you flagged in #662 (comment) — please address before merge rather than deferring. The current references/autofix-output-parse-contract.md promises stability for fields you've already confirmed don't hold ((mode:autofix) suffix, Depth:, Track:, Category:, Phase 1 subagents: / Phase 3 specialized reviewers: blocks). Shipping a contract that misleads downstream parsers is worse than shipping a smaller, accurate one — post-merge hooks and CI consumers will key off documented anchors and silently break when they drop.
- Trim the contract in
references/autofix-output-parse-contract.mdto what actually holds reliably — the first-line✓ Documentation complete|updated|No documentation writtenstring andFile created:/File updated:paths. Move every other anchor under a clearly labeled Best-effort fields (may be dropped in headless runs) section, with a one-line note that callers must not hard-fail when these are missing. - Update the Output contract table in the PR description to match.
Not asking you to land the structural fix here, that's a separate PR can tackle later. Just don't let the doc overstate what the runtime delivers today.
…iability @tmchow review #pullrequestreview-4231940863 asked us to split the parse contract into reliable / best-effort sections grounded in what actually holds in headless runs. Backed the split with an 18-run sweep against this branch covering {success-complete, no-op} x {lightweight, standard, deep} x 3 runs. Reliable: ✓ Documentation complete and ✓ No documentation written (mode:autofix) as shape-detector substrings (search anywhere; first-line discipline holds 0/18). When the no-op substring is present, Depth:, Reason:, Context considered: co-occur 100% (9/9 in measured cells). Best-effort: ✓ Documentation updated (success-updated path unreachable in claude -p), (mode:autofix) suffix on success lines (lightweight emits its own variants), File created: / File updated:, Track: / Category: / Overlap detected:, Refresh candidate: / Discoverability recommendation:, and the Phase 1 subagents: / Phase 3 specialized reviewers: blocks (0/18 anywhere). Callers must not hard-fail when these are missing. Structural fix to make best-effort anchors reliable is held for a follow-up PR per the original review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Headless reliability sweep — empirical findings (PR 662)Following #pullrequestreview-4231940863, I ran a formal sweep of every parser-anchor field in Bench
Methodology For each cell The Two prompts:
After each run, untracked Intended-vs-actual shape map
The standard/deep "complete" cells all flipped to no-op because Phase 1 verification caught that the claimed fix file ( First-line discipline (literal Across all 18 runs, the documented
The lightweight mode emits its own suffix variants ( Anchor presence (anywhere in stdout body)
Reliability classification The data does not support keeping anything as "reliable" under a 100% / zero-drift bar. The pragmatic story:
Resulting contract trim Reliable section keeps the shape-detector substrings (search anywhere, not first-line) plus the no-op block's supporting anchors:
Best-effort section: Raw run logs are at The structural fix to make the demoted anchors reliable (subagent dispatch for the output step, structured output format, or aligning the lightweight template's suffix with the autofix template) is deferred to a follow-up PR per the original review. |
|
Re: #pullrequestreview-4231940863 Took the empirical route on this — ran an 18-run headless sweep against the current branch so the demotion list is data-grounded rather than asserted. Full methodology and per-cell tallies in the empirical findings comment. The data largely supports your reliable set, with two adjustments:
Trimmed
One runtime finding worth flagging beyond the contract trim: autofix-lightweight invocations emit a different suffix than the autofix template promises. Successes use The structural fix to make best-effort anchors reliable (subagent dispatch for the output step or a structured output format) is held for a follow-up PR per your earlier guidance. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 579e5da9c2
ℹ️ 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".
| - `✓ Documentation complete` — success-complete shape (substring; the `(mode:autofix)` suffix is unreliable on this line, see best-effort) | ||
| - `✓ No documentation written (mode:autofix)` — no-op shape |
There was a problem hiding this comment.
The contract currently treats only ✓ Documentation complete and the no-op line as reliable shape detectors, but this same change also defines a valid autofix success path that emits ✓ Documentation updated (mode:autofix) on overlap-high updates (depth:standard|deep). Parsers that follow this contract can misclassify successful update runs as unknown/failure, which can break headless callers (CI/hooks) that gate on parseable success. Please make ✓ Documentation updated a reliable success anchor (or ensure update output also includes the complete anchor).
Useful? React with 👍 / 👎.
2 participants
Summary
Adds a headless affordance to
/ce-compoundfor programmatic callers such as post-merge hooks, CI jobs, and orchestrator pipelines.mode:autofixenables unattended operation. It suppresses the mode-selection prompt, session-history opt-in, Discoverability consent, Phase 2.5 selective-refresh prompt, and the post-run "What's next?" menu.depth:lightweight|standard|deepselects the unattended execution depth and defaults tolightweight.depth:lightweight-> single-pass documentation, no subagents, no overlap check.depth:standard-> Phase 1 parallel research, overlap-update behavior, and Phase 3 specialized reviewers. Session history is off.depth:deep-> Standard path plus Session Historian dispatch.depth:is parsed only whenmode:autofixis present. Invalid autofix depth values halt before subagent dispatch withce-compound failed. Reason: unknown depth:<value>. Valid values: lightweight, standard, deep.Motivation
ce-compoundpreviously required interactive prompts, which made subprocess invocation unreliable in unattended contexts. A call such asclaude -p "/ce-compound ..."orcodex exec "Run /ce-compound" --ephemeralcould hang or degrade because there was no user available to answer the documented fallback prompt.Sibling skills in this plugin already have explicit unattended modes (
ce-compound-refreshusesmode:autofix;ce-code-reviewsupportsmode:autofix,mode:headless, andmode:report-only;ce-doc-reviewsupportsmode:headless). This PR bringsce-compoundin line with that pattern.Output Contract
The full parser-facing contract lives at
plugins/compound-engineering/skills/ce-compound/references/autofix-output-parse-contract.md; it is not loaded by the skill at runtime.Reliable anchors
Search anywhere in stdout — first-line discipline does not hold; the agent prepends narration before any template block.
Shape detector substrings:
✓ Documentation complete(mode:autofix)suffix is unreliable on this line; see best-effort.✓ No documentation written (mode:autofix)No-op block supporting anchors (present when the no-op detector substring appears):
Depth:lightweight|standard|deep).Reason:Context considered:Best-effort fields
Callers MUST NOT hard-fail when these are absent — they are emitted opportunistically and frequently drop in headless runs:
✓ Documentation updatedsubstring (success-updated path is unreachable inclaude -p— demoted by construction)(mode:autofix)suffix on success-shape lines (lightweight mode emits its own suffix variants)File created:/File updated:pathsTrack:/Category:/Overlap detected:fieldsRefresh candidate:andDiscoverability recommendation:blocksPhase 1 subagents:/Phase 3 specialized reviewers:blocksTesting
bun test tests/compound-support-files.test.ts tests/pipeline-review-contract.test.ts- 51 pass / 0 fail.bun run release:validate- clean.bun testoverall - 896 pass / 9 fail. The 9 failures are all intests/skills/ce-polish-beta-project-type.test.tsand predate this branch.