diff --git a/CHANGELOG.md b/CHANGELOG.md index 5112f956..163f1319 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,69 @@ All notable changes to codexclaw are documented here. The format follows ## [Unreleased] +## [0.2.13] — 2026-08-25 + +Three gates that obstructed the agents following them, and the operational +lessons from a release train that got several of these wrong first. + +### Fixed + +- **The attest gate rejected what its own documentation taught.** `coerceAttest` + requires `from`/`to` before any other check, and the "Required attest keys" + table in `cxc-pabcd` — the table agents copy — named neither, nor `planUnit`, + `workPhaseId`, or `testReceiptPath`. A goalplan-bound `P>A` therefore cost + three separate refusals, one turn each. `attest JSON missing valid from/to` + appears 50+ times across four repos since 2026-08-13. + + The table now names every key each edge requires, with copy-paste objects. The + refusal names the real edge instead of restating the problem: `to` is the verb, + `from` is the session's phase, and only that edge's extra keys are listed. An + illegal edge gets its legal routes and no example, because every example would + be rejected. + + Same family, found while inventorying: the goal-idle block emitted `evidence` + where the schema says `did` — silently, since `IDLE>P` is ungated; + `review-round`, `plan`, `metric` and `divergence` rejected `--help`; and + `cxc freeze --help` ran the freeze and wrote `freeze.json`, exiting 0. + +- **Interview readiness was unreachable.** `isInterviewReady` demanded level + `max` on all four dimensions, and no shipped writer could produce it — + `deriveLevel` tops out at `high` and `--dim =max` is rejected. Every + interview either dead-ended or spent an attested override, which made the + override's ledger row meaningless: it recorded "bypassed the gate" for the + thorough interview and the skipped one alike. + + Simply accepting `high` would have been worse — four `--known` flags reach + all-`high` in one command. So the gate now asks the append-only Q&A ledger where + a level came from: a dimension counts when a question was asked, answered, and + attributed with `--map`. `max` still satisfies readiness as a deliberate + assertion, and the override survives as the exception it was designed to be. + + Not breaking: an interview that passes today still passes. Worth knowing on + upgrade — a session already sitting at all-`high` becomes shape-ready, so + `flags.interview`, `cxc freeze`, and the human `orchestrate p` free-pass will + now treat it as ready. The agent CLI path still requires the ledger backing. + This unsticks interviews that were stranded; it does not close any that were open. + +### Added + +- **`DEVOPS-*` freeze-gate rules** in `cxc-dev-devops` §2.8 and its references: + pin a readiness report to the SHA its gates describe; never excuse a red gate + inside the report it failed; unresolved review threads on merged PRs block GO; + a gate with no implementing phase is a wish. Plus the evidence mechanics — + replay CI's real partition, prove "environmental" with a baseline triple, do + not change the verification instrument while certifying with it, re-read the + head before claiming exact-head evidence. + +### Changed + +- **The flaky-test policy is elimination-first and has one owner.** + `dev-testing` said a flake is a bug and also said "quarantine if blocking"; + the router and its reference both claimed the protocol, with different + strength. `references/ci-pipeline.md` §5 is now canonical `TEST-FLAKE-*`: + re-running to green is not a fix, quarantine needs test name, owner, deadline + and suspected cause recorded together, and "environmental" needs proof. + ## [0.2.12] — 2026-08-22 Three bugs filed against codexclaw, all of them cases where the tool obstructed diff --git a/README.ko.md b/README.ko.md index 807f2bc5..45c0e32f 100644 --- a/README.ko.md +++ b/README.ko.md @@ -13,7 +13,7 @@

CI - 1,961 tests passing + 1,995 tests passing 28 skills 22 hooks Documentation diff --git a/README.md b/README.md index 6f7e3fde..3af0cb96 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@

CI - 1,961 tests passing + 1,995 tests passing 28 skills 22 hooks Documentation diff --git a/README.zh.md b/README.zh.md index df1bce02..6788e32b 100644 --- a/README.zh.md +++ b/README.zh.md @@ -13,7 +13,7 @@

CI - 1,961 tests passing + 1,995 tests passing 28 skills 22 hooks Documentation diff --git a/bin/codexclaw.mjs b/bin/codexclaw.mjs index b1c1f1fe..49cfb015 100755 --- a/bin/codexclaw.mjs +++ b/bin/codexclaw.mjs @@ -264,6 +264,9 @@ const TOP_LEVEL_HELP = [ " loop init|show|validate manage the project-local goalplan substrate", " goalplan init|show|validate deprecated alias for loop", " plan init [--phases N] scaffold the devlog/_plan unit the P>A gate verifies", + " receipt test -- produce the test receipt a bound C>D requires", + " review-round open|show|abort the opt-in A-gate plan-audit round", + " scan record|show record interview coverage and contradiction scans", " metric record/show objective metrics", " divergence record divergence mode and candidate archive state", "", @@ -282,6 +285,8 @@ const TOP_LEVEL_HELP = [ "", "Agent notes:", " Mutating PABCD commands require the current session id: cxc orchestrate P --session ", + " Every --attest object names the edge it advances: {\"from\":\"\",\"to\":\"\",\"did\":\"...\"}", + " plus that edge's keys. Run cxc orchestrate --help for the per-edge examples.", " Use --json only on subcommands that document it, such as orchestrate status --json.", " For command-specific help, start with: cxc orchestrate --help or cxc map --help.", ].join("\n"); diff --git a/cli/package.json b/cli/package.json index f8b8a79a..136e3612 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/cli", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "codexclaw CLI — status, subagent config, provider toggle, GUI launcher.", diff --git a/devlog/_plan/260822_attest_win_parity/040_recipe_verified.md b/devlog/_plan/260822_attest_win_parity/040_recipe_verified.md new file mode 100644 index 00000000..d97e5f6b --- /dev/null +++ b/devlog/_plan/260822_attest_win_parity/040_recipe_verified.md @@ -0,0 +1,39 @@ +# 040 - the win32 recipe, executed rather than assumed + +The attest fix tells Windows users to do this: + +```powershell +'' | Set-Content -Encoding utf8 .codexclaw/attest.json +cxc orchestrate --session --attest-file .codexclaw/attest.json +``` + +That recommendation was executed before it was written down, because the +archive already contains a case where the obvious encoding advice is wrong +(`fuck-powershell#7`: `Out-File -Encoding utf8` writes a BOM on 5.1, and +`utf8NoBOM` does not exist there at all). + +## Measured + +``` +bytes=48 head=efbbbf7b2266726f bom=UTF-8 BOM +utf8 matches=1 +``` + +So `Set-Content -Encoding utf8` does prepend a BOM here too. The recipe is still +safe, because `orchestrate-cli.test.ts` already carries an explicit case: + +> `#31: --attest-file tolerates a UTF-8 BOM (PowerShell 5.1 Set-Content -Encoding utf8)` + +The CLI accepted the file; the only refusal was the phase gate (`IDLE -> D`), +which is correct. + +## Why this is worth a page + +Two of the three recommendations in the wider archive turned out to be wrong on +this host. A workaround that has not been run on the platform it targets is a +guess, and shipping a guess inside an error message is worse than shipping no +message — the agent trusts it and loses a turn. + +The recipe is now exercised continuously rather than tested once: every attest in +this loop since the fix has gone through `--attest-file`, including the ones +closing these cycles. diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/000_baseline_and_scope.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/000_baseline_and_scope.md new file mode 100644 index 00000000..c02f62fc --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/000_baseline_and_scope.md @@ -0,0 +1,146 @@ +# 000 — baseline and scope: the attest from/to cascade, and the lessons opencodex paid for + +Two things share one unit because they share one failure shape: **a gate that +rejects work the documentation told the agent to do.** + +The attest half is the shipped CLI refusing an attestation whose shape the skill +never described. The devops half is opencodex's v2.32.1 freeze train discovering, +repeatedly, that a gate nobody can satisfy honestly gets satisfied dishonestly — +a red suite argued into an exception, a flaky test re-run until green, a +readiness report describing a tree that had already moved. + +The flaky-test policy sits between them. It is currently the clearest instance +in this repo of a rule that contradicts itself in the same file. + +## Baseline + +Repo: `/Users/jun/Developer/new/700_projects/codexclaw` at `74245989` +(`devlog: the win32 recipe was executed, not assumed`), version 0.2.12. + +Pre-existing dirty state, NOT created by this unit and to be preserved: + +``` + M scripts/dev-symlink.sh +?? devlog/_plan/260722_260722-repo-governance-config/ +?? devlog/_plan/260814_260814-fix-main-ci-windows-worktree/ +?? mktemp: +``` + +Test baseline before any edit, full declared command: + +``` +npm test +-> tests 1961 pass 1961 fail 0 duration_ms 36311.187 exit 0 +``` + +The pabcd-state slice alone, run twice by the error-hunt lane: +865 pass / 0 fail both times, exit 0, 6.24s then 9.54s. Same 865 test names. +**No flaky test was observed in this repo's own suite.** That matters for scope: +the flaky work here is policy text, not a test repair. + +## The defect, stated exactly + +`coerceAttest` returns null unless `from` and `to` are strings +(`components/pabcd-state/src/attest.ts:91-96`). The CLI turns that null into: + +``` +attest JSON missing valid from/to +``` + +at `orchestrate-cli.ts:227` (inline) plus `orchestrate-grammar.ts:88` for the +chat surface. The `--attest-file` path at `:257` emits a DIFFERENT string, +`attest file is missing valid from/to` — same defect, separate wording, +and therefore separate test coverage (see 002 blocker 2). + +The contract agents actually read — the "Required attest keys" table at +`skills/pabcd/SKILL.md:91-98` — lists `did`, `auditOutput`, `auditVerdict`, +`auditResidual`, `checkOutput`, `exitCode`. It never names `from` or `to`. +An agent that copies the table writes `{"did":"..."}` and is refused before any +other check runs. + +This is not theoretical and it is not rare: + +``` +cxc chat search "missing valid from/to" --days 0 +-> 50 hits (3/9457 files scanned) +``` + +across opencodex, ima2-gen, cli-jaw and codexclaw sessions, the oldest sampled +at 2026-08-13. Every one of those is a wasted turn inside somebody's loop. + +### It is a cascade, not a single error + +The from/to refusal is only the first gate. Fixing it alone walks the agent into +the next two, because `planUnit`, `workPhaseId` and `testReceiptPath` are ALSO +absent from every skill doc (`rg` over pabcd/loop/interview returns zero hits for +all three) while the runtime requires them: + +| # | Refusal | Source | +|---|---------|--------| +| 1 | `attest JSON missing valid from/to` | `orchestrate-cli.ts:227` (parse time, before session/plan/binding) | +| 2 | `P -> A requires "planUnit"` | `plan-gate.ts:38-43` | +| 3 | `A goalplan is bound ... pass "workPhaseId"` | `attest.ts:148` | +| 4 | `C -> D on a goalplan-bound session requires "testReceiptPath"` | `check-gate.ts:37` | + +A bound HOTL session on P>A therefore needs FIVE keys the skill names ONE of. +Documenting only `from`/`to` would trade one round trip for two. + +### Why 260822 did not already fix this + +`devlog/_plan/260822_attest_win_parity/` fixed a different failure with a +similar surface: on Windows, PowerShell mangles inline `--attest '{...}'` so the +CLI sees invalid JSON. That unit made the recipes platform-aware and pushed +`--attest-file`. It never touched the key list, because the key list was not the +bug it was chasing. The from/to omission survived that sweep intact. + +## Scope + +IN: + +- `skills/pabcd/SKILL.md` attest table and the copy-paste examples under it +- `skills/interview/SKILL.md:64,144` (two override examples missing from/to) +- `skills/loop/SKILL.md` attest references +- `structure/20_pabcd_dispatch_doctrine.md:72` (names the keys as invalid JSON) +- `components/pabcd-state/src/orchestrate-cli.ts` + `orchestrate-grammar.ts` + null-coerce error text, and `hook.ts` injected examples +- `components/pabcd-state/test/` regression coverage +- `skills/dev-devops/SKILL.md` + `references/ci-cd-deploy.md`, + `references/sre-foundations.md` (new DEVOPS-* rules) +- `skills/dev-testing/SKILL.md` §5.4 + `references/ci-pipeline.md` §5, + `skills/dev-debugging/SKILL.md` Scenario D + anti-pattern row, + `skills/dev/references/skill-ownership.md` (missing flaky row) +- this devlog unit + +OUT: + +- `~/.codex/plugins/cache/codexclaw/**` — the installed payload. The repo is the + source of truth; a rebuild may be RUN, no cache file is authored. +- the opencodex repo. It is read-only evidence this cycle: no branch, no PR, no + commit there. +- `git push`, PRs, npm publish, version bumps. Local commits only (LOOP-GIT-01). +- repairing the interview-readiness dead end found by the error-hunt lane (001 + §F.1). It is a design decision about what `isInterviewReady` should accept, + not a text fix, and it gets its own unit. +- the docs-site quickstart and guide attest examples. They are a live + copy-paste surface and they are incomplete, but docs-site is its own build + with its own review; recorded as a follow-up (002 nit 4). + +**Amended after audit (002 blocker 3):** `cxc freeze --help` was originally +listed OUT here while 010 §7 listed it IN. It is now **IN**. It is a workspace +mutation behind a read-only-looking flag that exits 0, so nothing signals it, +and the guard belongs in the same file family wp1 already opens. + +## Accept criteria + +| # | Criterion | Proof | +|---|-----------|-------| +| 1 | The attest table names every key the runtime requires per edge, with a copy-paste object | diff + `rg` for `workPhaseId` in `skills/pabcd` returning hits | +| 2 | The null-coerce refusal prints a correct example for the requested edge | actual CLI output of a failing invocation | +| 3 | A regression test pins that message | test name + green run | +| 4 | dev-devops carries the freeze-train lessons as named rules with devlog citations | `rg` for the new rule ids | +| 5 | Flaky guidance is elimination-first with one canonical owner and no surviving contradiction | `rg -i -e flaky -e quarantine` across the four skills | +| 6 | `npm test` green at the final tree | `cxc receipt test` path, exit 0 | + +## Terminal outcome expected + +DONE. `NOOP` is unavailable: the divergence is measured above, not hypothesized. diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/001_current_state_inventory.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/001_current_state_inventory.md new file mode 100644 index 00000000..d748f314 --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/001_current_state_inventory.md @@ -0,0 +1,221 @@ +# 001 — current-state inventory + +Four read-only `xai/grok-4.6` lanes produced this, dispatched in parallel from +the P phase of wp0. Every row below was re-checked against the tree before +being written down; claims the lanes could not evidence are marked as such. + +## A. Where the attest contract is documented, and whether it is true + +| Surface | file:line | from/to? | planUnit / workPhaseId? | +|---|---|---|---| +| **Required attest keys table** | `skills/pabcd/SKILL.md:91-98` | **No** | **No** | +| chat grammar | `skills/pabcd/SKILL.md:49` | no JSON | — | +| Windows recipe | `skills/pabcd/SKILL.md:56-58` | placeholder `''` | — | +| per-phase artifact prose | `skills/pabcd/SKILL.md:76-82,108-112` | no JSON | — | +| loop mandate | `skills/loop/SKILL.md:27-32,101` | no example object | — | +| interview override | `skills/interview/SKILL.md:64` | **No** (`{"override":true,...}`) | — | +| interview override | `skills/interview/SKILL.md:144` | **No** (`{"override":true}`) | — | +| interview override | `skills/interview/SKILL.md:180-181` | Yes | — | +| doctrine | `structure/20_pabcd_dispatch_doctrine.md:72` | names keys as `{"from","to","did"}` — **not valid JSON** | — | +| CLI help, posix | `orchestrate-cli.ts:166-168` | **Yes** | **Yes** | +| CLI help, win32 | `orchestrate-cli.ts:161` | Yes | Yes | +| Stop-block commands | `hook.ts:1036-1040` | Yes | no workPhaseId | +| goal-idle block | `hook.ts:1163-1164` | Yes, but key is `evidence` not `did` | — | +| loop-arm directive | `hook.ts:471-478,493` | **no object at all** | — | +| docs-site quickstart | `docs-site/.../quickstart.md:22,30,38,46,56` | Yes | A→B missing `auditVerdict`; P→A missing `planUnit` | + +The best attest documentation in the tree is `cxc orchestrate --help`. The worst +is the table in the skill the agent is instructed to load. `rg` over +pabcd/loop/interview SKILL.md returns **zero** hits for `planUnit`, +`workPhaseId`, and `testReceiptPath`. + +`hook.ts:1163` deserves its own line: the goal-idle block hands the agent +`{"from":"IDLE","to":"P","evidence":""}`. `evidence` is not +a field `coerceAttest` reads. IDLE→P is ungated so it advances anyway, which is +worse than failing — it teaches a wrong field name that fails silently. + +## B. Validation order (why fixing one key is not enough) + +From `parseOrchestrateCliArgs` → `runOrchestrateCli`: + +``` +parse: JSON.parse → coerceAttest (from/to must be strings) [1] +run: attestError short-circuit (:345) + --session guards (:373-395) + P>A: validatePlanArtifacts → planUnit [2] (:418-421) + gated + bound slug: validateWorkPhaseBinding [3] (:428-438) + I>P override: did, then from/to must be I/P (:455-459) + transition() → validateAttest: + attest null (attest.ts:173) + from/to mismatch (attest.ts:176-178) + did / A>B extras / C>D extras (attest.ts:182-230) + review-binding, SOURCE-DELTA, C>D receipt +``` + +Worked cascade for the literal skill-table copy on a bound P>A +(`--attest '{"did":"wrote the plan"}'`): + +1. `attest JSON missing valid from/to` +2. `P -> A requires "planUnit": ...` +3. `A goalplan is bound ... pass "workPhaseId" in the attest` +4. only then mismatch / empty-did / edge extras + +Three round trips, each one a full turn, all caused by one incomplete table. + +Note on `coerceAttest`: its comment at `attest.ts:88` claims it returns null +when from/to "are not valid phases". It does not check Phase membership — only +`typeof === "string"`. `{"from":"plan"}` coerces fine and dies later at +`attest.ts:178`. The comment is wrong; the behavior is defensible (the mismatch +error is more specific). Recorded, not a defect to fix. + +## C. The runtime error text + +`orchestrate-cli.ts:227`, `:257` and `orchestrate-grammar.ts:88` each emit a +bare string with no example, no phase context, no next command. Note `:227` and +`:257` are NOT the same literal — the file path emits +`attest file is missing valid from/to` — which matters for test coverage. +Compare `attest.ts:173`, which for a MISSING attest already prints: + +``` +P -> A requires an attestation with a non-empty "did". Pass --attest-file +(required on Windows) or --attest '{"from":"P","to":"A","did":"..."}'. +``` + +So the codebase already knows how to write this message. The malformed-attest +path just never got the same treatment. An agent whose attest is EMPTY gets +useful help; an agent whose attest is INCOMPLETE gets nothing. + +## D. opencodex lessons, with citations + +Existing dev-devops rule ids are only `DEVOPS-AUTH-01`, +`DEVOPS-RELEASE-PROOF-01`, `DEVOPS-AGENT-SAFETY-01`. None of the below is +already stated. `DEVOPS-RELEASE-PROOF-01` is adjacent — it governs a published +artifact's proof bundle — but it does not pin a readiness REPORT to a code SHA +and does not forbid rewriting a red gate. + +| Proposed id | Severity | Statement | Source | +|---|---|---|---| +| `DEVOPS-FREEZE-SHA-01` | STRICT | Pin a readiness/GO report to the code SHA its gates describe; if later commits exist, prove they are docs-only | `260824_v2_32_1_hotfix_train/900_go_nogo_readiness_report.md:3-7` | +| `DEVOPS-SUITE-PARTITION-01` | STRICT | A local one-process full suite is not the CI suite gate; replay CI's real partition and record both forms | `900:54-58`, `run-bun-test-batches.sh:50`, `ci.yml:234-244,301-338` | +| `DEVOPS-GATE-WEAKEN-01` | STRICT | A red named gate is not excused by rewriting the report; make it green or replace it with a pre-declared equivalent BEFORE the verdict | `900:47-49`, first freeze at `02c302a54` rejected on this count | +| `DEVOPS-REVIEW-THREADS-01` | STRICT | Unresolved review threads on merged PRs are a GO blocker; count after merge | `900:40-46`, pre-declared at `080_wp8:47` | +| `DEVOPS-BASELINE-DEFECT-01` | STRICT | A local red test is a candidate defect until all three hold: identical failure on the untouched baseline SHA, no merged unit touching that code, CI's matching job green at the freeze SHA | `900:69-72`, `010_wp1:179-181` | +| `DEVOPS-VERIFY-INSTRUMENT-01` | STRICT | Do not change the verification instrument while using it to certify a freeze | `070_wp2:44-45,117-121`, `000_baseline:140-145` | +| `DEVOPS-EXACT-HEAD-01` | STRICT | Re-read the PR/branch head immediately before claiming exact-head evidence; a remembered pass is not evidence | `070_wp2:136-139,87-98`; `260825_operator_visibility_train/000:63-65` | +| `DEVOPS-FLAKE-STABILITY-01` | DEFAULT | A flaky-capable suite is stable only after N consecutive greens at ONE head plus the required matrix; one green run is not a land signal | `070_wp2:94-98,125-126` | +| `DEVOPS-GATE-OWNER-01` | STRICT | A mandatory GO gate needs an implementing work-phase and a terminal recorded outcome | `090_wp9:6-8`, `000_baseline:243-245` | +| `DEVOPS-STALE-PROCESS-01` | STRICT | A live long-running process is not candidate evidence until its start time, binary and config are proven to match the freeze SHA | `090_wp9:21-23` (PID 922 from the bug report, measured as if it were the fix) | +| `DEVOPS-OBS-SIGNAL-01` | DEFAULT | When an operator surface lacks a signal, add the missing signal; never flip an already-true status bit to compensate | `260825_operator_visibility_train/020_wp3:16-22`, `001:97-118`, `030:75-87` | + +Read and deliberately NOT proposed as devops rules (they belong to product, +testing, or git-train surfaces): the sidecar backend ternary, the two-null-policy +warning, version-manager shim adoption, "reproduce from the reported observable", +and the `--ff-only`-vs-rebase decision. + +## E. Flaky policy: the repo currently contradicts itself + +**CONTRADICTS** (quarantine/retry/timeout as default or acceptable): + +| file:line | text | +|---|---| +| `dev-testing/SKILL.md:218` | `Protocol: detect → quarantine if blocking → assign owner → reinstate after repeated green runs.` | +| `dev-testing/references/ci-pipeline.md:96-102` | `## 5. Flaky Test Quarantine Strategy` — `2. move it to a quarantine tag or job` | +| `structure/30_contradiction_register.md:85` | C10 flake: "candidate for an explicit timeout or build/test serialization" | + +**ALIGNED** (already elimination-first) — the majority, which is why the +contradiction is so sharp: + +- `dev-testing/SKILL.md:77` `TEST-ANTI-FLAKE-01`: "A time-based flake is a bug. + Do not use sleep-based synchronization, retry-as-fix, or green-on-retry + acceptance without a deterministic cause and harness correction." +- `dev-testing/SKILL.md:220-224` `TEST-CI-GREEN-01`: "never blind-retry a failed + job" +- `dev-testing/SKILL.md:380` "Flakes are diagnosed, not accepted through retry." +- `dev-testing/SKILL.md:482,491` `.skip()` on a failing test is an escalation red flag +- `dev-testing/references/ci-pipeline.md:104-109` first-fix table +- `dev-debugging/SKILL.md:78` "Add retry/skip annotation" is the WRONG-patch column +- `dev-debugging/SKILL.md:309-311` Scenario D: fix isolation, then hunt siblings + +### The six contradictions + +| # | Conflict | +|---|---| +| C1 | `TEST-ANTI-FLAKE-01` (a flake is a bug) vs `:218` (quarantine if blocking) — same file | +| C2 | `TEST-CI-GREEN-01` (never blind-retry, fix on latest HEAD) vs `:218` (green without a fix) — same file | +| C3 | `SKILL.md:218` vs `ci-pipeline.md:96-102` — duplicated canonical text, and the router's version is WEAKER (no removal deadline) | +| C4 | `dev-testing` quarantine vs `dev-debugging:78` which lists skip as the anti-pattern | +| C5 | `dev-testing:491` (`.skip()` = red flag) vs quarantine tags, which are skip renamed | +| C6 | `dev-debugging:74` (raising a timeout is the wrong patch) vs `structure/30:85` (timeout as a candidate) | + +`dev-devops` carries NO flaky policy of its own — it defers at `SKILL.md:328` +to `dev-testing §5`. So it inherits whatever §5 says, and needs a pointer, not a +copy. + +### Canonical-owner precedent + +`DEV-STACK-*` is the model: canonical text in +`skills/dev/references/stacked-prs.md`, declared in +`skills/dev/references/skill-ownership.md` ("Each rule area has exactly one +canonical owner. Other skills may contain stubs but MUST NOT duplicate canonical +content."), and one-line pointer stubs in `pabcd`, `loop`, +`dev-code-reviewer`, `dev-devops`. + +The flaky family has **no row** in `skill-ownership.md`. That missing row is why +the policy drifted into two files with different strength. + +## F. Other defects found (recorded, mostly out of scope) + +The error-hunt lane swept every agent-facing error string and the shipped-vs- +documented CLI surface. Findings, ranked by how often they bite mid-loop: + +1. **I→P can never become ready honestly.** `isInterviewReady` requires all four + dimensions at `"max"` (`interview.ts:253-270`), `scan record --dim x=max` is + rejected (`scan-cli.ts:143-151`), and `deriveLevel` never emits `max` + (`scan-cli.ts:269-271`) — while `interview/SKILL.md:52` tells agents scan-record + is the path. Every HITL interview either dead-ends or forges `override:true`. + **Own unit. Not this one.** +2. **The attest table vs the gates.** This unit's wp1. +3. **`--help` is not a contract on the verbs used at P and A.** + `cxc review-round --help` and `cxc plan --help` exit 1 as unknown verbs + (`review-round-cli.ts:99`, `plan-cli.ts:75`); `cxc doctor --help`, + `cxc metric --help`, `cxc divergence --help` also fail. `help-verbs.test.ts` + covers only loop/receipt/scan — the #47 fix stopped halfway. Checkout + `bin/codexclaw.mjs:249-286` omits `receipt`, `review-round`, `scan`, + `release` from top-level help while telling agents to try ` --help`. +4. **`illegal transition X->Y`** (`fsm.ts:43`) names no legal edges and no + `cxc orchestrate status`. +5. **`cxc freeze --help` MUTATES the workspace.** It ignores `--help` and runs + the freeze, writing `.codexclaw/interview/freeze.json` — observed live at + 2026-08-25T02:43:37Z during this inventory. Exit 0, so nothing signals it. + `.codexclaw/` is gitignored, so the worktree stayed clean; the behavior is + still wrong. +6. `cxc loop --help` documents `--slug` on `steer`/`add-work-phase`/ + `add-criterion`; the runtime ignores it and binds by session + (`goalplan-cli.ts:306` vs `:184`). Passing the documented flag is a no-op. + `loop/SKILL.md:255-261` omits those three shipped verbs entirely. + +Items 3, 4, 5, 6 are the same family as the attest bug — a surface that refuses +or misleads an agent following the docs — and are cheap. They are folded into +this unit as wp1 scope where they touch the same files, and recorded as +follow-ups where they do not. Item 1 is a design decision, not a text fix, and +is explicitly deferred. + +## G. Suite health (baseline, no repair needed) + +`npm test`: 1961 pass / 0 fail / exit 0 / 36.3s. +pabcd-state alone, twice: 865 pass / 0 fail both runs, identical test-name sets. + +Contention-sensitive but currently green: `cli-bounds.test.ts` and the dist-CLI +spawn cases in `orchestrate-cli.test.ts` (which carry an in-file comment near +`:486` that concurrent spawn is flaky); `orchestrate-cli.test.ts:478` depends on +`utimesSync` + `Date.now`; nearly every IO test shares the `os.tmpdir()` pool via +unique `mkdtempSync` dirs. No `setTimeout`, no port binding. +`--test-concurrency=1` in the declared command is load-bearing. + +Removed from this watch list after audit: `session-split.test.ts:70` uses +`Date.now()` only to mint a name for a directory that must not exist. It has no +timing dependency and listing it here was wrong (002, citation corrections). + +**Conclusion for scope:** there is no flaky test to eliminate in this repo today. +The flaky work is policy text, and the policy must not be written as if the +repo's own suite were the problem. diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/002_plan_audit.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/002_plan_audit.md new file mode 100644 index 00000000..cb2afe7f --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/002_plan_audit.md @@ -0,0 +1,156 @@ +# 002 — plan audit, and what it changed + +Auditor: independent read-only `xai/grok-4.6` lane, dispatched at the A gate with +the five planning docs and instructions to verify every citation against the tree +rather than trust the plan. Verdict: **GO-WITH-FIXES**, four blockers, nine nits. + +The audit was worth its cost immediately: **blocker 1 disproved a technical claim +the plan had asserted as fact.** Recording that first, because it is the finding +that changes the implementation rather than the prose. + +## BLOCKER 1 (accepted, plan was wrong) — parse CAN name the edge + +010 claimed the parse function "does not know the requested phase edge", and +proposed placeholders `{"from":"","to":""}`. + +That is false, and it was checkable in one read: + +``` +orchestrate-cli.ts:200 const verb = VERBS[verbTok]; // argv[0] -> this IS `to` +orchestrate-cli.ts:227 if (!coerced) attestError = "attest JSON missing valid from/to"; +``` + +`verb` is resolved 27 lines BEFORE the attest loop runs. And the consumer side +already reads state on exactly this path: + +``` +orchestrate-cli.ts:345-348 + if (args.attestError && verb !== "status" && verb !== "reset") { + const sessionIdForError = args.session && sessionFileExists(...) ? args.session : null; + const context = sessionIdForError ? renderPhaseContext(readState(...), ...) : ""; +``` + +So `from` is available whenever `--session` names a real session file — which is +mandatory for every mutating verb anyway. + +**Amendment to 010 §5.** The message is built in `runOrchestrateCli`, not left as +a static string in the parser, and it is CONCRETE: + +- `to` is always the verb. +- `from` is the current phase when the session resolves; only then does a + placeholder appear. +- The forward-declared extra keys are the ones for THAT verb, not a menu of all + of them: `A` adds `planUnit`; `B` adds `auditOutput` + `auditVerdict`; `D` + adds `checkOutput` + `exitCode`; a bound goalplan adds `workPhaseId`, and a + bound `D` also adds `testReceiptPath`. + +A generic menu would be the same mistake in a longer form: the agent still has to +guess which half applies. + +## BLOCKER 2 (accepted) — the tests as written could not fail + +010 tests 1–3 asserted the message "contains `from` and `to`". The CURRENT +message is `attest JSON missing valid from/to`. It contains both. The tests would +have passed against the bug they exist to prevent. + +**Amendment to 010 TESTS.** Each assertion must pin a substring that does not +exist in the tree today. Minimum set per test: + +- the literal example fragment `"did":"` (today's message has no example) +- the recovery command `cxc orchestrate status` +- the verb-specific extra key (`planUnit` for `A`, `checkOutput` for `D`) + +And the `--attest-file` test asserts its own wording, since `:257` emits +`attest file is missing valid from/to` — a DIFFERENT string from `:227`, +which 010 had conflated (nit 2). Two paths, two assertions. + +## BLOCKER 3 (accepted) — the scope contradicted itself + +000 put `cxc freeze --help` OUT; 010 §7 put it IN. Both cannot be true. + +**Resolution: IN, and 000 is amended.** Reasons, in order: it is a MUTATION +triggered by a read-only-looking flag, it exits 0 so nothing signals it, and the +fix is a guard before IO in the same file family wp1 already opens. Leaving a +known workspace-mutating `--help` in place for a later unit, having just written +a document that names it, is the kind of deferral that never comes back. + +The rest of §7 (`review-round`, `plan`, `metric`, `divergence` help, and the +missing top-level verbs) stays IN for the same reason the auditor questioned it: +it is not the attest cascade, but it IS the same defect family — a surface that +refuses an agent following the docs — and it shares `help-verbs.test.ts`. That +file is already the home of the half-finished #47 fix. Finishing it there is +cheaper than a second unit that reopens the same test. + +## BLOCKER 4 (accepted) — CLI help is not the gold source + +010 §1 said the skill examples should "match `orchestrate-cli.ts:166-168`, which +is already correct". The auditor checked. It is not: there is no B→C example at +all, and the C→D example omits `testReceiptPath`. + +**Amendment to 010.** CLI help is a REPAIR TARGET in wp1, not the reference. Add +the missing B→C object and `testReceiptPath` to the C→D object, then the skill +table and help agree because both were fixed — not because one copied the other. + +This one matters beyond the typo: the plan was about to propagate an incomplete +example into the skill and call the result consistent. + +## Nits, dispositions + +| # | Finding | Disposition | +|---|---|---| +| 1 | `check-gate.ts:34` is the wrapper; the string is at `:37` | **Corrected** in 000 §"The defect" (001 never carried the wrong line) | +| 2 | `:227` and `:257` are different strings | **Folded into blocker 2** | +| 3 | "Exact wording is settled in B" — there is no B | **Corrected**: wording is settled HERE, above | +| 4 | docs-site quickstart neither IN nor OUT | **OUT, explicitly.** It is a live copy-paste surface and should be fixed, but it is a docs-site build with its own review; recorded as a follow-up rather than smuggled in | +| 5 | Freeze-train rules belong in a new §2.8, not §2.7 | **Accepted.** §2.7 is a 3-line published-artifact contract; the GO-report rules are a different shelf | +| 6 | Draft the `skill-ownership.md` row; name `N` for FLAKE-STABILITY | **Accepted.** Row drafted in 030; `N` is "declared in the GO report, minimum 3" — opencodex's own number, not invented | +| 7 | Test 6 must bind to the attest table rows, not a repo-wide `rg` | **Accepted.** A 37k-character file will contain any key name somewhere | +| 8 | Accept-criteria paths need the `plugins/codexclaw/` prefix | **Accepted** | +| 9 | `scan-cli.ts:146` teaches `{"override":true,...}` without from/to | **IN.** Same cascade, one line, and it is a runtime error handing the agent a command | + +## Citation corrections to 001 + +The auditor found four sloppy citations. All are in §G (the suite-health section) +or §B, none load-bearing for the diagnosis, and all are corrected here rather +than left to rot: + +- `orchestrate-cli.ts:257` emits `attest file is missing valid from/to`, + not the `:227` wording. +- The `testReceiptPath` refusal is `check-gate.ts:37`, not `:34`. +- `session-split.test.ts:70` uses `Date.now()` to mint a directory name, not as + an mtime dependency. It is NOT timing-sensitive; remove it from the watch list. +- The concurrent-spawn flake comment is near `orchestrate-cli.ts:486` in the test + file, not within `:498-525`. + +## What the audit did NOT change + +- The core diagnosis. Every load-bearing citation in §A and §B was verified + correct: the table at `pabcd/SKILL.md:91-98`, the `:227` wording, the + `evidence`-instead-of-`did` bug at `hook.ts:1163`, the invalid-JSON doctrine + line, the `:218` vs `:77` flaky contradiction, and the `ci-pipeline.md:96-102` + duplication. +- The dist/build note: `plugins/codexclaw/test/dist-freshness.test.mjs` exists, + compares compiled src against tracked dist, and pabcd-state's `dist/*.js` IS + tracked despite the root `.gitignore` entry. `npm run build` stays in wp1. +- The 11 DEVOPS-* rules: no overlap with the three existing ids. +- The four TEST-FLAKE-* rules: no contradiction with `TEST-ANTI-FLAKE-01` or + `TEST-CI-GREEN-01`, provided quarantine stays an exception-with-cost. + +## One weakening the audit forced on the premise + +001 §A implied agents have nowhere to learn the attest shape. For `from`/`to` +specifically that overstates it: CLI help, the Stop-hook examples, +`interview/SKILL.md:180-181`, `scan-cli.ts`, and the docs-site quickstart all +show them. The honest claim is narrower and still sufficient: **the skill the +agent is instructed to load is wrong, and it is the surface most likely to be +copied.** For `planUnit`, `workPhaseId` and `testReceiptPath` the original claim +stands unweakened — zero skill mentions, anywhere. + +## Verdict recorded + +``` +VERDICT: GO-WITH-FIXES +``` + +Four blockers, all folded into the plan above rather than rebutted. Nine nits, +seven accepted, one scoped OUT with a reason (nit 4), one folded (nit 2). diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/010_wp1_attest_contract_and_error_shape.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/010_wp1_attest_contract_and_error_shape.md new file mode 100644 index 00000000..ab056ecd --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/010_wp1_attest_contract_and_error_shape.md @@ -0,0 +1,200 @@ +# 010 — wp1: make the attest contract say what the gate enforces + +Phase: wp1. Depends on: wp0. Blocks: nothing (wp2/wp3 are independent text work). + +## Problem + +`skills/pabcd/SKILL.md:91-98` is the table agents copy. It omits `from`, `to`, +`planUnit`, `workPhaseId`, and `testReceiptPath`. The runtime requires all of +them on the matching edges. See 001 §A/§B for the full divergence and the +three-round-trip cascade it produces. + +Two halves, and both are needed: + +- **Documentation** stops producing the malformed attest in the first place. +- **The error message** rescues the agent that produced one anyway — from a stale + skill copy, another repo's docs, or its own memory. + +Fixing only the docs leaves `attest JSON missing valid from/to` teaching nothing +for the next year. Fixing only the message means every loop pays one round trip +forever. + +## MODIFY map + +### 1. `plugins/codexclaw/skills/pabcd/SKILL.md` — the table (:91-98) + +Replace the "Required attest keys" table so every gated row carries the full key +set, and add a copy-paste object per edge underneath. The entry rows must keep +saying "none" for the ATTEST, but must stop implying an attest passed on an entry +edge is free-form: `coerceAttest` runs on IDLE→P too, so +`--attest '{"did":"x"}'` fails there as well even though the edge is ungated. + +New shape: + +| Edge | Required attest keys | +|---|---| +| IDLE→P, I→P | none — pass no `--attest` at all. If you DO pass one it still parses, so it still needs `from`/`to`. | +| P→A | `from`, `to`, `did`, `planUnit`; `workPhaseId` when a goalplan is bound | +| A→B | `from`, `to`, `did`, `auditOutput`, `auditVerdict`; `auditResidual` when near-pass; `workPhaseId` when bound | +| B→C | `from`, `to`, `did`; `workPhaseId` when bound | +| C→D | `from`, `to`, `did`, `checkOutput`, `exitCode` (must be 0); `testReceiptPath` when bound; `workPhaseId` when bound | + +Plus one fenced block with four ready objects — the exact strings an agent should +be able to paste and edit. + +**Amended after audit (002 blocker 4).** An earlier draft said these must match +`orchestrate-cli.ts:166-168` "which is already correct". It is NOT: there is no +B→C example there at all, and its C→D example omits `testReceiptPath`. CLI help +is a REPAIR TARGET in this phase, not the reference. Fix both surfaces, then they +agree because both are right — not because one copied the other's gap. + +A one-line rule id so the requirement is citable: +**ATTEST-SHAPE-01 (STRICT):** every `--attest` object carries `from` and `to` +naming the edge being advanced, even on ungated edges, because the parser +coerces before the gate runs. + +### 2. `skills/interview/SKILL.md:64` and `:144` + +`{"override":true,...}` and `{"override":true}` are valid JSON, so they do NOT +fail as "not valid JSON" — they fail as missing from/to, which is the confusing +case. Replace both with the already-correct object from `:180-181`: +`{"from":"I","to":"P","did":"","override":true}`. + +### 3. `skills/loop/SKILL.md` + +Its attest references (:27-32, :101) carry no example object. Add a pointer to +the pabcd table rather than a second copy — `cxc-pabcd` owns the contract +(existing precedent: loop already defers phase semantics to pabcd). + +### 4. `structure/20_pabcd_dispatch_doctrine.md:72` + +`--attest '{"from","to","did"}'` is not valid JSON. Anyone pattern-matching it +writes a JSON array of strings or something worse. Make it a real object. + +### 5. `components/pabcd-state/src/orchestrate-cli.ts` — the null-coerce paths + +`:227` (inline) emits `attest JSON missing valid from/to`; `:257` (file) emits +`attest file is missing valid from/to`. Two different strings, both with +no example, no phase, no next command. + +**Amended after audit (002 blocker 1).** An earlier draft claimed the parser +cannot know the edge and proposed `""`/`""` placeholders. That +was wrong and the auditor disproved it: `const verb = VERBS[verbTok]` resolves at +`orchestrate-cli.ts:200`, twenty-seven lines BEFORE the attest loop, and +`runOrchestrateCli:345-348` already calls `readState` + `renderPhaseContext` on +this exact error path. So the message is built where the facts are, and it is +concrete: + +- `to` is ALWAYS the verb. No placeholder. +- `from` is the current phase whenever the session resolves — which is mandatory + for every mutating verb anyway. A placeholder appears only when it genuinely + cannot be known. +- Forward-declare the extra keys FOR THAT VERB, not a menu: `A` adds + `planUnit`; `B` adds `auditOutput` + `auditVerdict`; `D` adds `checkOutput` + + `exitCode`; a bound goalplan adds `workPhaseId`, and a bound `D` also adds + `testReceiptPath`. +- Name `cxc orchestrate status --session ` for the case where `from` is + unavailable. + +A generic menu of every possible key is the same failure in longer form: the +agent still has to guess which half applies to its edge. + +`orchestrate-grammar.ts:88` (chat surface) gets the same text. Note the chat +path currently discards `attestError` entirely (`hook.ts:773`) and the human +free-pass advances anyway — a separate finding, NOT fixed here, because changing +it would make the human surface stricter, which nobody asked for. + +### 6. `components/pabcd-state/src/hook.ts` — the injected examples + +- `:471-478,493` `loopArmDirective`: no example object at all. Add the P→A + object, both platform branches, so the prompt-time directive teaches the shape. +- `:1036-1040` `STOP_NEXT_COMMAND`: examples carry from/to but omit + `workPhaseId` (all) and `testReceiptPath` (C→D). The Stop hook knows whether a + goalplan is bound, so the emitted example should include the active work-phase + id when there is one. +- `:1163-1164` `buildGoalIdleBlock`: emits `evidence` where the schema says + `did`. Rename. This one advances today because IDLE→P is ungated, so it fails + SILENTLY rather than loudly — the worst of the three. + +### 7. `bin/codexclaw.mjs:249-286` + the `--help` verbs + +Folded in because it is the same defect family and the same test file. Top-level +help omits `receipt`, `review-round`, `scan`, `release` while instructing agents +to run ` --help`; and `review-round`/`plan` reject `--help` as an unknown +verb (`review-round-cli.ts:99`, `plan-cli.ts:75`). Add the `help|--help|-h` +branch that loop/receipt/scan already have, and list the missing verbs. + +`cxc freeze --help` writing `.codexclaw/interview/freeze.json` is the same +family but a mutation, not a message: handle `--help` before any IO. +`metric`/`divergence` demanding `--session` before printing usage is the same +one-line fix. + +## TESTS + +In `components/pabcd-state/test/`: + +**Amended after audit (002 blocker 2).** The first draft asserted the message +"contains `from` and `to`". Today's message is `attest JSON missing valid +from/to` — it contains both. Those tests would have passed against the very bug +they exist to prevent. Every assertion below pins a substring that does NOT +exist in the tree today. + +1. `orchestrate-cli`: an attest lacking `from`/`to` on verb `A` produces a + message containing the literal fragment `"did":"`, the string + `cxc orchestrate status`, and `planUnit` (the verb-specific key). None of the + three appears in the current output. +2. The same for `--attest-file`, asserting ITS wording (`attest file is + missing valid from/to`), since `:257` is a separate code path with a separate + literal that a single-path test leaves uncovered. It matters more than usual: + it is the REQUIRED path on Windows. +3. `orchestrate-grammar`: same assertion on the chat parser. +4. `hook`: `buildGoalIdleBlock` emits `"did"` and not `"evidence"`; + `loopArmDirective` contains a from/to-bearing object on both platform + branches (platform is injected — this repo's convention, `atomic-write.test.ts:4`). +5. `help-verbs.test.ts`: extend to `review-round`, `plan`, `metric`, + `divergence`, `freeze`. The freeze case asserts exit 0, output contains + `Usage`, AND that `.codexclaw/interview/freeze.json` was not created — the + mutation is the actual bug, so the assertion must touch the filesystem. +6. A doc-truth test bound to the TABLE ROWS, not the file: parse the attest table + out of `skills/pabcd/SKILL.md` and assert each gated row names the keys that + edge's gate requires. A file-wide `rg` would pass on any incidental mention + somewhere in a 37k-character document (002 nit 7). The repo already has this + genre — `shipped skill catalog exactly matches on-disk SKILL.md folders` — so + drift detection is house style, not an invention. + +Test 6 is the one that stops this regressing. Everything else fixes today's text; +test 6 reddens the build when the next contract change forgets the skill again. + +## Accept criteria + +| # | Criterion | Proof | +|---|---|---| +| 1 | Every gated row names `from`/`to` + its edge-specific keys | table diff | +| 2 | `rg -e workPhaseId -e planUnit -e testReceiptPath plugins/codexclaw/skills/pabcd/SKILL.md` has hits | command output | +| 3 | The null-coerce refusal names the target verb and shows a correct example | live CLI output, BOTH flag forms (different literals) | +| 4 | No injected example uses `evidence` as an attest key | `rg` + test | +| 5 | `cxc orchestrate --help` carries a B→C object and a C→D object with `testReceiptPath` | help output | +| 6 | `--help` exits 0 with usage on every shipped verb, and `cxc freeze --help` creates no `.codexclaw/interview/freeze.json` | test output + `ls` | +| 7 | `scan-cli.ts:146`'s override example carries `from`/`to` | `rg` | +| 8 | `npm test` green after `npm run build` | receipt | + +## Scope boundary + +IN: the files above, plus `scan-cli.ts`'s override example (002 nit 9 — same +cascade, one line, and it is a runtime error handing the agent a command). + +OUT: making the chat surface honor `attestError` (`hook.ts:773` drops it and the +human free-pass advances anyway; changing that makes the HUMAN surface stricter, +which nobody asked for); the interview-readiness dead end (001 §F.1); `--slug` on +loop steer/add-*; `fsm.ts:43`'s illegal-transition message; the docs-site +quickstart (002 nit 4). + +Note on the grammar test: it pins the chat PARSER's error field only. Because +`hook.ts:773` discards `attestError`, that test does not protect chat UX, and it +is labeled as such so a later reader does not mistake it for coverage it lacks. + +## Note on dist + +`test/dist-freshness.test.mjs` fails on src/dist drift and `dist/` is committed +(260822 unit, §7). Editing `src/` alone reddens the suite. `npm run build` is +part of this phase, not an afterthought. diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/020_wp2_devops_release_train_rules.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/020_wp2_devops_release_train_rules.md new file mode 100644 index 00000000..5ac29a9a --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/020_wp2_devops_release_train_rules.md @@ -0,0 +1,126 @@ +# 020 — wp2: the freeze-train lessons, as rules dev-devops can cite + +Phase: wp2. Depends on: wp0. Independent of wp1 and wp3 (disjoint files). + +## Problem + +opencodex ran a release train (v2.32.1) and an operator-visibility train back to +back, and both produced failures whose common shape is: **a gate that could not +be satisfied honestly got satisfied narratively.** The first GO report argued a +red suite into an exception. A mandatory gate had no implementing phase. A live +process from the original bug report was measured as if it were the candidate +build. + +`cxc-dev-devops` today has three rule ids: `DEVOPS-AUTH-01`, +`DEVOPS-RELEASE-PROOF-01`, `DEVOPS-AGENT-SAFETY-01`. None of these lessons is +stated. `DEVOPS-RELEASE-PROOF-01` is the nearest neighbour and is genuinely +different: it governs the proof bundle for a PUBLISHED artifact (digest, builder +identity, deploy target, smoke, rollback). It says nothing about a readiness +REPORT, nothing about which command counts as "the suite", and nothing about +rewriting a gate after it fails. + +## MODIFY map + +Placement follows the existing router/reference split: STRICT release-gate rules +go in `SKILL.md` §2.7 next to `DEVOPS-RELEASE-PROOF-01`; the operational +mechanics go in the reference file that already owns that surface. + +### `skills/dev-devops/SKILL.md` §2.7 (Release Proof Contract) + +| id | severity | statement | source | +|---|---|---|---| +| `DEVOPS-FREEZE-SHA-01` | STRICT | Pin a readiness/GO report to the code SHA its gates describe. If the report head moved, prove the delta is docs-only with `git diff --name-only ` and keep every gate receipt on the freeze SHA. | `900_go_nogo_readiness_report.md:3-7`; the follow-up commit `bb89eafbe` exists precisely because the first version did not do this | +| `DEVOPS-GATE-WEAKEN-01` | STRICT | A red named gate is never excused inside the report that gate failed. Make the original command green, or replace it with a pre-declared equivalent CI actually runs, and declare the swap before the verdict. | `900:47-49`; the `02c302a54` freeze was audit-rejected on this count | +| `DEVOPS-REVIEW-THREADS-01` | STRICT | Unresolved review threads on merged PRs are a GO blocker. Count them after merge, not at merge time. | `900:40-46`; pre-declared at `080_wp8:47` | +| `DEVOPS-GATE-OWNER-01` | STRICT | A mandatory GO gate needs an implementing work-phase and a recorded terminal outcome (pass / not-reproduced / explicitly deregistered). | `090_wp9:6-8`, `000_baseline_scope_and_roadmap.md:243-245` | + +`DEVOPS-GATE-OWNER-01`'s source line is worth quoting in the skill verbatim +because it is the whole rule in seven words: "a gate nobody implements is not a +gate." + +### `skills/dev-devops/references/ci-cd-deploy.md` + +| id | severity | statement | source | +|---|---|---|---| +| `DEVOPS-SUITE-PARTITION-01` | STRICT | A local one-process full-suite run is not the CI suite gate. Replay CI's real partition — general shards plus each segregated job's exact command — and record both forms. | `900:54-58`; `run-bun-test-batches.sh:50` excludes three load-sensitive files that `ci.yml:301-338,340-370` runs as their own jobs | +| `DEVOPS-BASELINE-DEFECT-01` | STRICT | A local red test is a candidate defect until ALL THREE hold: identical failure on the untouched baseline SHA, no merged unit touching that code, and CI's matching job green at the freeze SHA. Two out of three is not evidence. | `900:69-72`, `010_wp1:179-181` | +| `DEVOPS-VERIFY-INSTRUMENT-01` | STRICT | Do not change the verification instrument (runner flags, parallelism, shard layout, timeouts, retry policy) while using it to certify a freeze. Change it against a known-good baseline, or defer it. | `070_wp2:44-45,117-121` | +| `DEVOPS-EXACT-HEAD-01` | STRICT | Re-read the PR/branch head immediately before claiming exact-head evidence. A remembered pass is not evidence; a contributor push mid-verification makes the recorded SHA stale. | `070_wp2:136-139,87-98`; `260825_operator_visibility_train/000:63-65` | +| `DEVOPS-FLAKE-STABILITY-01` | DEFAULT | A flaky-capable suite is stable only after N consecutive greens at ONE head plus the required matrix. One green run is not a land signal. | `070_wp2:94-98,125-126` (opencodex used three) | + +`DEVOPS-VERIFY-INSTRUMENT-01` and `DEVOPS-FLAKE-STABILITY-01` stay separate ids +on purpose: one says do not swap the instrument, the other says how many greens +count. Merging them would lose whichever half the citing text did not need. + +`DEVOPS-FLAKE-STABILITY-01` must POINT at the wp3 canonical flaky policy rather +than restate remediation. It is a release-gate counting rule, not a remediation +rule. + +### `skills/dev-devops/references/sre-foundations.md` + +| id | severity | statement | source | +|---|---|---|---| +| `DEVOPS-STALE-PROCESS-01` | STRICT | A live long-running process is not candidate evidence until its start time, binary, and config are proven to match the build under test. | `090_wp9:21-23` — a 100-call canary was run against PID 922, started two days earlier: the reporter's own pre-fix proxy | +| `DEVOPS-OBS-SIGNAL-01` | DEFAULT | When an operator surface is missing a signal, ADD the missing signal; never flip an already-true status bit to compensate. A degraded or ineligible verdict that reaches an operator command must carry a message. | `260825_operator_visibility_train/020_wp3:16-22`; the silent-ineligible half at `001:97-118` and `030:75-87` | + +`DEVOPS-STALE-PROCESS-01` has a local precedent worth cross-referencing: this +repo's own memory records proving process-start-time against dist mtime during a +cli-jaw deployment. Same rule, independently learned. + +## Explicitly NOT added + +Read and rejected as devops rules because they belong elsewhere: the sidecar +backend-resolution ternary, the two-null-policy warning, version-manager shim +adoption, "reproduce from the reported observable, not from a mapper you chose to +call" (that is testing), and `--ff-only` versus rebase for an unpushed commit +(that is git-train). Recorded in 001 §D so the next pass does not re-derive them. + +## Accept criteria + +| # | Criterion | Proof | +|---|---|---| +| 1 | Each of the 11 rules exists once, with an id, a severity, and a devlog citation | `rg -e 'DEVOPS-'` across dev-devops | +| 2 | No rule duplicates `DEVOPS-RELEASE-PROOF-01`, `DEVOPS-AUTH-01`, or `DEVOPS-AGENT-SAFETY-01` | side-by-side read | +| 3 | Reference-file rules are not re-stated in SKILL.md (pointer only) | `rg` count per id is 1 outside its owner file | +| 4 | Every citation resolves to a real line in the opencodex devlog | spot-check 3 at random | + +## Scope boundary + +IN: the three dev-devops files. +OUT: the opencodex repo itself; `platform-engineering.md` and +`package-release.md` unless a rule genuinely lands there; any attempt to +backport these rules into cli-jaw or ima2-gen. + +## Delivered, and what the audit changed + +Landed in `3d5d0d34`, corrected in `836954db`. Placement moved from the planned +§2.7 to a **new §2.8**: §2.7 is a three-line published-artifact contract, and +GO/NO-GO decision rules are a different shelf. The auditor agreed this was the +right call rather than the deviation it looked like. + +**Three claims in the first version were invented.** Not misread sources — +fabricated supporting detail wrapped around real lessons. An independent lane +checked each against the opencodex devlogs and the live CI scripts: + +| Claimed | Actually | +|---|---| +| A train "ran a 100-call canary" whose numbers described the bug | The canary was a GO criterion, REJECTED as the wrong instrument before it ran, once PID 922 was identified as the reporter's pre-fix proxy (`090_wp9`:17, `080_wp8`:24). The measurement never happened. | +| "run 1 green, run 2 failed, run 3 produced the finding" | The recorded table is green, green, fail (`070`:87-98). | +| "OpenCodex used three" consecutive greens | Three was the bar they SET and never collected — which is why the PR was deferred (`070`:125-126). | +| "three load-sensitive files, each in its own job" | Three path PATTERNS covering seven files, run by TWO jobs: `storage policy` takes six together, `api usage` takes one (`run-bun-test-batches.sh`:50, `ci.yml`:304). | + +The last one is the instructive failure. That loose wording came from the source +report itself; copying a report's paraphrase into a STRICT rule is what made it +load-bearing. The corrected text cites the shell script and the workflow +directly, not the report's summary of them. + +The rules themselves survived unchanged. Only my evidence for them did not — and +a rule whose cited evidence is wrong is a rule the next reader is right to +distrust. + +Also corrected: the router pointed at `ci-cd-deploy.md` §9 and +`sre-foundations.md` §6, neither of which exists (the rules are at §6 and §7), +so an agent following the pointer would have landed in Anti-Patterns. Per-rule +line citations attached, the cli-jaw start-time-vs-dist-mtime precedent +cross-referenced, `/health` corrected to `/healthz`, and four AI-tell sentences +removed. diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/030_wp3_flaky_policy_elimination_first.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/030_wp3_flaky_policy_elimination_first.md new file mode 100644 index 00000000..8c7af5df --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/030_wp3_flaky_policy_elimination_first.md @@ -0,0 +1,183 @@ +# 030 — wp3: one flaky policy, elimination-first, one owner + +Phase: wp3. Depends on: wp0. Independent of wp1/wp2 (disjoint files), except that +wp2's `DEVOPS-FLAKE-STABILITY-01` points here. + +## Problem + +The repo already believes the right thing in seven places and the wrong thing in +three. 001 §E has the full inventory; the summary is that +`dev-testing/SKILL.md:218` says + +> Protocol: detect → quarantine if blocking → assign owner → reinstate after +> repeated green runs. + +while `dev-testing/SKILL.md:77` says a flake is a bug and green-on-retry is not +acceptable, and `:220-224` says never blind-retry a failed job. An agent cannot +follow both. `ci-pipeline.md:96-102` then repeats the quarantine protocol as its +own §5 heading — "Flaky Test Quarantine Strategy" — so the deep reference and the +router both claim to own it, with different strength. + +Meanwhile `dev-debugging:78` lists "add retry/skip annotation" as the WRONG +patch, and `dev-testing:491` treats `.skip()` on a failing test as an escalation +red flag. A quarantine tag is a skip with a nicer name. Six contradictions, C1–C6 +in 001 §E. + +## The policy + +Elimination is the only resolution. Quarantine is an exception that must cost +something to take, and blind re-run is banned outright. + +Proposed rule ids, in the existing `TEST-*` family: + +**`TEST-FLAKE-ELIMINATE-01` (STRICT)** — A flaky test is a defect in the test or +the code under test. Diagnose the nondeterminism and remove it: replace timing +assumptions with deterministic waits or a fake clock, reset shared state in +fixtures, remove live network dependencies, pin fonts/time/locale for snapshots. +A flake is closed when the cause is named, not when the suite is green. + +**`TEST-FLAKE-RERUN-01` (STRICT)** — Re-running a failed job or test to obtain +green is not a resolution and is never recorded as one. A re-run is permitted +only as a diagnostic to measure failure RATE, and the measurement is written down. +Raising a timeout to make a test pass is the same violation wearing a config +change. (Merges the intent already present at `dev-testing:77,220-224,380` and +`dev-debugging:74,78`.) + +**`TEST-FLAKE-QUARANTINE-01` (DEFAULT, exception path)** — Quarantine is allowed +only when the flake blocks unrelated delivery AND all four are recorded in the +same change: the exact test name, the named owner, a removal deadline, and the +suspected cause. A quarantine without a deadline is a deletion. Quarantine never +closes the defect; it defers it, and the deadline is the receipt. + +**`TEST-FLAKE-ATTRIBUTION-01` (DEFAULT)** — Before calling a failure +environmental, prove it: identical failure on the untouched baseline, no change +touching that code, and the matching CI job green at the same SHA. This is the +test-side mirror of `DEVOPS-BASELINE-DEFECT-01` and exists because "it's flaky" +is the most common way a real defect gets waved through. + +That last rule is the one this repo did not have. Every existing line says do not +HIDE a flake; none says how to PROVE something is not your defect. Without it, +"environmental" is an assertion. + +## Ownership + +Follow the `DEV-STACK-*` precedent exactly, since +`skills/dev/references/skill-ownership.md:3` already states the doctrine: "Each +rule area has exactly one canonical owner. Other skills may contain stubs but +MUST NOT duplicate canonical content." + +| File | Role after this phase | +|---|---| +| `dev-testing/references/ci-pipeline.md` §5 | **CANONICAL.** Rewritten as "Flaky Test Policy (canonical — `TEST-FLAKE-*`)". Full rules, the first-fix table, and the quarantine exception form. | +| `dev-testing/SKILL.md` §5.4 | Pointer stub: rule ids + one line each + path. Delete the quarantine protocol sentence at :218 and stop duplicating the first-fix table. | +| `dev-testing/SKILL.md` §1.5 (:77) | `TEST-ANTI-FLAKE-01` stays as the one-line STRICT rule, with a pointer. It is already correct; do not restate the protocol beneath it. | +| `dev-debugging/SKILL.md` Scenario D + row :78 | Keep the RCA method (shared mutable state, isolation, hunt siblings). Add "policy: `dev-testing` `references/ci-pipeline.md` §5". No quarantine or retry rules here. | +| `dev-devops/SKILL.md` §6 | Already a pointer to `dev-testing` §5. Leave it. `DEVOPS-FLAKE-STABILITY-01` (wp2) cites the canonical file rather than restating. | +| `dev/references/skill-ownership.md` | **Add the missing row.** Its absence is why the policy drifted into two files. | +| `structure/30_contradiction_register.md:85` | C10 lists "candidate for an explicit timeout" for a flake. Update the disposition to name the new rule; do not silently delete a register entry. | + +Choosing `ci-pipeline.md` over a new `flaky-tests.md`: the router already points +at it for the full §5 template (`dev-testing/SKILL.md:204-205`), which is the +same shape as `dev` §5 → `stacked-prs.md`. A new sibling file would also be +defensible; rewriting the section that already holds the only multi-step protocol +is the smaller ownership move and leaves no orphan heading. + +## What this policy must NOT become + +A rule that makes an honest agent lie. If a flake genuinely blocks delivery and +cannot be root-caused in the current cycle, the exception path must exist and be +usable — otherwise the pressure that produced "quarantine if blocking" simply +reappears as an undocumented skip. The four required fields are the cost; +forbidding the exception entirely would be theater. + +This is also why `TEST-FLAKE-ATTRIBUTION-01` is DEFAULT and not STRICT: proving +the negative sometimes requires CI access an agent does not have. It must be a +recorded gap, not a blocked turn. + +## Note on this repo's own suite + +There is no flaky test here to eliminate. `npm test` is 1961/1961 green and the +pabcd-state slice ran twice with identical results (001 §G). The policy is +written for the repos this skill governs, and the contention-sensitive tests +listed in 001 §G are the local watch list, not a defect list. Do not "fix" a +green test to demonstrate the policy. + +## Accept criteria + +| # | Criterion | Proof | +|---|---|---| +| 1 | `ci-pipeline.md` §5 is the single canonical policy, carrying all four `TEST-FLAKE-*` rules | file diff | +| 2 | `dev-testing/SKILL.md:218` no longer prescribes quarantine as the protocol | `rg -n -e quarantine skills/dev-testing/SKILL.md` | +| 3 | No surviving text in dev-testing, dev-debugging, or dev-devops recommends re-run or timeout-raise as a resolution | `rg -i` across the three, read in context | +| 4 | `skill-ownership.md` has the flaky row in DEV-STACK format | diff | +| 5 | C1–C6 from 001 §E are each individually resolved | one line per contradiction in the closeout | +| 6 | `structure/30_contradiction_register.md:85` disposition updated, not deleted | diff | + +## Scope boundary + +IN: the seven files above. +OUT: editing any test file; changing `--test-concurrency=1`; touching +`dev-data`'s ETL quarantine (a different meaning of the word); `qa`/`loop`/ +`pabcd` retry language, which is repair-loop discipline, not CI flakes. + +## Delivered, and what the audit changed + +Landed in `6cf7b698`, corrected in `6268501b`. The audit returned +GO-WITH-FIXES with **zero blockers** — C1–C6 were verified closed in the tree +rather than relocated, and all four required cross-references resolved. The eight +nits split into two categories. + +### Two were severity-class holes, not prose + +Both are worth recording because they are the failure mode a policy rewrite is +most prone to: the words were right and the classification let them be ignored. + +**`QUARANTINE-01` was DEFAULT end to end.** Under `dev` §0.2 a DEFAULT rule can +be waived with a stated reason — which meant the four required fields, the entire +receipt, were optional. The permission to defer is legitimately DEFAULT; the +receipt is not. Split: deferring stays DEFAULT, the four fields are now STRICT. + +Related: "blocks unrelated delivery" was unfalsifiable, because any red CI can be +called blocking. It now requires naming which delivery and why it is independent +of the code under test. + +**`ATTRIBUTION-01` (DEFAULT) mirrors `DEVOPS-BASELINE-DEFECT-01` (STRICT).** +Same triple, two classes. An agent who loaded only `dev-testing` could state "no +CI access", waive the DEFAULT rule, and proceed without the freeze-SHA check the +STRICT rule demands — reaching the weaker class by choosing which skill to read. +The text now states that a recorded gap is not a waiver and that the STRICT rule +governs when both apply. + +### One was a self-inflicted repeat of the bug being fixed + +The first router stub restated all four rules' testable payloads. That is +precisely the duplication that produced contradiction C3 — two copies drifting +apart, the router's weaker. 030 asked for "rule ids + one line each", and +following that literally reintroduced the mechanism. The stub is now a real +pointer: it names the rule ids and the canonical path, and stops. + +### The scope boundary was wrong in one direction + +030 scoped test files OUT. The audit found two comments that TEACH what the new +policy forbids: + +- `subagent-config/test/mcp.test.ts` justified a 30s ceiling as absorbing + scheduling jitter — timeout-raising as flake absorption, in the very file C10 + cites. +- `plugins/codexclaw/test/hook-e2e.test.mjs` said "skip rather than flake". + +Neither behavior changed; both comments did. The first is now named as a hang +detector with the unfixed contention tracked as C10 and the honest fixes listed; +the second explains that the skip is a missing-fixture precondition, not flake +avoidance. A comment that teaches a forbidden move is guidance regardless of +which file it lives in, so "no test edits" was the wrong boundary for a policy +whose whole point is that the repo should not contradict itself. + +### Remaining nits, all fixed + +`TEST-ANTI-FLAKE-01` got the pointer this doc promised; the dropped +"passes locally, fails in CI" row is back as a cause; `skill-ownership.md` points +at `ci-cd-deploy.md` §6.2/§6.5 rather than `dev-devops` §6, which never mentions +the family; the `.skip()` red flag names the one quarantine form that is not a +red flag; a `§5.5` cross-reference that resolved to the wrong local section was +qualified; and C10's "~8s" was corrected to match the live 30s. diff --git a/devlog/_plan/260825_attest_fromto_and_devops_lessons/900_closeout.md b/devlog/_plan/260825_attest_fromto_and_devops_lessons/900_closeout.md new file mode 100644 index 00000000..30dd431d --- /dev/null +++ b/devlog/_plan/260825_attest_fromto_and_devops_lessons/900_closeout.md @@ -0,0 +1,127 @@ +# 900 — closeout + +Four work-phases, ten commits, three independent audits. Terminal outcome: +**DONE**. + +Baseline `74245989` (npm test 1961/0) → head `f0e6442c` (npm test 1987/0). +26 net new tests. No behavior was changed in any test to make it pass. + +## What shipped + +| wp | Commits | Delivered | +|----|---------|-----------| +| wp0 | `472beb3f` | Roadmap unit: 000 baseline, 001 inventory, 002 plan audit, 010/020/030 decade docs | +| wp1 | `49d90e64`, `c19e5781`, `db129710` | The attest contract, the runtime error shape, and the `--help` family | +| wp2 | `3d5d0d34`, `836954db`, `3ae38473` | 11 `DEVOPS-*` freeze-gate rules | +| wp3 | `6cf7b698`, `6268501b`, `f0e6442c` | Elimination-first `TEST-FLAKE-*` policy with one canonical owner | + +## The defect that started it + +`attest JSON missing valid from/to` had 50+ occurrences across four repos since +2026-08-13, and the gate was not at fault. `coerceAttest` rejects an attest +without `from`/`to` before any other check, while the table agents are +instructed to copy named neither those keys nor `planUnit`, `workPhaseId`, or +`testReceiptPath`. A goalplan-bound P>A therefore cost three round trips, each +one a turn, all caused by one incomplete table. + +Both halves were needed. The docs stop producing the malformed attest; the error +message rescues the agent that produced one anyway from a stale copy or its own +memory. Fixing only the docs would have left the refusal teaching nothing for +another year. + +## What the audits caught that self-review did not + +This is the part worth keeping. Three lanes, three findings I would have shipped. + +**wp0 — the plan asserted a falsehood about its own codebase.** 010 claimed the +parser cannot know which edge is being advanced, and proposed +`""/""` placeholders. The auditor read +`orchestrate-cli.ts:200` and disproved it in one line: `verb` is `argv[0]`, +resolved 27 lines before the attest loop, and `runOrchestrateCli:345-348` +already reads session state on that exact error path. The shipped message names +the real edge because an auditor checked a claim I had not. + +**wp1 — I introduced the bug I was fixing.** Renaming `buildGoalIdleBlock`'s +`evidence` key to `did` corrupted the template literal's closing backtick into a +backslash, and no existing test covered that function's rendered text. It would +have shipped. Its regression test now asserts balanced backticks and no trailing +backslash — a structural property rather than the wording. + +**wp2 — I fabricated evidence.** Three historical claims in the first +dev-devops draft were invented detail wrapped around real lessons: a 100-call +canary that "produced numbers" (it was rejected as the wrong instrument before it +ran), a green/fail/finding run sequence (the table is green/green/fail), and +"three files each in its own job" (three path patterns, seven files, two jobs). +The rules survived; my evidence for them did not. A rule whose cited evidence is +wrong is a rule the next reader is right to distrust. + +**wp3 — the words were right and the classification let them be ignored.** +`QUARANTINE-01` was DEFAULT end to end, so its four-field receipt — the entire +accountability mechanism — was waivable. And `ATTRIBUTION-01` (DEFAULT) mirrored +`DEVOPS-BASELINE-DEFECT-01` (STRICT) on the same triple, letting an agent reach +the weaker class by choosing which skill to load. + +## Verification + +`npm test` 1987 pass / 0 fail / exit 0, receipt at +`.codexclaw/evidence//test-receipt.json`. + +A green suite is not by itself evidence that the new tests work, so each class +was falsified deliberately: + +| Test | Falsified by | Result | +|------|--------------|--------| +| The hint assertions | Neutering `renderAttestShapeHint` to return `""` | 5 of 13 failed | +| The doc-drift test | Deleting `planUnit` from the P>A table row | 1 failed: `P->A row must name "planUnit"` | + +Both were re-run at wp4 against the final tree, not quoted from wp1. The hint +count grew from 3 to 5 because the Stop-command and arming-directive coverage +added later depends on the same function. + +The drift test's FIRST version passed against injected drift, because it read the +whole table row and the Notes column happened to mention `planUnit`. It was +narrowed to the contract cell and re-verified. A test that has never failed is +not evidence. + +## Deliberately not done + +- **The interview-readiness dead end.** `isInterviewReady` requires all four + dimensions at `max`; `scan record --dim x=max` is rejected and `deriveLevel` + never emits `max`, so an honest I>P is unreachable through the shipped CLI + while `interview/SKILL.md` says scan-record is the path. Every HITL interview + either dead-ends or forges an override. This is a design decision about what + readiness should accept, not a text fix, and it needs its own unit. +- **The chat surface discarding `attestError`.** `hook.ts` drops it and the human + free-pass advances regardless. Fixing that makes the HUMAN path stricter, which + nobody asked for. +- **docs-site attest examples.** A live copy-paste surface, incomplete, but its + own build with its own review. +- **`cxc loop --help`'s `--slug`** on `steer`/`add-*`, which the runtime ignores. +- **The C10 contention itself.** The 30s ceiling is now honestly named as a hang + detector, and the two real fixes — build/test serialization, or removing the + real-process dependency — are recorded rather than done. + +## Not pushed + +Ten local commits. `LOOP-GIT-01`: committing is autonomous, pushing is an +external state change that needs explicit approval. The pre-existing dirty +worktree (`scripts/dev-symlink.sh`, two untracked `devlog/_plan` directories, a +stray `mktemp:` path) was preserved untouched throughout. + +**One correction to that claim, and it happened twice.** `3ae38473` staged with +`git add devlog` and swept in the two untracked plan units — +`260722_repo-governance-config` and `260814_fix-main-ci-windows-worktree` — +which belong to the user's own in-progress work. Caught in the wp4 acceptance +sweep by noticing they had vanished from `git status`, and untracked in +`438ddd31`. + +Then `260e2b49` did it again, because I reached for `git add devlog` a second +time. Untracked again in `d0c2ad0`. The files were never modified on disk in +either case; only their tracked status was. + +Recorded twice rather than quietly repaired, because a mistake that recurs after +being fixed is a habit, not a slip. The fix is mechanical: stage explicit paths, +never a directory that also contains someone else's work. It is also why the +acceptance sweep re-reads `git status` instead of trusting that scope was +respected — the first occurrence was invisible until something compared the +worktree against its baseline. diff --git a/devlog/_plan/260825_interview_readiness_reachable/000_plan.md b/devlog/_plan/260825_interview_readiness_reachable/000_plan.md new file mode 100644 index 00000000..6b9dd829 --- /dev/null +++ b/devlog/_plan/260825_interview_readiness_reachable/000_plan.md @@ -0,0 +1,285 @@ +# 000 — the readiness gate nobody can pass + +`isInterviewReady` demands a level that no production writer can produce. Every +HITL interview therefore ends the same way: dead-end, or an attested override. + +## The defect + +`interview.ts`: + +```ts +export const DIMENSION_LEVELS = ["low", "mid", "high", "max"] as const; + +export function isInterviewReady(tracker) { + for (const d of DIMENSIONS) { + if (!isValidScore(score) || score.level !== "max") return false; // :260 + } + ... + return roundIdNum(tracker.scanRounds) >= 1; // :268 +} +``` + +`scan-cli.ts` is the only production writer of `dimensions[d].level`, and it +cannot write `max` by either route: + +```ts +function deriveLevel(score) { // :273 + if (score.known.length === 0 && score.unknown.length === 0) return "low"; + if (score.unknown.length > 0) return "mid"; + return "high"; // <- the ceiling of --derive +} +``` + +```ts +if (pair.value === "max") { // :143 + return { error: "scan record: --dim cannot set 'max'. ..." }; +} +``` + +So the gate's condition is unreachable through the shipped CLI. The override at +`orchestrate-cli.ts` is the only exit, and it was designed for the opposite +case: + +> the sanctioned way past an **unready** interview + +When the hatch is the only path, its ledger row stops distinguishing anything. +"Bypassed the readiness gate" is written for the thorough interview and the +skipped one alike, which is the same as not recording it. + +## Two honest options, and the one being rejected + +**(b) An attested `max`-assertion path.** Add a command that writes `max` and a +ledger row proving a human asserted it. + +Rejected. It rebuilds the override with extra steps: the agent still asserts +readiness rather than demonstrating it, and we would then own two attested +bypass surfaces instead of one. The existing override already covers "I judge +this ready without the evidence" and covers it honestly. + +**(a) Make readiness reachable from evidence the scan already derives.** CHOSEN, +but not in the naive form. + +`deriveLevel` computes: + +| level | meaning | +|---|---| +| `low` | nothing known about this dimension | +| `mid` | facts recorded, **open gaps remain** | +| `high` | facts recorded, **no open gap** | + +### The naive version of this fix is worse than the bug + +The first draft of this plan said "accept `high`". Before writing it down I ran +the writer to see what `high` costs: + +``` +cxc scan record --session s1 --known goal=x --known constraint=x \ + --known success=x --known ontology=x +-> levels: {"goal":"high","constraint":"high","success":"high","ontology":"high"} + scanRounds: 1 +``` + +Four `--known` flags with the literal value `x`. One command. `--known` is free +text an agent types, and `deriveLevel` only asks whether `unknown` is empty — a +dimension that was never questioned has no gaps to be missing. + +So "accept high" trades a gate nobody can pass for a gate nobody can fail. That +is the trade the scan-cli comment warns about, arriving through a different +door: `--known` is a writer flag with no attestation and no trail, exactly like +the `--dim=max` it forbids. + +### What actually distinguishes a real interview + +`--derive` does not read agent input. It reads the Q&A ledger +(`.codexclaw/interviews/.jsonl`), which is written by +`captureInterviewAnswers` from real `request_user_input` traffic — questions the +user was actually asked and answers the user actually gave. `deriveFromLedger` +then refuses to guess: an unmapped question is skipped entirely, an unanswered +question becomes an open gap that pins the dimension at `mid`. + +That is the property worth gating on. Not the level, which is a number an agent +can write — **the provenance of the level**. + +### The rule + +A dimension satisfies readiness when it is: + +- at `max` — an operator's explicit assertion, unchanged; or +- at `high` **and derived from the answer ledger** — at least one of its + `known` entries traces to a recorded `answer_recorded` event. + +`max` is not deleted and `--known` is not removed. `--known` still records +facts and still moves a dimension off `low`; it just cannot, by itself, carry a +dimension across the readiness line. + +## The change + +### A tracker field cannot carry this, and measuring says so + +The obvious move is a `derived?: boolean` on `DimensionScore`. I probed it +before writing it down: + +``` +writeState(... dimensions.goal = {level:"high", ..., derived:true}) +on disk: {"level":"high","known":["k"],"unknown":[],"confidence":1} +after readState: {"level":"high","known":["k"],"unknown":[],"confidence":1} +``` + +`reconstructScore` (`interview.ts:154`) rebuilds a score from a whitelist of +four fields. An unknown key does not survive the write, let alone the read. So +the field would require widening the fail-closed reconstruct — and the moment +`derived` is reconstructable, a hand-edited session JSON can assert it. That is +the `--dim=max` objection again: a flag with no attestation and no trail, this +time spelled as a field. + +### The ledger is the evidence, so ask the ledger + +`isInterviewReady(tracker)` stays pure and unchanged in signature — every +existing caller keeps working. Readiness gains a second, IO-bearing form used by +the gate: + +```ts +// interview.ts — pure, unchanged contract, now the SHAPE half. +export function isInterviewReady(tracker): boolean + +// scan-cli.ts or a small sibling — the EVIDENCE half. +export function dimensionsBackedByAnswers(cwd, sessionId, map): Set +``` + +`evaluateInterviewGate` — already the single place that decides I->P — becomes +the composition: + +- **shape**: all four dimensions at `max`, or at `high`, plus the untouched + conditions (contradictions empty, assumptions recorded, `scanRounds >= 1`); AND +- **evidence**: every dimension the shape counted as `high` appears in + `dimensionsBackedByAnswers`, computed by re-reading + `.codexclaw/interviews/.jsonl`. + +The attribution map is the missing link: `deriveFromLedger` takes it as a +`--map` argument and then throws it away, so a later reader cannot tell which +dimension a `questionId` belonged to. Persisting it in the tracker would make it +hand-editable, which is the objection this design exists to avoid. + +It goes in the ledger instead. `scan-cli` already appends a `scan_completed` +event per round; `InterviewEvent` gains an optional `map?: Record` carrying the attributions that round used. The file is append-only +and is the same file the answers live in, so the evidence and its interpretation +share one provenance. + +`dimensionsBackedByAnswers` then reads both event kinds from that one file and +returns the dimensions holding a mapped `question_asked` + `answer_recorded` +pair. Nothing is persisted in the tracker, so no state edit can forge it. + +**What this is not.** It is not tamper-proof: the ledger is a file on disk, and +anyone who can write the session state can write the ledger. It removes the +*accidental* path — the one an agent takes because it is easier than asking — +and leaves forgery as a deliberate act. For a soft-gate whose honest bypass is +one attested command away, that is the right bar; a cryptographic one would cost +more than the gate is worth. + +`max` keeps its meaning: an operator assertion that needs no ledger backing, +because it is the level `--dim` cannot write and only a deliberate hand-edit or +a future attested writer can produce. + +### Why not `scanRounds >= N` for N>1 + +Rescan rounds measure how much contradiction-hunting was needed, not how +complete the result is. A first-round interview with no gaps and no +contradictions is complete. Forcing a second round would add a step without adding evidence. + +### What this does not fix + +An agent can still write the ledger by calling `request_user_input` and +answering itself — but it cannot, because `request_user_input` is hard-denied +while a goal is active, and in HITL the answers come from the human. The gate +now costs a real question and a real answer per dimension. That is not +unforgeable; it is expensive enough to stop being the path of least resistance, +which is the honest bar for a soft-gate whose escape hatch is one attested +command away. + +## MODIFY map + +| File | Change | +|---|---| +| `src/interview.ts` | `isInterviewReady` accepts `high` or `max` (SHAPE only); its doc comment currently states the `max` rule as fact. `evaluateInterviewGate` gains `cwd`/`sessionId` and composes shape with evidence, so the gate — not the pure predicate — owns the provenance requirement | +| `src/interview-ledger.ts` | export `dimensionsBackedByAnswers(cwd, sessionId)`: reads the session's Q&A events AND the `scan_completed` maps from the same file, returns the dimensions holding a mapped question_asked + answer_recorded pair | +| `src/state.ts` | `InterviewEvent.map?: Record`, threaded through `appendInterviewEvent`/`readInterviewEvents` the way `ontologySchema` is threaded through the tracker | +| `src/scan-cli.ts` | include `--map` in the `scan_completed` event it already appends; the `--dim=max` rejection names `--derive` as the honest path instead of implying override is the only route; the stale `deriveLevel` comment at `:268` | +| `skills/interview/SKILL.md` | the readiness description currently says `max` gates I->P and points at override; it must describe the path that works | +| `test/interview.test.ts` | the existing "true only for all-max" test encodes the defect as a contract | +| `test/scan-cli.test.ts` | new end-to-end: capture answers -> `scan record --derive` -> `isInterviewReady` true, with NO override | + +Added after the A-round audit, which found the map incomplete: + +| File | Change | +|---|---| +| `src/scan-cli.ts:268` | `deriveLevel`'s comment claims `--dim =max` is the operator assertion. The parser at `:143` rejects that exact flag. The comment has been wrong since the rejection landed | +| `test/interview.test.ts:30` | `"true only for all-max"` asserts `high` is NOT ready — the defect encoded as a contract | +| `test/scan-cli.test.ts:264,337` | both assert the `--dim=max` error names `override.*true`; that message is changing | +| `test/scan-cli.test.ts:426` | already proves `--known goal=...` alone reaches `high`. It stays true and stays green — it is now the SETUP for the new "trivial path is not ready" assertion | +| `test/orchestrate-apply.test.ts:115`, `test/orchestrate-cli.test.ts:619` | the existing override tests. Test 5 asserts behavior these already cover, so it extends them rather than duplicating | + +Fixtures at `orchestrate-cli.test.ts:20`, `orchestrate-apply.test.ts:80`, +`fsm.test.ts:124` and `freeze.test.ts:86` hand-build `level:"max"`. They keep +passing untouched — `max` still satisfies readiness — but they do NOT prove a +derived `high` opens I->P. That is what the new end-to-end is for. + +Runtime callers (`fsm.ts:99`, `state.ts:289`, `freeze-cli.ts:88`, +`evaluateInterviewGate` via `orchestrate-cli.ts:512` and +`orchestrate-apply.ts:128`) read the predicate and never hard-code the level, so +no signature changes. `evaluateInterviewGate`'s warning text stays accurate: an +undermapped dimension is still "incomplete". + +## TESTS + +1. **The end-to-end that proves the fix** — capture real answers through + `captureInterviewAnswers`, run `runScanCli` with `--derive` and `--map`, then + assert `isInterviewReady` is true. This must drive the WRITER, not hand-build + a tracker; a hand-built tracker would prove only that the predicate changed. +2. **A dimension with an open gap still blocks.** `mid` is not enough. +3. **The trivial path still blocks.** Four `--known` flags produce all-`high` + and readiness stays FALSE, because none of them is `derived`. This is the + test that makes the fix meaningful rather than cosmetic — it is the exact + command measured above. +4. **A `--known` fact on a derived dimension does not un-derive it, and does not + by itself derive a different one.** Mixing the two writers is the realistic + case, and the flag must be additive without being promotive. +5. **`max` still satisfies readiness** — demoted from sole path, not removed. +6. **The unchanged conditions still block** at all-derived-`high`: one + contradiction, one unrecorded assumption, `scanRounds === 0`. +7. **The override still works and still ledgers.** Extends the existing tests at + `orchestrate-apply.test.ts:115` / `orchestrate-cli.test.ts:619` rather than + duplicating them. + +## What the A-round audit changed + +The auditor returned **FAIL** on the first draft, and it was right to. That +draft said "accept `high`" full stop. Two of its three blockers are the trivial +`--known` path, which I had independently measured and amended before the +verdict arrived — the amendment above is the auditor's own recommended fix (ii), +reached separately. The third blocker was mine to fix: the MODIFY map omitted +every test and comment that encodes the old rule, which is how a "small" change +turns into a red suite and a surprised author. + +One correction the audit forced on my reasoning, not just the plan: I wrote that +"data shape alone never proves a scan ran". For this writer that is false — +`runScanCli` increments `scanRounds` unconditionally, so the counter proves a +command ran, not that an interview happened. `derived` is what carries the +provenance; `scanRounds` only orders the rounds. +## Accept criteria + +| # | Criterion | Proof | +|---|---|---| +| 1 | A complete interview reaches ready without override | test 1, driving the real writer | +| 2 | **The trivial path does NOT reach ready** — four `--known` flags leave it false | test 3; this is the criterion that makes the fix real rather than cosmetic | +| 3 | Incomplete interviews still blocked | tests 2 and 6 | +| 4 | Mixing `--known` with `--derive` neither un-derives nor falsely derives | test 4 | +| 5 | `max` still satisfies readiness; the override survives and still ledgers | tests 5 and 7 | +| 6 | The skill describes the working path | diff + `rg` for stale `max` guidance | +| 7 | `npm test` green | receipt; baseline 1987/0 | + +## Scope boundary + +IN: every file in the MODIFY map above, plus this unit. +OUT: the interview flow itself, question generation, the auto-resolve loop, +`evaluateInterviewGate`'s warning text beyond what this change makes false. diff --git a/package.json b/package.json index 262fb97b..312508e9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codexclaw", - "version": "0.2.12", + "version": "0.2.13", "private": true, "description": "cli-jaw-style dev discipline + multi-model subagents for the OpenAI Codex runtime.", "type": "module", diff --git a/plugins/codexclaw/.codex-plugin/plugin.json b/plugins/codexclaw/.codex-plugin/plugin.json index b65933fb..bc8e980a 100644 --- a/plugins/codexclaw/.codex-plugin/plugin.json +++ b/plugins/codexclaw/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codexclaw", - "version": "0.2.12+codex.20260818121334", + "version": "0.2.13+codex.260825074116", "description": "cli-jaw-style dev discipline (dev skills + PABCD) and multi-model subagents for the OpenAI Codex runtime, with optional opencodex provider routing.", "author": { "name": "lidge-jun", diff --git a/plugins/codexclaw/components/config-guard/package.json b/plugins/codexclaw/components/config-guard/package.json index 1d88c48b..44680c3b 100644 --- a/plugins/codexclaw/components/config-guard/package.json +++ b/plugins/codexclaw/components/config-guard/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/config-guard", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "Controlled feature-flag activation: enables only codexclaw's declared [features] flags via the official `codex features` CLI, with a revert manifest and backup.", diff --git a/plugins/codexclaw/components/cxc-ops/package.json b/plugins/codexclaw/components/cxc-ops/package.json index 765ed776..fab2b4c9 100644 --- a/plugins/codexclaw/components/cxc-ops/package.json +++ b/plugins/codexclaw/components/cxc-ops/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/cxc-ops", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "codexclaw ops CLI — doctor (plugin health), reset (scoped state cleanup).", diff --git a/plugins/codexclaw/components/messenger-bridge/package.json b/plugins/codexclaw/components/messenger-bridge/package.json index 0ce6e041..37de7498 100644 --- a/plugins/codexclaw/components/messenger-bridge/package.json +++ b/plugins/codexclaw/components/messenger-bridge/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/messenger-bridge", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "codexclaw messenger bridge — cxc serve HTTP server + SQLite state substrate (zero third-party deps).", diff --git a/plugins/codexclaw/components/pabcd-state/dist/divergence-cli.js b/plugins/codexclaw/components/pabcd-state/dist/divergence-cli.js index 587e1235..ae1d5746 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/divergence-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/divergence-cli.js @@ -69,11 +69,37 @@ function readSession(argv ) { return readFlag(argv, "--session") ?? readFlag(argv, "-s"); } +export function renderDivergenceHelp() { + return [ + "cxc divergence — record the deliberate divergence mode and its candidates", + "", + "Usage:", + " cxc divergence mode on --session --collapse P|D --reason [--json]", + " cxc divergence mode off --session --reason [--json]", + " cxc divergence candidate add --session --kind strong-1|add-1|alternative", + " --change-class parameter-tweak|branch-toggle|state-space-redesign|evaluator-change", + " --title --rationale --source [--source ]...", + " [--killed-at-phase P|A|B|C|D] [--json]", + " cxc divergence candidate list --session [--json]", + " cxc divergence --help", + "", + "Notes:", + " Collapse EARLY at P for satisfy-spec work; collapse LATE at D for", + " maximize-metric work where the local metric can deceive (cxc-loop).", + " Turn divergence off once the plateau is broken — it is a mode, not a state.", + " Every candidate needs at least one --source: an unsourced candidate is a guess.", + ].join("\n"); +} + export function runDivergenceCli(argv , cwd ) { const cwdOut = readFlag(argv, "--cwd") ?? cwd; const topic = argv[0] ?? ""; const verb = argv[1] ?? ""; const json = hasFlag(argv, "--json"); + // 260825 wp1: same defect as metric — --help died on the session guard. + if (argv.length === 0 || topic === "help" || topic === "--help" || topic === "-h") { + return { code: 0, output: renderDivergenceHelp() }; + } const sessionId = readSession(argv); if (!sessionId) return { code: 1, output: "divergence: --session is required" }; diff --git a/plugins/codexclaw/components/pabcd-state/dist/freeze-cli.js b/plugins/codexclaw/components/pabcd-state/dist/freeze-cli.js index 24b9ef9b..bb8d5a33 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/freeze-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/freeze-cli.js @@ -46,6 +46,9 @@ function listPlanFiles(planDir ) { + + + export function parseFreezeArgs(argv ) { const get = (flag ) => { const i = argv.indexOf(flag); @@ -55,10 +58,31 @@ export function parseFreezeArgs(argv ) { cwd: get("--cwd") ?? process.cwd(), sessionId: get("--session") ?? "default", dryRun: argv.includes("--dry-run"), + // 260825 wp1: `cxc freeze --help` used to fall straight through to runFreeze, + // which WROTE .codexclaw/interview/freeze.json and exited 0 — a workspace + // mutation behind a read-only-looking flag, with nothing in the output to + // signal it. Help is now parsed, and runFreeze returns before any IO. + help: argv.length === 0 || argv.some((a) => a === "help" || a === "--help" || a === "-h"), }; } export function runFreeze(args ) { + if (args.help) { + return [ + "cxc freeze — build or preview the interview freeze manifest", + "", + "Usage:", + " cxc freeze --session [--cwd ]", + " cxc freeze --dry-run --session [--cwd ]", + " cxc freeze --help", + "", + "Notes:", + " Hashes the plan files under .codexclaw/plan// and writes the manifest", + " at .codexclaw/interview/freeze.json, then reports staleness against any", + " existing manifest.", + " --dry-run previews without writing. --help never writes.", + ].join("\n"); + } const state = readState(args.cwd, args.sessionId); const tracker = state.interview; const ready = isInterviewReady(tracker); diff --git a/plugins/codexclaw/components/pabcd-state/dist/hook.js b/plugins/codexclaw/components/pabcd-state/dist/hook.js index 45933e1e..864e518a 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/hook.js +++ b/plugins/codexclaw/components/pabcd-state/dist/hook.js @@ -465,17 +465,23 @@ export const TRIGGER_AUTHORITY_NOTE = [ * no inline spelling that works. Telling a Windows agent otherwise is how it * concludes the FSM is broken. Same reasoning as `stopNextCommand` below. */ +/** The P>A object every arming surface shows. One definition, so the win32 and + * posix branches cannot drift, and so from/to are never dropped from one of them. */ +const PA_ATTEST_EXAMPLE = + '{"from":"P","to":"A","did":"...","planUnit":"devlog/_plan/YYMMDD_slug","workPhaseId":"wp1"}'; + export function loopArmDirective(platform = process.platform) { const advance = platform === "win32" ? [ "4. Advance EVERY forward edge yourself. On Windows write the JSON first, then attest:", - " `'' | Set-Content -Encoding utf8 .codexclaw/attest.json` then", + ` \`'${PA_ATTEST_EXAMPLE}' | Set-Content -Encoding utf8 .codexclaw/attest.json\` then`, " `cxc orchestrate --session --attest-file .codexclaw/attest.json` —", " inline --attest cannot survive PowerShell argument parsing (quotes are stripped,", " and escaping them splits the value at its first space).", ] : [ "4. Advance EVERY forward edge yourself with `cxc orchestrate --attest ` —", + ` e.g. \`cxc orchestrate A --session --attest '${PA_ATTEST_EXAMPLE}'\` —`, ]; return [ "[codexclaw: LOOP — orchestrate arming mandate (ORCH-MANDATE-01)]", @@ -490,6 +496,8 @@ export function loopArmDirective(platform = process.platform) " HITL (no such ask): enter the cycle explicitly via `cxc orchestrate I|P --session `.", ...advance, " a phase without its persisted transition + artifact did not happen (ORCH-ARTIFACT-01).", + ' EVERY attest carries "from" and "to" naming the edge: they are coerced before any', + " gate runs, so omitting them is refused on every edge (ATTEST-SHAPE-01).", " When a goalplan is bound, include the active workPhaseId in every gated attest", " (one work-phase = one full PABCD cycle).", "5. After D closes to IDLE with work remaining under an active goal, immediately re-enter", @@ -1034,10 +1042,16 @@ function bumpStopCounter(cwd , state ) { const STOP_NEXT_COMMAND = { I: '`cxc orchestrate P --attest \'{"from":"I","to":"P","did":"interview complete with recorded requirements"}\'`', - P: '`cxc orchestrate A --attest \'{"from":"P","to":"A","did":"diff-level plan written with files and acceptance criteria"}\'`', - A: '`cxc orchestrate B --attest \'{"from":"A","to":"B","did":"audit loop closed: blockers folded into plan","auditOutput":"","auditVerdict":"pass|near-pass","auditResidual":""}\'`', - B: '`cxc orchestrate C --attest \'{"from":"B","to":"C","did":"implementation completed and verifier reviewed it"}\'`', - C: '`cxc orchestrate D --attest \'{"from":"C","to":"D","did":"checks passed","checkOutput":"","exitCode":0}\'`', + // 260825 wp1: these are the commands agents copy straight out of a Stop block, + // so an example missing a required key spends the agent's next turn on a + // refusal. P>A needs planUnit; every gated edge needs workPhaseId when a + // goalplan is bound; C>D needs testReceiptPath. The bound-only keys are shown + // with a marker rather than omitted — a key you must delete is cheaper to fix + // than a key you never knew existed. + P: '`cxc orchestrate A --attest \'{"from":"P","to":"A","did":"diff-level plan written with files and acceptance criteria","planUnit":"devlog/_plan/YYMMDD_slug","workPhaseId":""}\'`', + A: '`cxc orchestrate B --attest \'{"from":"A","to":"B","did":"audit loop closed: blockers folded into plan","auditOutput":"","auditVerdict":"pass|near-pass","auditResidual":"","workPhaseId":""}\'`', + B: '`cxc orchestrate C --attest \'{"from":"B","to":"C","did":"implementation completed and verifier reviewed it","workPhaseId":""}\'`', + C: '`cxc orchestrate D --attest \'{"from":"C","to":"D","did":"checks passed","checkOutput":"","exitCode":0,"testReceiptPath":"","workPhaseId":""}\'`', D: '`cxc orchestrate reset` after the DONE summary is recorded', }; @@ -1160,8 +1174,8 @@ export function buildGoalIdleBlock( // Same PowerShell constraint as loopArmDirective: inline JSON cannot survive // argument parsing, so win32 gets the write-then-attest pair instead. const startNext = platform === "win32" - ? `Either start the next work-phase now: write the JSON with \`'{"from":"IDLE","to":"P","evidence":""}' | Set-Content -Encoding utf8 .codexclaw/attest.json\` then run \`cxc orchestrate P --session ${sessionId} --attest-file .codexclaw/attest.json\`` - : `Either start the next work-phase now: \`cxc orchestrate P --session ${sessionId} --attest '{"from":"IDLE","to":"P","evidence":""}'\``; + ? `Either start the next work-phase now: write the JSON with \`'{"from":"IDLE","to":"P","did":""}' | Set-Content -Encoding utf8 .codexclaw/attest.json\` then run \`cxc orchestrate P --session ${sessionId} --attest-file .codexclaw/attest.json\`` + : `Either start the next work-phase now: \`cxc orchestrate P --session ${sessionId} --attest '{"from":"IDLE","to":"P","did":""}'\``; const lines = [ "[codexclaw — goal continuation] A host goal is ACTIVE but no PABCD cycle is in flight.", "GOAL-IDLE-CONTINUE-01: IDLE is not the end while the goal is active (LOOP-CONTINUE-01). Do not end the turn here.", diff --git a/plugins/codexclaw/components/pabcd-state/dist/interview-ledger.js b/plugins/codexclaw/components/pabcd-state/dist/interview-ledger.js index 27c237b9..160bd616 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/interview-ledger.js +++ b/plugins/codexclaw/components/pabcd-state/dist/interview-ledger.js @@ -139,6 +139,77 @@ export function readQaEvents(cwd , sessionId ) return out; } +/** + * 260825: which dimensions hold a question the user was actually asked AND + * actually answered. + * + * This exists because `deriveLevel` cannot tell the difference between a + * dimension settled by an interview and one an agent typed a fact into. Both + * reach `high`; only the first is evidence. Measured before this was written: + * + * cxc scan record --session s --known goal=x --known constraint=x \ + * --known success=x --known ontology=x + * -> all four dimensions "high", scanRounds 1, in one command + * + * The answer is provenance, and provenance lives in this append-only file + * rather than on the tracker, which `writeState` rewrites wholesale and which a + * hand edit can set to anything. A dimension counts only when the SAME file + * carries all three: the question was asked, it was answered, and a + * `scan_completed` round attributed that questionId to that dimension. + * + * Not tamper-proof: this is a JSONL file, and whoever can write session state + * can append to it. It closes the accidental path — the one an agent takes + * because it is cheaper than asking — and leaves forgery a deliberate act. + */ +export function dimensionsBackedByAnswers(cwd , sessionId ) { + let raw ; + try { + raw = readFileSync(ledgerPath(cwd, sessionId), "utf8"); + } catch { + return new Set(); + } + const asked = new Set (); + const answered = new Set (); + // questionId -> dimension, accumulated across every scan round in this session. + const attribution = new Map (); + + for (const line of splitLines(raw)) { + const t = line.trim(); + if (!t) continue; + let o ; + try { + o = JSON.parse(t); + } catch { + continue; + } + if (!isRecord(o)) continue; + + if (o.event === "question_asked" && typeof o.questionId === "string") { + asked.add(o.questionId); + continue; + } + // An empty `answers` array is not an answer. The capture writer only emits + // the row when the user responded, but a hand-appended row need not. + if (o.event === "answer_recorded" && typeof o.questionId === "string") { + if (Array.isArray(o.answers) && o.answers.some((a) => typeof a === "string" && a.trim().length > 0)) { + answered.add(o.questionId); + } + continue; + } + if (o.event === "scan_completed" && isRecord(o.map)) { + for (const [questionId, dimension] of Object.entries(o.map)) { + if (typeof dimension === "string" && dimension.length > 0) attribution.set(questionId, dimension); + } + } + } + + const backed = new Set (); + for (const [questionId, dimension] of attribution) { + if (asked.has(questionId) && answered.has(questionId)) backed.add(dimension); + } + return backed; +} + /** True when an event with this id already exists (dedup guard). */ function alreadyRecorded(cwd , sessionId , eventId ) { return readQaEvents(cwd, sessionId).some((e) => e.eventId === eventId); diff --git a/plugins/codexclaw/components/pabcd-state/dist/interview.js b/plugins/codexclaw/components/pabcd-state/dist/interview.js index 0efd0a34..5ffa4df1 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/interview.js +++ b/plugins/codexclaw/components/pabcd-state/dist/interview.js @@ -228,10 +228,21 @@ export function reconstructInterview(v ) { /** * Readiness predicate (single source of truth for flags.interview). True ONLY when: * - tracker is a well-formed object, - * - all four dimensions are at level "max", + * - all four dimensions are at level "high" or "max", * - contradictions[] is empty, * - every assumption has recorded:true. * Never trusts a `ready` field on the tracker (none exists); always recomputed. + * + * 260825: this is the SHAPE half only. "max" used to be the sole accepted level, + * which no shipped writer could produce — `deriveLevel` tops out at "high" and + * `--dim =max` is rejected — so every interview either dead-ended or spent an + * attested override, and the override's ledger row stopped distinguishing + * anything. Accepting "high" alone would be the opposite failure: four `--known` + * flags reach all-high in one command. + * + * The provenance half lives in `evaluateInterviewGate`, which re-reads the + * append-only Q&A ledger. Callers that only need the shape (freeze, state flags) + * keep using this; the I->P decision uses the gate. */ /** Strict, fail-closed shape check for a single dimension score. */ function isValidScore(v ) { @@ -253,11 +264,11 @@ function isValidScore(v ) { export function isInterviewReady(tracker ) { if (!tracker || !isRecord(tracker)) return false; if (!isRecord(tracker.dimensions)) return false; - // Every dimension must be a fully-valid score at level "max" (T3: a partial - // {level:"max"} object must NOT pass). + // Every dimension must be a fully-valid score at "high" or "max" (T3: a partial + // score object must NOT pass, whatever its level claims). for (const d of DIMENSIONS) { const score = tracker.dimensions[d]; - if (!isValidScore(score) || score.level !== "max") return false; + if (!isValidScore(score) || (score.level !== "high" && score.level !== "max")) return false; } // Any contradiction (incl. malformed sentinels) blocks; contradictions must be empty. if (!Array.isArray(tracker.contradictions) || tracker.contradictions.length > 0) return false; @@ -286,7 +297,17 @@ export function isInterviewReady(tracker ) { * Evaluate the I->P soft-gate. This is advisory: the caller may advise-block or, on an * explicit human override, pre-flip the interview flag and proceed (logging the override). */ -export function evaluateInterviewGate(tracker ) { +/** + * The I->P gate: data shape AND provenance. + * + * `evidence` is optional so existing shape-only callers keep compiling, but the + * I->P decision must pass it. Without it the gate degrades to the shape check, + * which four `--known` flags can satisfy — see `dimensionsBackedByAnswers`. + */ +export function evaluateInterviewGate( + tracker , + evidence , +) { const t = tracker && isRecord(tracker) ? tracker : null; const scanRan = !!t && roundIdNum(t.scanRounds) >= 1; const highContradictionCount = @@ -296,7 +317,25 @@ export function evaluateInterviewGate(tracker ) const warnings = []; if (!scanRan) warnings.push("no contradiction scan has been recorded for this interview"); if (highContradictionCount > 0) warnings.push(`${highContradictionCount} high-severity contradiction(s) still open`); - const ready = isInterviewReady(tracker); + const shapeReady = isInterviewReady(tracker); + // A dimension at "high" is only as good as where the level came from. "max" is + // an explicit assertion no writer produces by accident, so it needs no ledger + // backing; "high" is derived, and derivation must trace to a real answer. + const unbacked = []; + if (shapeReady && evidence && t) { + for (const d of DIMENSIONS) { + const score = t.dimensions[d]; + if (isRecord(score) && score.level === "high" && !evidence.backedDimensions.has(d)) unbacked.push(d); + } + } + if (unbacked.length > 0) { + warnings.push( + `${unbacked.join(", ")} reached "high" without an answered question in the interview ledger — ` + + `ask and record one per dimension (\`cxc scan record --derive --map =\`), ` + + `or assert the level deliberately`, + ); + } + const ready = shapeReady && unbacked.length === 0; if (!ready && warnings.length === 0) warnings.push("interview is not ready (dimensions/assumptions incomplete)"); return { ready, scanRan, highContradictionCount, warnings }; } diff --git a/plugins/codexclaw/components/pabcd-state/dist/metric-cli.js b/plugins/codexclaw/components/pabcd-state/dist/metric-cli.js index e5600483..29270bda 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/metric-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/metric-cli.js @@ -69,9 +69,34 @@ function renderJson(value , json ) { return json ? JSON.stringify(value) : ""; } +export function renderMetricHelp() { + return [ + "cxc metric — session-scoped objective metrics for maximize-goal loops", + "", + "Usage:", + " cxc metric record --session --name --value [--source operator-entered|evaluate.sh] [--work-phase ] [--json]", + " cxc metric ingest --session --source evaluate.sh [--json] (reads stdin)", + " cxc metric show --session [--json]", + " cxc metric kind --session [--set satisfy|maximize] [--json]", + " cxc metric parse-line --session (reads stdin)", + " cxc metric --help", + "", + "Notes:", + " Two non-improving rows on the same metric switch the Stop block to", + " \"step back and re-plan with divergence\" (cxc-loop objective plateau).", + " --source records HOW the number was obtained; an operator-entered value and", + " an evaluate.sh value are not interchangeable evidence.", + ].join("\n"); +} + export function runMetricCli(argv , cwd , stdin = "") { const verb = argv[0] ?? ""; const json = hasFlag(argv, "--json"); + // 260825 wp1: --help used to be rejected with "--session is required", + // so the usage text below was unreachable from the documented entry point. + if (argv.length === 0 || verb === "help" || verb === "--help" || verb === "-h") { + return { code: 0, output: renderMetricHelp() }; + } const sessionId = readSession(argv); if (!sessionId) return { code: 1, output: "metric: --session is required" }; diff --git a/plugins/codexclaw/components/pabcd-state/dist/orchestrate-apply.js b/plugins/codexclaw/components/pabcd-state/dist/orchestrate-apply.js index 28b78660..75d4766c 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/orchestrate-apply.js +++ b/plugins/codexclaw/components/pabcd-state/dist/orchestrate-apply.js @@ -125,6 +125,10 @@ export function applyHumanTransition( // overridable gate flags: a ready interview opens it; an explicit human override // pre-flips it and records an audit entry; otherwise we advise-block. if (from === "I" && to === "P") { + // Shape-only on purpose. This is the HUMAN free-pass path: it has no cwd to + // read the interview ledger from, and a human is the authority the + // provenance check exists to approximate. The agent CLI path passes the + // ledger evidence. const gate = evaluateInterviewGate(state.interview ?? null); if (gate.ready) { flags.interview = true; diff --git a/plugins/codexclaw/components/pabcd-state/dist/orchestrate-cli.js b/plugins/codexclaw/components/pabcd-state/dist/orchestrate-cli.js index 84f1d23e..81a7de8b 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/orchestrate-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/orchestrate-cli.js @@ -17,7 +17,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import { join, resolve } from "node:path"; import { homedir } from "node:os"; import { coerceAttest, validateWorkPhaseBinding, GATED_TRANSITIONS, } from "./attest.js"; -import { canEnter, transition } from "./fsm.js"; +import { canEnter, transition, isLegalEdge, VALID_TRANSITIONS } from "./fsm.js"; import { validatePlanArtifacts } from "./plan-gate.js"; import { captureSourceIdentity, compareSource, describeSource } from "./source-identity.js"; import { randomBytes } from "node:crypto"; @@ -102,6 +102,7 @@ import { latestRound, supersedeStaleRounds } from "./review-round.js"; import { planFilesHash, recomputed } from "./review-round-cli.js"; import { validateCheckReceipt } from "./check-gate.js"; import { evaluateInterviewGate } from "./interview.js"; +import { dimensionsBackedByAnswers } from "./interview-ledger.js"; import { applyHumanTransition, clearedIdle } from "./orchestrate-apply.js"; import { resetRenderLedger } from "./render-observations.js"; @@ -165,7 +166,8 @@ export function renderOrchestrateHelp(platform = process.platfo "Attestation examples:", " cxc orchestrate A --session --attest '{\"from\":\"P\",\"to\":\"A\",\"did\":\"wrote and audited the plan\",\"planUnit\":\"devlog/_plan/260714_slug\",\"workPhaseId\":\"wp1\"}'", " cxc orchestrate B --session --attest '{\"from\":\"A\",\"to\":\"B\",\"did\":\"audit passed\",\"auditOutput\":\"VERDICT: PASS\",\"auditVerdict\":\"pass\",\"workPhaseId\":\"wp1\"}'", - " cxc orchestrate D --session --attest '{\"from\":\"C\",\"to\":\"D\",\"did\":\"verified\",\"checkOutput\":\"tests passed\",\"exitCode\":0,\"workPhaseId\":\"wp1\"}'", + " cxc orchestrate C --session --attest '{\"from\":\"B\",\"to\":\"C\",\"did\":\"implemented \",\"workPhaseId\":\"wp1\"}'", + " cxc orchestrate D --session --attest '{\"from\":\"C\",\"to\":\"D\",\"did\":\"verified\",\"checkOutput\":\"tests passed\",\"exitCode\":0,\"testReceiptPath\":\".codexclaw/evidence//test-receipt.json\",\"workPhaseId\":\"wp1\"}'", ]; return [ "cxc orchestrate — agent-gated IPABCD phase control", @@ -185,7 +187,9 @@ export function renderOrchestrateHelp(platform = process.platfo " status is read-only and may use the latest-session fallback when --session is omitted.", "", ...attestExamples, - " (workPhaseId is required on gated edges whenever a goalplan is bound to the session)", + " Every attest carries from/to naming the edge; they are coerced before any gate runs.", + " (workPhaseId is required on gated edges whenever a goalplan is bound to the session,", + " and testReceiptPath is required on C -> D for a bound session — see `cxc receipt test`)", "", "Status:", " cxc orchestrate status --session ", @@ -305,6 +309,59 @@ function renderPhaseContext(state , sessionId ) { return `current=${state.phase} session=${sessionId}`; } +/** + * Build the recovery half of a malformed-attest refusal (260825 wp1). + * + * The old text was the bare `attest JSON missing valid from/to`, which names the + * problem and nothing else — and it was the single most-hit agent-facing failure + * in the archive, because the skill table that agents copy never listed from/to + * at all. An agent whose attest was EMPTY already got a worked example from + * `attest.ts`; an agent whose attest was INCOMPLETE got nothing and retried the + * same omission. + * + * An earlier draft of the fix assumed the parser cannot know which edge is being + * advanced, and proposed `""/""` placeholders. An audit + * disproved it: `verb` is argv[0], resolved before the attest loop runs, so `to` + * is ALWAYS known, and the caller already reads session state on this exact error + * path, so `from` is known whenever `--session` resolves. A placeholder appears + * only for the value that genuinely cannot be determined. + * + * The extra keys named are the ones for THIS edge, not a menu of every key the + * FSM has. A menu would leave the agent guessing which half applies — the same + * failure in a longer form. + */ +export function renderAttestShapeHint(verb , from ) { + if (verb === "status" || verb === "reset") return ""; + const to = verb; + // A hint is only useful if the attest it teaches would be ACCEPTED. When the + // requested edge is illegal from the current phase, printing the real phase as + // `from` hands the agent an object that clears this gate and is then refused by + // the FSM adjacency check — two wrong refusals instead of one. In that case say + // what is actually wrong: the edge, not the JSON. + const legal = from === null || isLegalEdge(from, to); + if (from !== null && !legal) { + return ( + ` Note ${from} -> ${to} is not a legal edge, so no attest can advance it:` + + ` legal from ${from} is ${(VALID_TRANSITIONS[from] ?? []).join("|")}.` + ); + } + const fromText = from ?? ""; + const extras = { + A: ', plus "planUnit":"devlog/_plan/YYMMDD_slug"', + B: ', plus "auditOutput":"" and "auditVerdict":"pass|near-pass|fail"', + D: ', plus "checkOutput":"" and "exitCode":0', + }; + const extra = extras[to] ?? ""; + const statusHint = from + ? "" + : " Run `cxc orchestrate status --session ` to read the current phase."; + return ( + ` Every attest names the edge it advances: {"from":"${fromText}","to":"${to}","did":"..."}${extra}.` + + ` A goalplan-bound session also needs "workPhaseId"${to === "D" ? ' and "testReceiptPath"' : ''}.` + + statusHint + ); +} + function renderStatus(state , json , elsewhere = []) { if (json) { return JSON.stringify({ @@ -344,8 +401,18 @@ export function runOrchestrateCli(args // malformed --attest is a hard error before any state mutation (except control verbs). if (args.attestError && args.verb !== "status" && args.verb !== "reset") { const sessionIdForError = args.session && sessionFileExists(args.cwd, args.session) ? args.session : null; - const context = sessionIdForError ? `${renderPhaseContext(readState(args.cwd, sessionIdForError), sessionIdForError)}; ` : ""; - return { code: 1, output: `orchestrate ${args.verb}: ${context}${args.attestError}` }; + const stateForError = sessionIdForError ? readState(args.cwd, sessionIdForError) : null; + const context = + stateForError && sessionIdForError + ? `${renderPhaseContext(stateForError, sessionIdForError)}; ` + : ""; + // Only a SHAPE failure gets the worked example. "not valid JSON" and + // "requires a path argument" are different problems, and an example of a + // well-formed object would only muddy them. + const hint = args.attestError.includes("missing valid from/to") + ? renderAttestShapeHint(args.verb, stateForError?.phase ?? null) + : ""; + return { code: 1, output: `orchestrate ${args.verb}: ${context}${args.attestError}.${hint}` }; } const sessionId = resolveSession(args.cwd, args.session); @@ -443,7 +510,9 @@ export function runOrchestrateCli(args // which has no override support. This adds equivalent logic for I→P only, // recording actor:"agent" instead of actor:"human". if (state.phase === "I" && to === "P") { - const gate = evaluateInterviewGate(state.interview ?? null); + const gate = evaluateInterviewGate(state.interview ?? null, { + backedDimensions: dimensionsBackedByAnswers(args.cwd, sessionId), + }); if (gate.ready) { // Interview is ready — let the normal transition() path handle it. // (It will derive flags.interview=true from the tracker.) diff --git a/plugins/codexclaw/components/pabcd-state/dist/orchestrate-grammar.js b/plugins/codexclaw/components/pabcd-state/dist/orchestrate-grammar.js index 8b55dfa1..92ab16f1 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/orchestrate-grammar.js +++ b/plugins/codexclaw/components/pabcd-state/dist/orchestrate-grammar.js @@ -85,7 +85,20 @@ function parseAttestTail(rest ) try { const parsed = JSON.parse(json) ; const att = coerceAttest(parsed); - if (!att) return { rawAttest: json, attest: null, attestError: "attest JSON missing valid from/to" }; + if (!att) { + return { + rawAttest: json, + attest: null, + // Same wording as the CLI path (260825 wp1). The chat surface currently + // discards attestError (hook.ts handleOrchestrateCommand) and the human + // free-pass advances anyway, so this text is not yet user-visible — but + // leaving the two parsers disagreeing is how the next reader concludes + // one of them is right. + attestError: + 'attest JSON missing valid from/to. Every attest names the edge it advances: ' + + '{"from":"","to":"","did":"..."} plus that edge\'s keys (ATTEST-SHAPE-01).', + }; + } return { rawAttest: json, attest: att }; } catch { return { rawAttest: json, attest: null, attestError: "attest JSON is not valid JSON" }; diff --git a/plugins/codexclaw/components/pabcd-state/dist/plan-cli.js b/plugins/codexclaw/components/pabcd-state/dist/plan-cli.js index 818d24bf..b5a339de 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/plan-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/plan-cli.js @@ -71,6 +71,11 @@ export function derivePlanSlug(raw ) { /** Structural argv parse. argv excludes the `plan` kind token. */ export function parsePlanCliArgs(argv , cwd ) { const verb = (argv[0] ?? "").toLowerCase(); + // #47 finished (260825 wp1): --help was an unknown verb, so the top-level help's + // "run --help" pointer led to a rejection. + if (argv.length === 0 || verb === "help" || verb === "--help" || verb === "-h") { + return { verb: "help", slug: "", phases: 1, cwd, date: null }; + } if (verb !== "init") { return { error: `unknown plan verb '${argv[0] ?? ""}' (expected init)` }; } @@ -148,6 +153,25 @@ function phaseDoc(n , slug ) { } export function runPlanCli(args ) { + if (args.verb === "help") { + return { + code: 0, + output: [ + "cxc plan — scaffold a devlog plan unit (DIFFLEVEL-ROADMAP-01)", + "", + "Usage:", + " cxc plan init --slug [--phases ] [--date ] [--cwd ]", + " cxc plan --help", + "", + "Notes:", + " Creates devlog/_plan/_/ with 000_plan.md plus one decade doc", + " (010, 020, ...) per phase. P>A requires such a unit to exist on disk with", + " numbered docs — a chat-message plan does not satisfy Plan.", + " --date is for callers that already carry their own prefix; omit it to stamp today.", + " init refuses to overwrite an existing unit.", + ].join("\n"), + }; + } // args.date is the caller's own prefix when they passed one; only stamp today // when they did not (issue #30 - the doubled prefix came from stamping always). const unitName = `${args.date ?? yymmdd()}_${args.slug}`; diff --git a/plugins/codexclaw/components/pabcd-state/dist/review-round-cli.js b/plugins/codexclaw/components/pabcd-state/dist/review-round-cli.js index eeb580a1..92baab4b 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/review-round-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/review-round-cli.js @@ -95,6 +95,12 @@ function v2SpawnSurface() { export function parseReviewRoundCliArgs(argv , cwd ) { const verb = (argv[0] ?? "").toLowerCase(); + // #47 finished (260825 wp1): --help was an unknown verb here, so an agent that + // followed the top-level "run --help" pointer hit a rejection and had to + // learn every flag from refusals. Same branch shape as scan-cli. + if (argv.length === 0 || verb === "help" || verb === "--help" || verb === "-h") { + return { verb: "help" , cwd, planPaths: [] }; + } if (!VERBS.has(verb)) { return { error: `unknown review-round verb '${argv[0] ?? ""}' (expected open|show|abort)` }; } @@ -156,7 +162,30 @@ export function planFilesHash(files ) { return sha256(files.map((f) => `${f.path}\u0000${f.sha256}`).join("\u0000")); } +export function renderReviewRoundHelp() { + return [ + "cxc review-round — the opt-in A-gate plan-audit round (LEAN-REVIEW-01)", + "", + "Usage:", + " cxc review-round open --session [--plan-path ]... [--cwd ] [--json]", + " cxc review-round show --session [--cwd ] [--json]", + " cxc review-round abort --session [--reason ] [--cwd ]", + " cxc review-round --help", + "", + "Notes:", + " A round is OPTIONAL. With no round open, A>B advances on the attest alone.", + " With a round whose verdict was RECORDED, that verdict is binding: you cannot", + " attest \"pass\" over a reviewer's \"fail\".", + " open requires the session to be at phase A with a bound goalplan and a plan", + " binding recorded by P>A.", + " abort closes a round that no reviewer will finish, so the cycle is not stuck.", + ].join("\n"); +} + export function runReviewRoundCli(args ) { + if ((args.verb ) === "help") { + return { code: 0, output: renderReviewRoundHelp() }; + } const session = (args.session ?? "").trim(); if (session.length === 0) return { output: "review-round: --session is required", code: 1 }; const state = readState(args.cwd, session); diff --git a/plugins/codexclaw/components/pabcd-state/dist/scan-cli.js b/plugins/codexclaw/components/pabcd-state/dist/scan-cli.js index 5aaa3902..254100b4 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/scan-cli.js +++ b/plugins/codexclaw/components/pabcd-state/dist/scan-cli.js @@ -141,17 +141,23 @@ export function parseScanCliArgs( if (!isDimension(pair.key)) return { error: `scan record: unknown dimension '${pair.key}' (expected ${DIMENSIONS.join("|")})` }; if (!isLevel(pair.value)) return { error: `scan record: invalid level '${pair.value}' (expected ${DIMENSION_LEVELS.join("|")})` }; if (pair.value === "max") { - // `max` on all four dimensions is what isInterviewReady gates I->P on. - // The sanctioned way to reach P without a genuinely ready interview is - // `cxc orchestrate P --attest '{"override":true,...}'`, which validates - // the narrative and writes an auditable ledger row. Letting a writer flag - // grant `max` would be the same power with no attestation and no trail. + // `max` satisfies readiness without any ledger backing, so a writer flag + // that granted it would be the override with no attestation and no trail. + // + // Until 260825 this message named the override as the ONLY way onward, + // because it was: the gate demanded `max` and nothing could write it. Now + // the honest path exists, so name that first — an agent reaching for this + // flag usually wants a ready interview, not a bypass. return { error: - "scan record: --dim cannot set 'max'. That level gates I->P via isInterviewReady; " + - "write {\"from\":\"I\",\"to\":\"P\",\"did\":\"\",\"override\":true} to a file and run " + - "`cxc orchestrate P --session --attest-file ` (the file flag is required on Windows) " + - "so the bypass is attested and recorded in the ledger.", + "scan record: --dim cannot set 'max'. To make a dimension count for I->P, ask a " + + "question, record the answer, and attribute it: " + + "`cxc scan record --session --derive --map =`. " + + "The gate re-reads the interview ledger, so a level with no answered question " + + "behind it does not open P. If the interview genuinely is NOT complete, bypass it " + + "deliberately: write {\"from\":\"I\",\"to\":\"P\",\"did\":\"\",\"override\":true} to a " + + "file and run `cxc orchestrate P --session --attest-file ` (the file flag " + + "is required on Windows) so the bypass is attested and recorded.", }; } dims[pair.key] = pair.value; @@ -266,9 +272,15 @@ function deriveFromLedger( } /** - * Coverage-derived level. Deliberately never promotes to "max": that level gates - * I->P through isInterviewReady, so it stays an explicit operator assertion - * (`--dim =max`) rather than something a heuristic can hand out. + * Coverage-derived level, ceiling "high". + * + * "max" is unreachable from here AND from `--dim`, which rejects it. No writer + * produces it; it survives as the level that satisfies readiness without ledger + * backing — a deliberate hand-assertion, not something a heuristic hands out. + * + * "high" says only that this dimension holds facts and lists no open gap. It does + * NOT say the facts came from an interview: `--known goal=x` reaches "high" too. + * The I->P gate checks provenance separately, in `dimensionsBackedByAnswers`. */ function deriveLevel(score ) { // "low" means nothing is known about this dimension at all. An asked-but- @@ -299,7 +311,9 @@ export function runScanCli(args ) " --cwd matters when the answer ledger lives outside the process cwd:", " answers are read from /.codexclaw/interviews/.jsonl.", " --derive folds captured answers in; --map attributes a questionId to a dimension.", - " --dim cannot set 'max' — that level gates I->P and must be attested.", + " --derive is what makes a dimension count for I->P readiness: the gate re-reads the", + " ledger and requires an asked+answered+mapped question per dimension. --known records", + " a fact but lends no provenance, and --dim cannot set 'max'.", ].join("\n"), code: 0, }; @@ -315,6 +329,10 @@ export function runScanCli(args ) roundId, contradictionCount: args.contradictionCount, highContradictionCount: args.highContradictionCount, + // Carry the attributions this round used. Without them the ledger records + // that questions were answered but not which dimension each one settled, + // and readiness cannot tell a derived level from a typed one. + ...(args.derive && Object.keys(args.map ?? {}).length > 0 ? { map: { ...args.map } } : {}), }); let dimensions = tracker.dimensions; let derivedCount = 0; diff --git a/plugins/codexclaw/components/pabcd-state/dist/state.js b/plugins/codexclaw/components/pabcd-state/dist/state.js index 38bfe92b..4fdfbad9 100644 --- a/plugins/codexclaw/components/pabcd-state/dist/state.js +++ b/plugins/codexclaw/components/pabcd-state/dist/state.js @@ -397,6 +397,20 @@ export const SCAN_EVENT_KINDS = new Set + + + + + + + + + + + + + + diff --git a/plugins/codexclaw/components/pabcd-state/package.json b/plugins/codexclaw/components/pabcd-state/package.json index 7e520525..9c17101a 100644 --- a/plugins/codexclaw/components/pabcd-state/package.json +++ b/plugins/codexclaw/components/pabcd-state/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/pabcd-state", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "IPABCD finite-state machine backed by per-session .codexclaw/sessions/.json + shared ledger.jsonl.", diff --git a/plugins/codexclaw/components/pabcd-state/src/divergence-cli.ts b/plugins/codexclaw/components/pabcd-state/src/divergence-cli.ts index d4992c48..816a4457 100644 --- a/plugins/codexclaw/components/pabcd-state/src/divergence-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/divergence-cli.ts @@ -69,11 +69,37 @@ function readSession(argv: string[]): string | null { return readFlag(argv, "--session") ?? readFlag(argv, "-s"); } +export function renderDivergenceHelp(): string { + return [ + "cxc divergence — record the deliberate divergence mode and its candidates", + "", + "Usage:", + " cxc divergence mode on --session --collapse P|D --reason [--json]", + " cxc divergence mode off --session --reason [--json]", + " cxc divergence candidate add --session --kind strong-1|add-1|alternative", + " --change-class parameter-tweak|branch-toggle|state-space-redesign|evaluator-change", + " --title --rationale --source [--source ]...", + " [--killed-at-phase P|A|B|C|D] [--json]", + " cxc divergence candidate list --session [--json]", + " cxc divergence --help", + "", + "Notes:", + " Collapse EARLY at P for satisfy-spec work; collapse LATE at D for", + " maximize-metric work where the local metric can deceive (cxc-loop).", + " Turn divergence off once the plateau is broken — it is a mode, not a state.", + " Every candidate needs at least one --source: an unsourced candidate is a guess.", + ].join("\n"); +} + export function runDivergenceCli(argv: string[], cwd: string): DivergenceCliResult { const cwdOut = readFlag(argv, "--cwd") ?? cwd; const topic = argv[0] ?? ""; const verb = argv[1] ?? ""; const json = hasFlag(argv, "--json"); + // 260825 wp1: same defect as metric — --help died on the session guard. + if (argv.length === 0 || topic === "help" || topic === "--help" || topic === "-h") { + return { code: 0, output: renderDivergenceHelp() }; + } const sessionId = readSession(argv); if (!sessionId) return { code: 1, output: "divergence: --session is required" }; diff --git a/plugins/codexclaw/components/pabcd-state/src/freeze-cli.ts b/plugins/codexclaw/components/pabcd-state/src/freeze-cli.ts index 289927ce..8c693693 100644 --- a/plugins/codexclaw/components/pabcd-state/src/freeze-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/freeze-cli.ts @@ -44,6 +44,9 @@ export interface FreezeCliArgs { cwd: string; sessionId: string; dryRun: boolean; + /** True for `help`/`--help`/`-h` or a bare invocation. runFreeze returns + * before any filesystem write when this is set. */ + help?: boolean; } export function parseFreezeArgs(argv: string[]): FreezeCliArgs { @@ -55,10 +58,31 @@ export function parseFreezeArgs(argv: string[]): FreezeCliArgs { cwd: get("--cwd") ?? process.cwd(), sessionId: get("--session") ?? "default", dryRun: argv.includes("--dry-run"), + // 260825 wp1: `cxc freeze --help` used to fall straight through to runFreeze, + // which WROTE .codexclaw/interview/freeze.json and exited 0 — a workspace + // mutation behind a read-only-looking flag, with nothing in the output to + // signal it. Help is now parsed, and runFreeze returns before any IO. + help: argv.length === 0 || argv.some((a) => a === "help" || a === "--help" || a === "-h"), }; } export function runFreeze(args: FreezeCliArgs): string { + if (args.help) { + return [ + "cxc freeze — build or preview the interview freeze manifest", + "", + "Usage:", + " cxc freeze --session [--cwd ]", + " cxc freeze --dry-run --session [--cwd ]", + " cxc freeze --help", + "", + "Notes:", + " Hashes the plan files under .codexclaw/plan// and writes the manifest", + " at .codexclaw/interview/freeze.json, then reports staleness against any", + " existing manifest.", + " --dry-run previews without writing. --help never writes.", + ].join("\n"); + } const state = readState(args.cwd, args.sessionId); const tracker = state.interview; const ready = isInterviewReady(tracker); diff --git a/plugins/codexclaw/components/pabcd-state/src/hook.ts b/plugins/codexclaw/components/pabcd-state/src/hook.ts index 4533620e..f7402e5d 100644 --- a/plugins/codexclaw/components/pabcd-state/src/hook.ts +++ b/plugins/codexclaw/components/pabcd-state/src/hook.ts @@ -465,17 +465,23 @@ export const TRIGGER_AUTHORITY_NOTE = [ * no inline spelling that works. Telling a Windows agent otherwise is how it * concludes the FSM is broken. Same reasoning as `stopNextCommand` below. */ +/** The P>A object every arming surface shows. One definition, so the win32 and + * posix branches cannot drift, and so from/to are never dropped from one of them. */ +const PA_ATTEST_EXAMPLE = + '{"from":"P","to":"A","did":"...","planUnit":"devlog/_plan/YYMMDD_slug","workPhaseId":"wp1"}'; + export function loopArmDirective(platform: NodeJS.Platform = process.platform): string { const advance = platform === "win32" ? [ "4. Advance EVERY forward edge yourself. On Windows write the JSON first, then attest:", - " `'' | Set-Content -Encoding utf8 .codexclaw/attest.json` then", + ` \`'${PA_ATTEST_EXAMPLE}' | Set-Content -Encoding utf8 .codexclaw/attest.json\` then`, " `cxc orchestrate --session --attest-file .codexclaw/attest.json` —", " inline --attest cannot survive PowerShell argument parsing (quotes are stripped,", " and escaping them splits the value at its first space).", ] : [ "4. Advance EVERY forward edge yourself with `cxc orchestrate --attest ` —", + ` e.g. \`cxc orchestrate A --session --attest '${PA_ATTEST_EXAMPLE}'\` —`, ]; return [ "[codexclaw: LOOP — orchestrate arming mandate (ORCH-MANDATE-01)]", @@ -490,6 +496,8 @@ export function loopArmDirective(platform: NodeJS.Platform = process.platform): " HITL (no such ask): enter the cycle explicitly via `cxc orchestrate I|P --session `.", ...advance, " a phase without its persisted transition + artifact did not happen (ORCH-ARTIFACT-01).", + ' EVERY attest carries "from" and "to" naming the edge: they are coerced before any', + " gate runs, so omitting them is refused on every edge (ATTEST-SHAPE-01).", " When a goalplan is bound, include the active workPhaseId in every gated attest", " (one work-phase = one full PABCD cycle).", "5. After D closes to IDLE with work remaining under an active goal, immediately re-enter", @@ -1034,10 +1042,16 @@ function bumpStopCounter(cwd: string, state: State): number | "release" { const STOP_NEXT_COMMAND: Partial> = { I: '`cxc orchestrate P --attest \'{"from":"I","to":"P","did":"interview complete with recorded requirements"}\'`', - P: '`cxc orchestrate A --attest \'{"from":"P","to":"A","did":"diff-level plan written with files and acceptance criteria"}\'`', - A: '`cxc orchestrate B --attest \'{"from":"A","to":"B","did":"audit loop closed: blockers folded into plan","auditOutput":"","auditVerdict":"pass|near-pass","auditResidual":""}\'`', - B: '`cxc orchestrate C --attest \'{"from":"B","to":"C","did":"implementation completed and verifier reviewed it"}\'`', - C: '`cxc orchestrate D --attest \'{"from":"C","to":"D","did":"checks passed","checkOutput":"","exitCode":0}\'`', + // 260825 wp1: these are the commands agents copy straight out of a Stop block, + // so an example missing a required key spends the agent's next turn on a + // refusal. P>A needs planUnit; every gated edge needs workPhaseId when a + // goalplan is bound; C>D needs testReceiptPath. The bound-only keys are shown + // with a marker rather than omitted — a key you must delete is cheaper to fix + // than a key you never knew existed. + P: '`cxc orchestrate A --attest \'{"from":"P","to":"A","did":"diff-level plan written with files and acceptance criteria","planUnit":"devlog/_plan/YYMMDD_slug","workPhaseId":""}\'`', + A: '`cxc orchestrate B --attest \'{"from":"A","to":"B","did":"audit loop closed: blockers folded into plan","auditOutput":"","auditVerdict":"pass|near-pass","auditResidual":"","workPhaseId":""}\'`', + B: '`cxc orchestrate C --attest \'{"from":"B","to":"C","did":"implementation completed and verifier reviewed it","workPhaseId":""}\'`', + C: '`cxc orchestrate D --attest \'{"from":"C","to":"D","did":"checks passed","checkOutput":"","exitCode":0,"testReceiptPath":"","workPhaseId":""}\'`', D: '`cxc orchestrate reset` after the DONE summary is recorded', }; @@ -1160,8 +1174,8 @@ export function buildGoalIdleBlock( // Same PowerShell constraint as loopArmDirective: inline JSON cannot survive // argument parsing, so win32 gets the write-then-attest pair instead. const startNext = platform === "win32" - ? `Either start the next work-phase now: write the JSON with \`'{"from":"IDLE","to":"P","evidence":""}' | Set-Content -Encoding utf8 .codexclaw/attest.json\` then run \`cxc orchestrate P --session ${sessionId} --attest-file .codexclaw/attest.json\`` - : `Either start the next work-phase now: \`cxc orchestrate P --session ${sessionId} --attest '{"from":"IDLE","to":"P","evidence":""}'\``; + ? `Either start the next work-phase now: write the JSON with \`'{"from":"IDLE","to":"P","did":""}' | Set-Content -Encoding utf8 .codexclaw/attest.json\` then run \`cxc orchestrate P --session ${sessionId} --attest-file .codexclaw/attest.json\`` + : `Either start the next work-phase now: \`cxc orchestrate P --session ${sessionId} --attest '{"from":"IDLE","to":"P","did":""}'\``; const lines = [ "[codexclaw — goal continuation] A host goal is ACTIVE but no PABCD cycle is in flight.", "GOAL-IDLE-CONTINUE-01: IDLE is not the end while the goal is active (LOOP-CONTINUE-01). Do not end the turn here.", diff --git a/plugins/codexclaw/components/pabcd-state/src/interview-ledger.ts b/plugins/codexclaw/components/pabcd-state/src/interview-ledger.ts index dbc31c7f..a2a07b6e 100644 --- a/plugins/codexclaw/components/pabcd-state/src/interview-ledger.ts +++ b/plugins/codexclaw/components/pabcd-state/src/interview-ledger.ts @@ -139,6 +139,77 @@ export function readQaEvents(cwd: string, sessionId: string): InterviewQaEvent[] return out; } +/** + * 260825: which dimensions hold a question the user was actually asked AND + * actually answered. + * + * This exists because `deriveLevel` cannot tell the difference between a + * dimension settled by an interview and one an agent typed a fact into. Both + * reach `high`; only the first is evidence. Measured before this was written: + * + * cxc scan record --session s --known goal=x --known constraint=x \ + * --known success=x --known ontology=x + * -> all four dimensions "high", scanRounds 1, in one command + * + * The answer is provenance, and provenance lives in this append-only file + * rather than on the tracker, which `writeState` rewrites wholesale and which a + * hand edit can set to anything. A dimension counts only when the SAME file + * carries all three: the question was asked, it was answered, and a + * `scan_completed` round attributed that questionId to that dimension. + * + * Not tamper-proof: this is a JSONL file, and whoever can write session state + * can append to it. It closes the accidental path — the one an agent takes + * because it is cheaper than asking — and leaves forgery a deliberate act. + */ +export function dimensionsBackedByAnswers(cwd: string, sessionId: string): Set { + let raw: string; + try { + raw = readFileSync(ledgerPath(cwd, sessionId), "utf8"); + } catch { + return new Set(); + } + const asked = new Set(); + const answered = new Set(); + // questionId -> dimension, accumulated across every scan round in this session. + const attribution = new Map(); + + for (const line of splitLines(raw)) { + const t = line.trim(); + if (!t) continue; + let o: unknown; + try { + o = JSON.parse(t); + } catch { + continue; + } + if (!isRecord(o)) continue; + + if (o.event === "question_asked" && typeof o.questionId === "string") { + asked.add(o.questionId); + continue; + } + // An empty `answers` array is not an answer. The capture writer only emits + // the row when the user responded, but a hand-appended row need not. + if (o.event === "answer_recorded" && typeof o.questionId === "string") { + if (Array.isArray(o.answers) && o.answers.some((a) => typeof a === "string" && a.trim().length > 0)) { + answered.add(o.questionId); + } + continue; + } + if (o.event === "scan_completed" && isRecord(o.map)) { + for (const [questionId, dimension] of Object.entries(o.map)) { + if (typeof dimension === "string" && dimension.length > 0) attribution.set(questionId, dimension); + } + } + } + + const backed = new Set(); + for (const [questionId, dimension] of attribution) { + if (asked.has(questionId) && answered.has(questionId)) backed.add(dimension); + } + return backed; +} + /** True when an event with this id already exists (dedup guard). */ function alreadyRecorded(cwd: string, sessionId: string, eventId: string): boolean { return readQaEvents(cwd, sessionId).some((e) => e.eventId === eventId); diff --git a/plugins/codexclaw/components/pabcd-state/src/interview.ts b/plugins/codexclaw/components/pabcd-state/src/interview.ts index c74e9edd..0f7382cf 100644 --- a/plugins/codexclaw/components/pabcd-state/src/interview.ts +++ b/plugins/codexclaw/components/pabcd-state/src/interview.ts @@ -228,10 +228,21 @@ export function reconstructInterview(v: unknown): InterviewTracker | null { /** * Readiness predicate (single source of truth for flags.interview). True ONLY when: * - tracker is a well-formed object, - * - all four dimensions are at level "max", + * - all four dimensions are at level "high" or "max", * - contradictions[] is empty, * - every assumption has recorded:true. * Never trusts a `ready` field on the tracker (none exists); always recomputed. + * + * 260825: this is the SHAPE half only. "max" used to be the sole accepted level, + * which no shipped writer could produce — `deriveLevel` tops out at "high" and + * `--dim =max` is rejected — so every interview either dead-ended or spent an + * attested override, and the override's ledger row stopped distinguishing + * anything. Accepting "high" alone would be the opposite failure: four `--known` + * flags reach all-high in one command. + * + * The provenance half lives in `evaluateInterviewGate`, which re-reads the + * append-only Q&A ledger. Callers that only need the shape (freeze, state flags) + * keep using this; the I->P decision uses the gate. */ /** Strict, fail-closed shape check for a single dimension score. */ function isValidScore(v: unknown): v is DimensionScore { @@ -253,11 +264,11 @@ function isValidScore(v: unknown): v is DimensionScore { export function isInterviewReady(tracker: InterviewTracker | null): boolean { if (!tracker || !isRecord(tracker)) return false; if (!isRecord(tracker.dimensions)) return false; - // Every dimension must be a fully-valid score at level "max" (T3: a partial - // {level:"max"} object must NOT pass). + // Every dimension must be a fully-valid score at "high" or "max" (T3: a partial + // score object must NOT pass, whatever its level claims). for (const d of DIMENSIONS) { const score = tracker.dimensions[d]; - if (!isValidScore(score) || score.level !== "max") return false; + if (!isValidScore(score) || (score.level !== "high" && score.level !== "max")) return false; } // Any contradiction (incl. malformed sentinels) blocks; contradictions must be empty. if (!Array.isArray(tracker.contradictions) || tracker.contradictions.length > 0) return false; @@ -286,7 +297,17 @@ export interface InterviewGate { * Evaluate the I->P soft-gate. This is advisory: the caller may advise-block or, on an * explicit human override, pre-flip the interview flag and proceed (logging the override). */ -export function evaluateInterviewGate(tracker: InterviewTracker | null): InterviewGate { +/** + * The I->P gate: data shape AND provenance. + * + * `evidence` is optional so existing shape-only callers keep compiling, but the + * I->P decision must pass it. Without it the gate degrades to the shape check, + * which four `--known` flags can satisfy — see `dimensionsBackedByAnswers`. + */ +export function evaluateInterviewGate( + tracker: InterviewTracker | null, + evidence?: { backedDimensions: ReadonlySet }, +): InterviewGate { const t = tracker && isRecord(tracker) ? tracker : null; const scanRan = !!t && roundIdNum(t.scanRounds) >= 1; const highContradictionCount = @@ -296,7 +317,25 @@ export function evaluateInterviewGate(tracker: InterviewTracker | null): Intervi const warnings: string[] = []; if (!scanRan) warnings.push("no contradiction scan has been recorded for this interview"); if (highContradictionCount > 0) warnings.push(`${highContradictionCount} high-severity contradiction(s) still open`); - const ready = isInterviewReady(tracker); + const shapeReady = isInterviewReady(tracker); + // A dimension at "high" is only as good as where the level came from. "max" is + // an explicit assertion no writer produces by accident, so it needs no ledger + // backing; "high" is derived, and derivation must trace to a real answer. + const unbacked: string[] = []; + if (shapeReady && evidence && t) { + for (const d of DIMENSIONS) { + const score = t.dimensions[d]; + if (isRecord(score) && score.level === "high" && !evidence.backedDimensions.has(d)) unbacked.push(d); + } + } + if (unbacked.length > 0) { + warnings.push( + `${unbacked.join(", ")} reached "high" without an answered question in the interview ledger — ` + + `ask and record one per dimension (\`cxc scan record --derive --map =\`), ` + + `or assert the level deliberately`, + ); + } + const ready = shapeReady && unbacked.length === 0; if (!ready && warnings.length === 0) warnings.push("interview is not ready (dimensions/assumptions incomplete)"); return { ready, scanRan, highContradictionCount, warnings }; } diff --git a/plugins/codexclaw/components/pabcd-state/src/metric-cli.ts b/plugins/codexclaw/components/pabcd-state/src/metric-cli.ts index f2d5e4ec..48c790fa 100644 --- a/plugins/codexclaw/components/pabcd-state/src/metric-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/metric-cli.ts @@ -69,9 +69,34 @@ function renderJson(value: unknown, json: boolean): string { return json ? JSON.stringify(value) : ""; } +export function renderMetricHelp(): string { + return [ + "cxc metric — session-scoped objective metrics for maximize-goal loops", + "", + "Usage:", + " cxc metric record --session --name --value [--source operator-entered|evaluate.sh] [--work-phase ] [--json]", + " cxc metric ingest --session --source evaluate.sh [--json] (reads stdin)", + " cxc metric show --session [--json]", + " cxc metric kind --session [--set satisfy|maximize] [--json]", + " cxc metric parse-line --session (reads stdin)", + " cxc metric --help", + "", + "Notes:", + " Two non-improving rows on the same metric switch the Stop block to", + " \"step back and re-plan with divergence\" (cxc-loop objective plateau).", + " --source records HOW the number was obtained; an operator-entered value and", + " an evaluate.sh value are not interchangeable evidence.", + ].join("\n"); +} + export function runMetricCli(argv: string[], cwd: string, stdin = ""): MetricCliResult { const verb = argv[0] ?? ""; const json = hasFlag(argv, "--json"); + // 260825 wp1: --help used to be rejected with "--session is required", + // so the usage text below was unreachable from the documented entry point. + if (argv.length === 0 || verb === "help" || verb === "--help" || verb === "-h") { + return { code: 0, output: renderMetricHelp() }; + } const sessionId = readSession(argv); if (!sessionId) return { code: 1, output: "metric: --session is required" }; diff --git a/plugins/codexclaw/components/pabcd-state/src/orchestrate-apply.ts b/plugins/codexclaw/components/pabcd-state/src/orchestrate-apply.ts index 92e44299..47aabc8c 100644 --- a/plugins/codexclaw/components/pabcd-state/src/orchestrate-apply.ts +++ b/plugins/codexclaw/components/pabcd-state/src/orchestrate-apply.ts @@ -125,6 +125,10 @@ export function applyHumanTransition( // overridable gate flags: a ready interview opens it; an explicit human override // pre-flips it and records an audit entry; otherwise we advise-block. if (from === "I" && to === "P") { + // Shape-only on purpose. This is the HUMAN free-pass path: it has no cwd to + // read the interview ledger from, and a human is the authority the + // provenance check exists to approximate. The agent CLI path passes the + // ledger evidence. const gate = evaluateInterviewGate(state.interview ?? null); if (gate.ready) { flags.interview = true; diff --git a/plugins/codexclaw/components/pabcd-state/src/orchestrate-cli.ts b/plugins/codexclaw/components/pabcd-state/src/orchestrate-cli.ts index f8314d9b..b4a13175 100644 --- a/plugins/codexclaw/components/pabcd-state/src/orchestrate-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/orchestrate-cli.ts @@ -17,7 +17,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import { join, resolve } from "node:path"; import { homedir } from "node:os"; import { coerceAttest, validateWorkPhaseBinding, GATED_TRANSITIONS, type Attestation } from "./attest.ts"; -import { canEnter, transition } from "./fsm.ts"; +import { canEnter, transition, isLegalEdge, VALID_TRANSITIONS } from "./fsm.ts"; import { validatePlanArtifacts } from "./plan-gate.ts"; import { captureSourceIdentity, compareSource, describeSource } from "./source-identity.ts"; import { randomBytes } from "node:crypto"; @@ -102,6 +102,7 @@ import { latestRound, supersedeStaleRounds } from "./review-round.ts"; import { planFilesHash, recomputed } from "./review-round-cli.ts"; import { validateCheckReceipt } from "./check-gate.ts"; import { evaluateInterviewGate } from "./interview.ts"; +import { dimensionsBackedByAnswers } from "./interview-ledger.ts"; import { applyHumanTransition, clearedIdle } from "./orchestrate-apply.ts"; import { resetRenderLedger } from "./render-observations.ts"; import type { OrchestrateVerb } from "./orchestrate-grammar.ts"; @@ -165,7 +166,8 @@ export function renderOrchestrateHelp(platform: NodeJS.Platform = process.platfo "Attestation examples:", " cxc orchestrate A --session --attest '{\"from\":\"P\",\"to\":\"A\",\"did\":\"wrote and audited the plan\",\"planUnit\":\"devlog/_plan/260714_slug\",\"workPhaseId\":\"wp1\"}'", " cxc orchestrate B --session --attest '{\"from\":\"A\",\"to\":\"B\",\"did\":\"audit passed\",\"auditOutput\":\"VERDICT: PASS\",\"auditVerdict\":\"pass\",\"workPhaseId\":\"wp1\"}'", - " cxc orchestrate D --session --attest '{\"from\":\"C\",\"to\":\"D\",\"did\":\"verified\",\"checkOutput\":\"tests passed\",\"exitCode\":0,\"workPhaseId\":\"wp1\"}'", + " cxc orchestrate C --session --attest '{\"from\":\"B\",\"to\":\"C\",\"did\":\"implemented \",\"workPhaseId\":\"wp1\"}'", + " cxc orchestrate D --session --attest '{\"from\":\"C\",\"to\":\"D\",\"did\":\"verified\",\"checkOutput\":\"tests passed\",\"exitCode\":0,\"testReceiptPath\":\".codexclaw/evidence//test-receipt.json\",\"workPhaseId\":\"wp1\"}'", ]; return [ "cxc orchestrate — agent-gated IPABCD phase control", @@ -185,7 +187,9 @@ export function renderOrchestrateHelp(platform: NodeJS.Platform = process.platfo " status is read-only and may use the latest-session fallback when --session is omitted.", "", ...attestExamples, - " (workPhaseId is required on gated edges whenever a goalplan is bound to the session)", + " Every attest carries from/to naming the edge; they are coerced before any gate runs.", + " (workPhaseId is required on gated edges whenever a goalplan is bound to the session,", + " and testReceiptPath is required on C -> D for a bound session — see `cxc receipt test`)", "", "Status:", " cxc orchestrate status --session ", @@ -305,6 +309,59 @@ function renderPhaseContext(state: State, sessionId: string): string { return `current=${state.phase} session=${sessionId}`; } +/** + * Build the recovery half of a malformed-attest refusal (260825 wp1). + * + * The old text was the bare `attest JSON missing valid from/to`, which names the + * problem and nothing else — and it was the single most-hit agent-facing failure + * in the archive, because the skill table that agents copy never listed from/to + * at all. An agent whose attest was EMPTY already got a worked example from + * `attest.ts`; an agent whose attest was INCOMPLETE got nothing and retried the + * same omission. + * + * An earlier draft of the fix assumed the parser cannot know which edge is being + * advanced, and proposed `""/""` placeholders. An audit + * disproved it: `verb` is argv[0], resolved before the attest loop runs, so `to` + * is ALWAYS known, and the caller already reads session state on this exact error + * path, so `from` is known whenever `--session` resolves. A placeholder appears + * only for the value that genuinely cannot be determined. + * + * The extra keys named are the ones for THIS edge, not a menu of every key the + * FSM has. A menu would leave the agent guessing which half applies — the same + * failure in a longer form. + */ +export function renderAttestShapeHint(verb: OrchestrateVerb, from: Phase | null): string { + if (verb === "status" || verb === "reset") return ""; + const to = verb; + // A hint is only useful if the attest it teaches would be ACCEPTED. When the + // requested edge is illegal from the current phase, printing the real phase as + // `from` hands the agent an object that clears this gate and is then refused by + // the FSM adjacency check — two wrong refusals instead of one. In that case say + // what is actually wrong: the edge, not the JSON. + const legal = from === null || isLegalEdge(from, to); + if (from !== null && !legal) { + return ( + ` Note ${from} -> ${to} is not a legal edge, so no attest can advance it:` + + ` legal from ${from} is ${(VALID_TRANSITIONS[from] ?? []).join("|")}.` + ); + } + const fromText = from ?? ""; + const extras: Partial> = { + A: ', plus "planUnit":"devlog/_plan/YYMMDD_slug"', + B: ', plus "auditOutput":"" and "auditVerdict":"pass|near-pass|fail"', + D: ', plus "checkOutput":"" and "exitCode":0', + }; + const extra = extras[to] ?? ""; + const statusHint = from + ? "" + : " Run `cxc orchestrate status --session ` to read the current phase."; + return ( + ` Every attest names the edge it advances: {"from":"${fromText}","to":"${to}","did":"..."}${extra}.` + + ` A goalplan-bound session also needs "workPhaseId"${to === "D" ? ' and "testReceiptPath"' : ''}.` + + statusHint + ); +} + function renderStatus(state: State, json: boolean, elsewhere: string[] = []): string { if (json) { return JSON.stringify({ @@ -344,8 +401,18 @@ export function runOrchestrateCli(args: OrchestrateCliArgs | OrchestrateCliHelpA // malformed --attest is a hard error before any state mutation (except control verbs). if (args.attestError && args.verb !== "status" && args.verb !== "reset") { const sessionIdForError = args.session && sessionFileExists(args.cwd, args.session) ? args.session : null; - const context = sessionIdForError ? `${renderPhaseContext(readState(args.cwd, sessionIdForError), sessionIdForError)}; ` : ""; - return { code: 1, output: `orchestrate ${args.verb}: ${context}${args.attestError}` }; + const stateForError = sessionIdForError ? readState(args.cwd, sessionIdForError) : null; + const context = + stateForError && sessionIdForError + ? `${renderPhaseContext(stateForError, sessionIdForError)}; ` + : ""; + // Only a SHAPE failure gets the worked example. "not valid JSON" and + // "requires a path argument" are different problems, and an example of a + // well-formed object would only muddy them. + const hint = args.attestError.includes("missing valid from/to") + ? renderAttestShapeHint(args.verb, stateForError?.phase ?? null) + : ""; + return { code: 1, output: `orchestrate ${args.verb}: ${context}${args.attestError}.${hint}` }; } const sessionId = resolveSession(args.cwd, args.session); @@ -443,7 +510,9 @@ export function runOrchestrateCli(args: OrchestrateCliArgs | OrchestrateCliHelpA // which has no override support. This adds equivalent logic for I→P only, // recording actor:"agent" instead of actor:"human". if (state.phase === "I" && to === "P") { - const gate = evaluateInterviewGate(state.interview ?? null); + const gate = evaluateInterviewGate(state.interview ?? null, { + backedDimensions: dimensionsBackedByAnswers(args.cwd, sessionId), + }); if (gate.ready) { // Interview is ready — let the normal transition() path handle it. // (It will derive flags.interview=true from the tracker.) diff --git a/plugins/codexclaw/components/pabcd-state/src/orchestrate-grammar.ts b/plugins/codexclaw/components/pabcd-state/src/orchestrate-grammar.ts index b8aa0648..cee40d07 100644 --- a/plugins/codexclaw/components/pabcd-state/src/orchestrate-grammar.ts +++ b/plugins/codexclaw/components/pabcd-state/src/orchestrate-grammar.ts @@ -85,7 +85,20 @@ function parseAttestTail(rest: string): Pick","to":"","did":"..."} plus that edge\'s keys (ATTEST-SHAPE-01).', + }; + } return { rawAttest: json, attest: att }; } catch { return { rawAttest: json, attest: null, attestError: "attest JSON is not valid JSON" }; diff --git a/plugins/codexclaw/components/pabcd-state/src/plan-cli.ts b/plugins/codexclaw/components/pabcd-state/src/plan-cli.ts index e139c836..80d5f36a 100644 --- a/plugins/codexclaw/components/pabcd-state/src/plan-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/plan-cli.ts @@ -11,7 +11,7 @@ import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { join, relative, resolve } from "node:path"; export interface PlanCliArgs { - verb: "init"; + verb: "init" | "help"; slug: string; phases: number; cwd: string; @@ -71,6 +71,11 @@ export function derivePlanSlug(raw: string): string { /** Structural argv parse. argv excludes the `plan` kind token. */ export function parsePlanCliArgs(argv: string[], cwd: string): PlanCliArgs | { error: string } { const verb = (argv[0] ?? "").toLowerCase(); + // #47 finished (260825 wp1): --help was an unknown verb, so the top-level help's + // "run --help" pointer led to a rejection. + if (argv.length === 0 || verb === "help" || verb === "--help" || verb === "-h") { + return { verb: "help", slug: "", phases: 1, cwd, date: null }; + } if (verb !== "init") { return { error: `unknown plan verb '${argv[0] ?? ""}' (expected init)` }; } @@ -148,6 +153,25 @@ function phaseDoc(n: number, slug: string): string { } export function runPlanCli(args: PlanCliArgs): PlanCliResult { + if (args.verb === "help") { + return { + code: 0, + output: [ + "cxc plan — scaffold a devlog plan unit (DIFFLEVEL-ROADMAP-01)", + "", + "Usage:", + " cxc plan init --slug [--phases ] [--date ] [--cwd ]", + " cxc plan --help", + "", + "Notes:", + " Creates devlog/_plan/_/ with 000_plan.md plus one decade doc", + " (010, 020, ...) per phase. P>A requires such a unit to exist on disk with", + " numbered docs — a chat-message plan does not satisfy Plan.", + " --date is for callers that already carry their own prefix; omit it to stamp today.", + " init refuses to overwrite an existing unit.", + ].join("\n"), + }; + } // args.date is the caller's own prefix when they passed one; only stamp today // when they did not (issue #30 - the doubled prefix came from stamping always). const unitName = `${args.date ?? yymmdd()}_${args.slug}`; diff --git a/plugins/codexclaw/components/pabcd-state/src/review-round-cli.ts b/plugins/codexclaw/components/pabcd-state/src/review-round-cli.ts index 0feb7806..f71f0d11 100644 --- a/plugins/codexclaw/components/pabcd-state/src/review-round-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/review-round-cli.ts @@ -95,6 +95,12 @@ export interface ReviewRoundCliParseError { error: string } export function parseReviewRoundCliArgs(argv: string[], cwd: string): ReviewRoundCliArgs | ReviewRoundCliParseError { const verb = (argv[0] ?? "").toLowerCase(); + // #47 finished (260825 wp1): --help was an unknown verb here, so an agent that + // followed the top-level "run --help" pointer hit a rejection and had to + // learn every flag from refusals. Same branch shape as scan-cli. + if (argv.length === 0 || verb === "help" || verb === "--help" || verb === "-h") { + return { verb: "help" as ReviewRoundVerb, cwd, planPaths: [] }; + } if (!VERBS.has(verb)) { return { error: `unknown review-round verb '${argv[0] ?? ""}' (expected open|show|abort)` }; } @@ -156,7 +162,30 @@ export function planFilesHash(files: PlanFileHash[]): string { return sha256(files.map((f) => `${f.path}\u0000${f.sha256}`).join("\u0000")); } +export function renderReviewRoundHelp(): string { + return [ + "cxc review-round — the opt-in A-gate plan-audit round (LEAN-REVIEW-01)", + "", + "Usage:", + " cxc review-round open --session [--plan-path ]... [--cwd ] [--json]", + " cxc review-round show --session [--cwd ] [--json]", + " cxc review-round abort --session [--reason ] [--cwd ]", + " cxc review-round --help", + "", + "Notes:", + " A round is OPTIONAL. With no round open, A>B advances on the attest alone.", + " With a round whose verdict was RECORDED, that verdict is binding: you cannot", + " attest \"pass\" over a reviewer's \"fail\".", + " open requires the session to be at phase A with a bound goalplan and a plan", + " binding recorded by P>A.", + " abort closes a round that no reviewer will finish, so the cycle is not stuck.", + ].join("\n"); +} + export function runReviewRoundCli(args: ReviewRoundCliArgs): ReviewRoundCliResult { + if ((args.verb as string) === "help") { + return { code: 0, output: renderReviewRoundHelp() }; + } const session = (args.session ?? "").trim(); if (session.length === 0) return { output: "review-round: --session is required", code: 1 }; const state = readState(args.cwd, session); diff --git a/plugins/codexclaw/components/pabcd-state/src/scan-cli.ts b/plugins/codexclaw/components/pabcd-state/src/scan-cli.ts index 7292901c..f64c412e 100644 --- a/plugins/codexclaw/components/pabcd-state/src/scan-cli.ts +++ b/plugins/codexclaw/components/pabcd-state/src/scan-cli.ts @@ -141,17 +141,23 @@ export function parseScanCliArgs( if (!isDimension(pair.key)) return { error: `scan record: unknown dimension '${pair.key}' (expected ${DIMENSIONS.join("|")})` }; if (!isLevel(pair.value)) return { error: `scan record: invalid level '${pair.value}' (expected ${DIMENSION_LEVELS.join("|")})` }; if (pair.value === "max") { - // `max` on all four dimensions is what isInterviewReady gates I->P on. - // The sanctioned way to reach P without a genuinely ready interview is - // `cxc orchestrate P --attest '{"override":true,...}'`, which validates - // the narrative and writes an auditable ledger row. Letting a writer flag - // grant `max` would be the same power with no attestation and no trail. + // `max` satisfies readiness without any ledger backing, so a writer flag + // that granted it would be the override with no attestation and no trail. + // + // Until 260825 this message named the override as the ONLY way onward, + // because it was: the gate demanded `max` and nothing could write it. Now + // the honest path exists, so name that first — an agent reaching for this + // flag usually wants a ready interview, not a bypass. return { error: - "scan record: --dim cannot set 'max'. That level gates I->P via isInterviewReady; " + - "write {\"from\":\"I\",\"to\":\"P\",\"did\":\"\",\"override\":true} to a file and run " + - "`cxc orchestrate P --session --attest-file ` (the file flag is required on Windows) " + - "so the bypass is attested and recorded in the ledger.", + "scan record: --dim cannot set 'max'. To make a dimension count for I->P, ask a " + + "question, record the answer, and attribute it: " + + "`cxc scan record --session --derive --map =`. " + + "The gate re-reads the interview ledger, so a level with no answered question " + + "behind it does not open P. If the interview genuinely is NOT complete, bypass it " + + "deliberately: write {\"from\":\"I\",\"to\":\"P\",\"did\":\"\",\"override\":true} to a " + + "file and run `cxc orchestrate P --session --attest-file ` (the file flag " + + "is required on Windows) so the bypass is attested and recorded.", }; } dims[pair.key] = pair.value; @@ -266,9 +272,15 @@ function deriveFromLedger( } /** - * Coverage-derived level. Deliberately never promotes to "max": that level gates - * I->P through isInterviewReady, so it stays an explicit operator assertion - * (`--dim =max`) rather than something a heuristic can hand out. + * Coverage-derived level, ceiling "high". + * + * "max" is unreachable from here AND from `--dim`, which rejects it. No writer + * produces it; it survives as the level that satisfies readiness without ledger + * backing — a deliberate hand-assertion, not something a heuristic hands out. + * + * "high" says only that this dimension holds facts and lists no open gap. It does + * NOT say the facts came from an interview: `--known goal=x` reaches "high" too. + * The I->P gate checks provenance separately, in `dimensionsBackedByAnswers`. */ function deriveLevel(score: DimensionScore): DimensionLevel { // "low" means nothing is known about this dimension at all. An asked-but- @@ -299,7 +311,9 @@ export function runScanCli(args: ScanCliArgs): { output: string; code: number } " --cwd matters when the answer ledger lives outside the process cwd:", " answers are read from /.codexclaw/interviews/.jsonl.", " --derive folds captured answers in; --map attributes a questionId to a dimension.", - " --dim cannot set 'max' — that level gates I->P and must be attested.", + " --derive is what makes a dimension count for I->P readiness: the gate re-reads the", + " ledger and requires an asked+answered+mapped question per dimension. --known records", + " a fact but lends no provenance, and --dim cannot set 'max'.", ].join("\n"), code: 0, }; @@ -315,6 +329,10 @@ export function runScanCli(args: ScanCliArgs): { output: string; code: number } roundId, contradictionCount: args.contradictionCount, highContradictionCount: args.highContradictionCount, + // Carry the attributions this round used. Without them the ledger records + // that questions were answered but not which dimension each one settled, + // and readiness cannot tell a derived level from a typed one. + ...(args.derive && Object.keys(args.map ?? {}).length > 0 ? { map: { ...args.map } } : {}), }); let dimensions = tracker.dimensions; let derivedCount = 0; diff --git a/plugins/codexclaw/components/pabcd-state/src/state.ts b/plugins/codexclaw/components/pabcd-state/src/state.ts index 8f824303..f636c6a2 100644 --- a/plugins/codexclaw/components/pabcd-state/src/state.ts +++ b/plugins/codexclaw/components/pabcd-state/src/state.ts @@ -399,6 +399,20 @@ export interface InterviewEvent { roundId: number; contradictionCount: number; highContradictionCount: number; + /** + * 260825: the `--map` attributions this scan round used, questionId -> dimension. + * + * `deriveFromLedger` takes the map as an argument and then discards it, so a + * later reader cannot tell which dimension a questionId belonged to. Readiness + * needs exactly that, to distinguish a level derived from a real answer from + * one an agent typed with `--known`. + * + * It lives here rather than on the tracker because the tracker is rewritten on + * every `writeState` and is hand-editable; this file is append-only and already + * holds the answers the map interprets. Evidence and interpretation share one + * provenance. + */ + map?: Record; } function interviewsDir(cwd: string): string { diff --git a/plugins/codexclaw/components/pabcd-state/test/attest-shape-hint.test.ts b/plugins/codexclaw/components/pabcd-state/test/attest-shape-hint.test.ts new file mode 100644 index 00000000..14980920 --- /dev/null +++ b/plugins/codexclaw/components/pabcd-state/test/attest-shape-hint.test.ts @@ -0,0 +1,260 @@ +/** + * attest-shape-hint.test.ts — 260825 wp1. + * + * `attest JSON missing valid from/to` was the most-hit agent-facing failure in the + * archive: 50+ occurrences across four repos, because the skill table agents copy + * never listed from/to. The message named the problem and nothing else, so the + * agent retried the same omission. + * + * These assertions deliberately pin substrings that did NOT exist before this + * change. Asserting that the output "contains from and to" would have passed + * against the original bare message — it contains both words — and an audit + * caught exactly that in the first draft of the plan. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { parseOrchestrateCliArgs, runOrchestrateCli, renderAttestShapeHint, renderOrchestrateHelp } from "../src/orchestrate-cli.ts"; +import { writeState, defaultState } from "../src/state.ts"; + +function freshCwd(): string { + return mkdtempSync(join(tmpdir(), "codexclaw-attest-hint-")); +} + +function seedSession(cwd: string, id: string, phase: string): void { + writeState(cwd, { ...defaultState(id), phase: phase as never }); +} + +test("inline --attest without from/to names the real edge and a worked example", () => { + const cwd = freshCwd(); + seedSession(cwd, "s1", "P"); + const args = parseOrchestrateCliArgs(["a", "--session", "s1", "--attest", '{"did":"wrote the plan"}'], cwd); + assert.ok(!("error" in args)); + const r = runOrchestrateCli(args as never); + + assert.equal(r.code, 1); + // `to` comes from the verb, which the parser resolves before the attest loop. + // `from` comes from the session state the error path already reads. + assert.match(r.output, /"from":"P","to":"A"/); + // A worked example: absent from the pre-fix message entirely. + assert.match(r.output, /"did":"\.\.\."/); + // The extra key for THIS edge, not a menu of every key the FSM has. + assert.match(r.output, /planUnit/); + // Positive form: a menu of every key would also "not mention auditVerdict" + // only by accident. Assert the bound-session note is edge-correct instead. + assert.match(r.output, /needs "workPhaseId"\./); + assert.doesNotMatch(r.output, /testReceiptPath/); + assert.doesNotMatch(r.output, /auditVerdict/); +}); + +test("--attest-file without from/to gets the hint on its own distinct wording", () => { + const cwd = freshCwd(); + seedSession(cwd, "s2", "C"); + const p = join(cwd, "bad-attest.json"); + writeFileSync(p, '{"did":"verified"}', "utf8"); + const args = parseOrchestrateCliArgs(["d", "--session", "s2", "--attest-file", p], cwd); + assert.ok(!("error" in args)); + const r = runOrchestrateCli(args as never); + + assert.equal(r.code, 1); + // :257 emits a DIFFERENT literal from :227 — the path is named. A single-path + // test would leave the Windows-required flag uncovered. + assert.match(r.output, /attest file .*bad-attest\.json is missing valid from\/to/); + // The pre-fix build also emitted this literal, so the path match alone proves + // nothing. The hint substrings below are what did not exist before. + assert.match(r.output, /"from":"C","to":"D"/); + assert.match(r.output, /checkOutput/); + assert.match(r.output, /exitCode/); +}); + +test("an unresolvable session yields a status pointer instead of a fabricated phase", () => { + const cwd = freshCwd(); + const args = parseOrchestrateCliArgs(["b", "--session", "never-created", "--attest", '{"did":"x"}'], cwd); + assert.ok(!("error" in args)); + const r = runOrchestrateCli(args as never); + + assert.equal(r.code, 1); + assert.match(r.output, /"to":"B"/); + // Never invent a `from`: say so and name the command that reveals it. + assert.match(r.output, /cxc orchestrate status --session/); + assert.match(r.output, /auditOutput/); +}); + +test("malformed JSON keeps its own diagnosis and gets no shape example", () => { + const cwd = freshCwd(); + seedSession(cwd, "s3", "P"); + const args = parseOrchestrateCliArgs(["a", "--session", "s3", "--attest", "{not json"], cwd); + assert.ok(!("error" in args)); + const r = runOrchestrateCli(args as never); + + assert.equal(r.code, 1); + assert.match(r.output, /attest JSON is not valid JSON/); + // A well-formed-object example would only muddy a syntax error. + assert.doesNotMatch(r.output, /"did":"\.\.\."/); +}); + +test("renderAttestShapeHint is silent for the control verbs", () => { + assert.equal(renderAttestShapeHint("status", "P"), ""); + assert.equal(renderAttestShapeHint("reset", "P"), ""); +}); + +test("orchestrate help ships a copy-paste object for every gated edge", () => { + for (const platform of ["linux", "win32"] as const) { + const help = renderOrchestrateHelp(platform); + assert.match(help, /"from":"P","to":"A"/, `${platform}: P->A example`); + } + // The posix branch carries the full ladder. B->C had no example at all and + // C->D omitted testReceiptPath until 260825 wp1, so the skill table could not + // honestly point at help as its source. + const posix = renderOrchestrateHelp("linux"); + assert.match(posix, /"from":"A","to":"B"/); + assert.match(posix, /"from":"B","to":"C"/); + assert.match(posix, /"from":"C","to":"D"/); + assert.match(posix, /testReceiptPath/); +}); + + +// Found by re-reading the shipped output before the reviewer got to it: the first +// version printed the CURRENT phase as `from` even when the requested edge was +// illegal, so an agent at P asking for D was handed {"from":"P","to":"D"} — an +// object that clears the coerce gate and is then refused by the FSM adjacency +// check. Two wrong refusals instead of one. A hint that teaches a rejected attest +// is worse than no hint. +test("an illegal edge names the legal routes instead of teaching a doomed attest", () => { + const cwd = freshCwd(); + seedSession(cwd, "s4", "P"); + const args = parseOrchestrateCliArgs(["d", "--session", "s4", "--attest", '{"did":"x"}'], cwd); + assert.ok(!("error" in args)); + const r = runOrchestrateCli(args as never); + + assert.equal(r.code, 1); + assert.match(r.output, /P -> D is not a legal edge/); + assert.match(r.output, /legal from P is I\|A/); + // Crucially: no example object, because every object would be refused. + assert.doesNotMatch(r.output, /"from":"P","to":"D"/); +}); + +test("a legal edge from the same phase still gets the worked example", () => { + const cwd = freshCwd(); + seedSession(cwd, "s5", "P"); + const args = parseOrchestrateCliArgs(["a", "--session", "s5", "--attest", '{"did":"x"}'], cwd); + assert.ok(!("error" in args)); + const r = runOrchestrateCli(args as never); + + assert.match(r.output, /"from":"P","to":"A"/); + assert.doesNotMatch(r.output, /not a legal edge/); +}); + + +// --------------------------------------------------------------------------- +// The injected surfaces. A Stop block and a goal-idle block are commands agents +// copy verbatim, so an example missing a key spends their next turn on a refusal +// — the same cascade this unit exists to close, one layer up. +// --------------------------------------------------------------------------- +import { stopNextCommand, buildGoalIdleBlock, loopArmDirective } from "../src/hook.ts"; + +test("every gated Stop command carries the keys its edge actually requires", () => { + // P>A needs planUnit; C>D needs testReceiptPath; every gated edge needs + // workPhaseId when a goalplan is bound. All three were absent. + assert.match(stopNextCommand("P", "linux") ?? "", /planUnit/); + assert.match(stopNextCommand("P", "linux") ?? "", /workPhaseId/); + assert.match(stopNextCommand("A", "linux") ?? "", /workPhaseId/); + assert.match(stopNextCommand("B", "linux") ?? "", /workPhaseId/); + assert.match(stopNextCommand("C", "linux") ?? "", /testReceiptPath/); + assert.match(stopNextCommand("C", "linux") ?? "", /workPhaseId/); + // from/to were already right here; assert them so a rewrite cannot drop them. + for (const phase of ["P", "A", "B", "C"] as const) { + assert.match(stopNextCommand(phase, "linux") ?? "", /"from":"/, `${phase} names from`); + assert.match(stopNextCommand(phase, "linux") ?? "", /"to":"/, `${phase} names to`); + } +}); + +test("the goal-idle block emits did, not evidence, and closes its backticks", () => { + const state = { ...defaultState("gi1"), phase: "IDLE" as const }; + for (const platform of ["linux", "win32"] as const) { + // buildGoalIdleBlock returns the hook JSON envelope, so read the reason. + const block = JSON.parse(buildGoalIdleBlock("/unused", state, "gi1", platform)).reason as string; + // 'evidence' is not a key coerceAttest reads. IDLE>P is ungated, so this + // advanced anyway and taught a wrong field name that failed silently. + assert.match(block, /"did":"/, `${platform}: uses did`); + assert.doesNotMatch(block, /"evidence":/, `${platform}: no evidence key`); + // A backtick-escape slip while renaming the key left a trailing backslash and + // an unterminated span in the rendered text. Caught in review, pinned here. + assert.doesNotMatch(block, /\\\\$/m, `${platform}: no line ends in a stray backslash`); + const ticks = (block.match(/`/g) ?? []).length; + assert.equal(ticks % 2, 0, `${platform}: backticks are balanced`); + } +}); + +test("the arming directive shows a from/to-bearing object on both platforms", () => { + for (const platform of ["linux", "win32"] as const) { + const d = loopArmDirective(platform); + assert.match(d, /"from":"P","to":"A"/, `${platform}: P>A object`); + assert.match(d, /planUnit/, `${platform}: planUnit`); + assert.match(d, /ATTEST-SHAPE-01/, `${platform}: names the rule`); + } +}); + + +// --------------------------------------------------------------------------- +// Drift detection. Everything above fixes today's text; this is what fails the +// build when the NEXT contract change forgets the skill again. The repo already +// works this way — see "shipped skill catalog exactly matches on-disk SKILL.md +// folders" — so the genre is house style, not an invention. +// --------------------------------------------------------------------------- +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, resolve as resolvePath } from "node:path"; +import { parseOrchestrateCommand } from "../src/orchestrate-grammar.ts"; + +const REPO = resolvePath(dirname(fileURLToPath(import.meta.url)), "..", "..", "..", "..", ".."); + +/** The attest table rows out of pabcd/SKILL.md, keyed by edge. */ +function attestTableRows(): Map { + const md = readFileSync(resolvePath(REPO, "plugins/codexclaw/skills/pabcd/SKILL.md"), "utf8"); + const rows = new Map(); + for (const line of md.split("\n")) { + const m = /^\|\s*(IDLE->P|I->P|P->A|A->B|B->C|C->D)\s*\|([^|]*)\|/.exec(line.trim()); + // Capture the KEYS cell only. Taking the rest of the row would let a + // mention in the Notes column satisfy a key the contract cell omits — the + // exact way this test first passed against injected drift. + if (m) rows.set(m[1], m[2]); + } + return rows; +} + +test("the pabcd attest table names every key its edge's gate requires", () => { + const rows = attestTableRows(); + // A file-wide grep would pass on any incidental mention in a 37k-character + // document. Bind to the ROW, so a key documented for the wrong edge fails. + const required: Record = { + "P->A": ["from", "to", "did", "planUnit"], + "A->B": ["from", "to", "did", "auditOutput", "auditVerdict"], + "B->C": ["from", "to", "did"], + "C->D": ["from", "to", "did", "checkOutput", "exitCode"], + }; + for (const [edge, keys] of Object.entries(required)) { + const row = rows.get(edge); + assert.ok(row, `the attest table has no row for ${edge}`); + for (const key of keys) { + assert.ok(row!.includes(key), `${edge} row must name "${key}" — the gate requires it`); + } + } + // The bound-session keys are stated once beneath the table rather than per row. + const md = readFileSync(resolvePath(REPO, "plugins/codexclaw/skills/pabcd/SKILL.md"), "utf8"); + assert.match(md, /workPhaseId/); + assert.match(md, /testReceiptPath/); + assert.match(md, /ATTEST-SHAPE-01/); +}); + +test("the chat grammar rejects a from/to-less attest with the same guidance", () => { + const cmd = parseOrchestrateCommand('orchestrate a --attest {"did":"x"}'); + assert.ok(cmd, "the line-anchored command must parse"); + assert.match(cmd!.attestError ?? "", /missing valid from\/to/); + // Both parsers now teach the same shape; a reader comparing them cannot + // conclude one of the two is authoritative. + assert.match(cmd!.attestError ?? "", /ATTEST-SHAPE-01/); +}); + diff --git a/plugins/codexclaw/components/pabcd-state/test/help-verbs.test.ts b/plugins/codexclaw/components/pabcd-state/test/help-verbs.test.ts index 67188385..f696c45e 100644 --- a/plugins/codexclaw/components/pabcd-state/test/help-verbs.test.ts +++ b/plugins/codexclaw/components/pabcd-state/test/help-verbs.test.ts @@ -75,3 +75,65 @@ test("scan record without --cwd still resolves against the process cwd", () => { assert.ok(!("error" in args)); assert.equal((args as { cwd: string }).cwd, CWD); }); + +// --------------------------------------------------------------------------- +// 260825 wp1 — #47 finished. These five verbs still rejected `--help` (or, worse, +// silently ran) long after the original fix landed for loop/receipt/scan. +// --------------------------------------------------------------------------- +import { parseReviewRoundCliArgs, runReviewRoundCli } from "../src/review-round-cli.ts"; +import { parsePlanCliArgs, runPlanCli } from "../src/plan-cli.ts"; +import { runMetricCli } from "../src/metric-cli.ts"; +import { runDivergenceCli } from "../src/divergence-cli.ts"; +import { parseFreezeArgs, runFreeze } from "../src/freeze-cli.ts"; +import { mkdtempSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +for (const token of ["help", "--help", "-h"]) { + test(`review-round ${token} prints usage and exits 0`, () => { + const args = parseReviewRoundCliArgs([token], CWD); + assert.ok(!("error" in args), `${token} must not be an unknown verb`); + const r = runReviewRoundCli(args as never); + assert.equal(r.code, 0); + assert.match(r.output, /Usage:/); + assert.match(r.output, /open --session/); + assert.match(r.output, /abort --session/); + }); + + test(`plan ${token} prints usage and exits 0`, () => { + const args = parsePlanCliArgs([token], CWD); + assert.ok(!("error" in args), `${token} must not be an unknown verb`); + const r = runPlanCli(args as never); + assert.equal(r.code, 0); + assert.match(r.output, /Usage:/); + assert.match(r.output, /--slug/); + }); + + test(`metric ${token} prints usage without demanding --session`, () => { + const r = runMetricCli([token], CWD); + assert.equal(r.code, 0); + assert.match(r.output, /Usage:/); + // The session guard used to fire first, making the usage unreachable. + assert.doesNotMatch(r.output, /--session is required/); + }); + + test(`divergence ${token} prints usage without demanding --session`, () => { + const r = runDivergenceCli([token], CWD); + assert.equal(r.code, 0); + assert.match(r.output, /Usage:/); + assert.doesNotMatch(r.output, /--session is required/); + }); +} + +// The freeze case is the one that mattered most: --help was not merely rejected, +// it fell through to the real run and WROTE the manifest, exiting 0 so nothing +// signalled the mutation. The assertion is therefore on the filesystem, not the +// wording — a help text that still writes would pass a text-only check. +test("freeze --help prints usage and writes nothing", () => { + const cwd = mkdtempSync(join(tmpdir(), "codexclaw-freeze-help-")); + const args = parseFreezeArgs(["--help", "--cwd", cwd, "--session", "s1"]); + const out = runFreeze(args); + assert.match(out, /Usage:/); + assert.equal(existsSync(join(cwd, ".codexclaw", "interview", "freeze.json")), false); +}); + diff --git a/plugins/codexclaw/components/pabcd-state/test/hook.test.ts b/plugins/codexclaw/components/pabcd-state/test/hook.test.ts index d01a2f50..87b1d7bd 100644 --- a/plugins/codexclaw/components/pabcd-state/test/hook.test.ts +++ b/plugins/codexclaw/components/pabcd-state/test/hook.test.ts @@ -243,7 +243,10 @@ test("posix arming directive is byte-identical to its pinned snapshot", () => { " workPhases[] + criteria[] in the goalplan -> `cxc orchestrate P --session `.", " HITL (no such ask): enter the cycle explicitly via `cxc orchestrate I|P --session `.", "4. Advance EVERY forward edge yourself with `cxc orchestrate --attest ` —", + ` e.g. \`cxc orchestrate A --session --attest '{\"from\":\"P\",\"to\":\"A\",\"did\":\"...\",\"planUnit\":\"devlog/_plan/YYMMDD_slug\",\"workPhaseId\":\"wp1\"}'\` —`, " a phase without its persisted transition + artifact did not happen (ORCH-ARTIFACT-01).", + ' EVERY attest carries "from" and "to" naming the edge: they are coerced before any', + " gate runs, so omitting them is refused on every edge (ATTEST-SHAPE-01).", " When a goalplan is bound, include the active workPhaseId in every gated attest", " (one work-phase = one full PABCD cycle).", "5. After D closes to IDLE with work remaining under an active goal, immediately re-enter", diff --git a/plugins/codexclaw/components/pabcd-state/test/interview-readiness.test.ts b/plugins/codexclaw/components/pabcd-state/test/interview-readiness.test.ts new file mode 100644 index 00000000..60c78700 --- /dev/null +++ b/plugins/codexclaw/components/pabcd-state/test/interview-readiness.test.ts @@ -0,0 +1,165 @@ +/** + * interview-readiness.test.ts — 260825. + * + * isInterviewReady demanded level "max" on all four dimensions, and no shipped + * writer could produce it: deriveLevel tops out at "high" and `--dim =max` is + * rejected. Every interview therefore dead-ended or spent an attested override, + * which made the override's ledger row meaningless — it recorded "bypassed the + * gate" for the thorough interview and the skipped one alike. + * + * Accepting "high" alone would have been the opposite failure. Measured before + * the fix was written: four `--known` flags reach all-high in one command. So + * the gate now asks the append-only Q&A ledger where a "high" came from. + * + * The load-bearing test here is the trivial-path one. Without it the change is + * indistinguishable from simply lowering the bar. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { parseScanCliArgs, runScanCli } from "../src/scan-cli.ts"; +import { evaluateInterviewGate, isInterviewReady, DIMENSIONS } from "../src/interview.ts"; +import { dimensionsBackedByAnswers, captureInterviewAnswers } from "../src/interview-ledger.ts"; +import { readState, writeState, defaultState } from "../src/state.ts"; + +function freshSession(): string { + const cwd = mkdtempSync(join(tmpdir(), "codexclaw-ivready-")); + writeState(cwd, { ...defaultState("s1"), phase: "I" }); + return cwd; +} + +/** Drive the REAL capture writer: the transport shape request_user_input produces. */ +function askAndAnswer(cwd: string, questionId: string, dimension: string, answer: string): void { + captureInterviewAnswers({ + cwd, + sessionId: "s1", + turnId: "t1", + toolInput: JSON.stringify({ questions: [{ id: questionId, header: "h", question: `what about ${dimension}?`, options: [] }] }), + toolResponse: JSON.stringify({ answers: { [questionId]: { answers: [answer] } } }), + }); +} + +function scan(cwd: string, argv: string[]): { output: string; code: number } { + const args = parseScanCliArgs(["record", "--session", "s1", "--cwd", cwd, ...argv], cwd); + assert.ok(!("error" in args), `scan args rejected: ${(args as { error?: string }).error}`); + return runScanCli(args as never); +} + +function gateOf(cwd: string) { + const st = readState(cwd, "s1"); + return { + tracker: st.interview, + gate: evaluateInterviewGate(st.interview, { backedDimensions: dimensionsBackedByAnswers(cwd, "s1") }), + }; +} + +const MAPPED = ["q1=goal", "q2=constraint", "q3=success", "q4=ontology"]; +const PAIRS: Array<[string, string]> = [["q1", "goal"], ["q2", "constraint"], ["q3", "success"], ["q4", "ontology"]]; + +test("a real interview reaches ready with NO override", () => { + const cwd = freshSession(); + for (const [qid, dim] of PAIRS) askAndAnswer(cwd, qid, dim, `the user's answer about ${dim}`); + scan(cwd, ["--derive", ...MAPPED.flatMap((m) => ["--map", m])]); + + const { tracker, gate } = gateOf(cwd); + for (const d of DIMENSIONS) assert.equal(tracker.dimensions[d].level, "high", `${d} derives to high`); + assert.equal(gate.ready, true, "an answered, mapped, scanned interview is ready"); + assert.deepEqual(gate.warnings, []); +}); + +test("the trivial path does NOT reach ready: four --known flags are not an interview", () => { + const cwd = freshSession(); + // The exact command measured while designing the fix. + scan(cwd, ["--known", "goal=x", "--known", "constraint=x", "--known", "success=x", "--known", "ontology=x"]); + + const { tracker, gate } = gateOf(cwd); + // The SHAPE is satisfied — this is precisely why shape alone cannot be the gate. + assert.equal(isInterviewReady(tracker), true, "all four reach high, so the shape check passes"); + assert.equal(gate.ready, false, "but no dimension traces to an answered question"); + assert.match(gate.warnings.join(" "), /without an answered question in the interview ledger/); +}); + +test("an unanswered question leaves its dimension at mid and blocks", () => { + const cwd = freshSession(); + for (const [qid, dim] of PAIRS.slice(0, 3)) askAndAnswer(cwd, qid, dim, "answered"); + // q4 asked, never answered. + captureInterviewAnswers({ + cwd, sessionId: "s1", turnId: "t1", + toolInput: JSON.stringify({ questions: [{ id: "q4", header: "h", question: "what about ontology?", options: [] }] }), + toolResponse: JSON.stringify({ answers: {} }), + }); + scan(cwd, ["--derive", ...MAPPED.flatMap((m) => ["--map", m])]); + + const { tracker, gate } = gateOf(cwd); + assert.equal(tracker.dimensions.ontology.level, "mid", "an open gap pins the dimension at mid"); + assert.equal(gate.ready, false); +}); + +test("--known on top of a derived dimension does not un-derive it", () => { + const cwd = freshSession(); + for (const [qid, dim] of PAIRS) askAndAnswer(cwd, qid, dim, "answered"); + scan(cwd, ["--derive", ...MAPPED.flatMap((m) => ["--map", m])]); + scan(cwd, ["--known", "goal=an extra fact recorded later"]); + + const { gate } = gateOf(cwd); + assert.equal(gate.ready, true, "adding a fact is additive, not destructive"); +}); + +test("--known cannot lend provenance to a dimension that never had a question", () => { + const cwd = freshSession(); + for (const [qid, dim] of PAIRS.slice(0, 3)) askAndAnswer(cwd, qid, dim, "answered"); + scan(cwd, ["--derive", "--map", "q1=goal", "--map", "q2=constraint", "--map", "q3=success"]); + // ontology gets a typed fact only. + scan(cwd, ["--known", "ontology=asserted without ever asking"]); + + const { tracker, gate } = gateOf(cwd); + assert.equal(tracker.dimensions.ontology.level, "high", "the typed fact still reaches high"); + assert.equal(gate.ready, false, "but high without an answer is not readiness"); + assert.match(gate.warnings.join(" "), /ontology reached "high"/); +}); + +test("max still satisfies readiness without ledger backing", () => { + const cwd = freshSession(); + scan(cwd, ["--known", "goal=x"]); + const st = readState(cwd, "s1"); + const tracker = { ...st.interview }; + for (const d of DIMENSIONS) tracker.dimensions[d] = { level: "max", known: ["k"], unknown: [], confidence: 1 }; + writeState(cwd, { ...st, interview: tracker }); + + const { gate } = gateOf(cwd); + assert.equal(gate.ready, true, "max is a deliberate assertion, not a derived level"); +}); + +test("the untouched conditions still block a fully derived interview", () => { + const cwd = freshSession(); + for (const [qid, dim] of PAIRS) askAndAnswer(cwd, qid, dim, "answered"); + scan(cwd, ["--derive", ...MAPPED.flatMap((m) => ["--map", m])]); + assert.equal(gateOf(cwd).gate.ready, true, "baseline for this test"); + + const st = readState(cwd, "s1"); + writeState(cwd, { + ...st, + interview: { ...st.interview, contradictions: [{ contradictionId: "c1", severity: "high", summary: "conflict" }] }, + }); + assert.equal(gateOf(cwd).gate.ready, false, "one contradiction still blocks"); + + const st2 = readState(cwd, "s1"); + writeState(cwd, { + ...st2, + interview: { ...st2.interview, contradictions: [], assumptions: [{ id: "a1", text: "unrecorded", recorded: false }] }, + }); + assert.equal(gateOf(cwd).gate.ready, false, "one unrecorded assumption still blocks"); +}); + +test("without ledger evidence the gate degrades to shape, and says nothing false", () => { + const cwd = freshSession(); + for (const [qid, dim] of PAIRS) askAndAnswer(cwd, qid, dim, "answered"); + scan(cwd, ["--derive", ...MAPPED.flatMap((m) => ["--map", m])]); + + // The human free-pass path has no cwd, so it calls the gate without evidence. + const shapeOnly = evaluateInterviewGate(readState(cwd, "s1").interview); + assert.equal(shapeOnly.ready, true, "shape-only stays the old contract for the human path"); +}); + diff --git a/plugins/codexclaw/components/pabcd-state/test/interview.test.ts b/plugins/codexclaw/components/pabcd-state/test/interview.test.ts index 96281bfe..aaeb28fa 100644 --- a/plugins/codexclaw/components/pabcd-state/test/interview.test.ts +++ b/plugins/codexclaw/components/pabcd-state/test/interview.test.ts @@ -27,11 +27,20 @@ test("defaultInterview is not ready (all dimensions low)", () => { assert.equal(isInterviewReady(defaultInterview()), false); }); -test("isInterviewReady: true only for all-max + no contradictions + recorded assumptions", () => { +test("isInterviewReady: high or max, no contradictions, recorded assumptions", () => { const t = readyTracker(); assert.equal(isInterviewReady(t), true); - // any non-max dimension -> false + // 260825: "high" now satisfies the SHAPE half. It used to be rejected here, + // which made readiness unreachable — deriveLevel tops out at "high" and + // `--dim =max` is refused, so no shipped writer could produce the level + // this test demanded. Provenance moved to evaluateInterviewGate, which asks + // the interview ledger where a "high" came from (interview-readiness.test.ts). t.dimensions.goal.level = "high"; + assert.equal(isInterviewReady(t), true, "a derived level satisfies the shape check"); + // Anything below "high" still means open gaps, and still blocks. + t.dimensions.goal.level = "mid"; + assert.equal(isInterviewReady(t), false, "mid means unanswered questions remain"); + t.dimensions.goal.level = "low"; assert.equal(isInterviewReady(t), false); }); diff --git a/plugins/codexclaw/components/pabcd-state/test/plan-cli.test.ts b/plugins/codexclaw/components/pabcd-state/test/plan-cli.test.ts index 4aaafebd..14221fa2 100644 --- a/plugins/codexclaw/components/pabcd-state/test/plan-cli.test.ts +++ b/plugins/codexclaw/components/pabcd-state/test/plan-cli.test.ts @@ -14,7 +14,11 @@ function planRoot(cwd: string): string { } test("plan-cli parse: init requires slug; --phases bounds enforced; slug normalized", () => { - assert.match((parsePlanCliArgs([], "/tmp") as { error: string }).error, /unknown plan verb/); + // 260825 wp1: a bare `cxc plan` is now help, not an error — the top-level help + // points agents at ` --help` and that pointer used to hit a rejection. + // An unknown verb is still an error; empty argv is not. + assert.equal((parsePlanCliArgs([], "/tmp") as { verb: string }).verb, "help"); + assert.match((parsePlanCliArgs(["nope"], "/tmp") as { error: string }).error, /unknown plan verb/); assert.match((parsePlanCliArgs(["init"], "/tmp") as { error: string }).error, /requires a /); assert.match((parsePlanCliArgs(["init", "x", "--phases", "0"], "/tmp") as { error: string }).error, /1-9/); const ok = parsePlanCliArgs(["init", "My Big Feature!", "--phases", "3"], "/tmp"); diff --git a/plugins/codexclaw/components/pabcd-state/test/scan-cli.test.ts b/plugins/codexclaw/components/pabcd-state/test/scan-cli.test.ts index cd4b0941..f142479b 100644 --- a/plugins/codexclaw/components/pabcd-state/test/scan-cli.test.ts +++ b/plugins/codexclaw/components/pabcd-state/test/scan-cli.test.ts @@ -270,10 +270,13 @@ test("scan record: --derive never hands out max, and an explicit --dim wins", () toolResponse: { answers: { q1: { answers: ["A"] } } }, }); run(cwd, ["record", "--session", "s-max", "--derive", "--map", "q1=goal"]); - // "max" gates I->P via isInterviewReady, so a heuristic must never grant it. + // "max" satisfies readiness with no ledger backing, so a heuristic must never + // grant it. Derivation tops out at "high", which the I->P gate then checks + // against the answer ledger (interview-readiness.test.ts). assert.equal(readState(cwd, "s-max").interview?.dimensions.goal.level, "high"); - // Nor can an operator flag grant it: that would bypass the attested override. + // Nor can an operator flag grant it: that would be readiness with no evidence + // and no attestation. const denied = parseScanCliArgs(["record", "--session", "s-max", "--dim", "goal=max"], cwd); assert.ok("error" in denied); diff --git a/plugins/codexclaw/components/provider-bridge/package.json b/plugins/codexclaw/components/provider-bridge/package.json index 824a0962..60831b15 100644 --- a/plugins/codexclaw/components/provider-bridge/package.json +++ b/plugins/codexclaw/components/provider-bridge/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/provider-bridge", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "Detect-only opencodex (ocx) status probe at session start; graceful native path when absent.", diff --git a/plugins/codexclaw/components/recall/package.json b/plugins/codexclaw/components/recall/package.json index 67072dcc..94d1b383 100644 --- a/plugins/codexclaw/components/recall/package.json +++ b/plugins/codexclaw/components/recall/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/recall", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "Read-only chat/memory recall search over the Codex session root (~/.codex): date-pruned rollout scan + thread/memory sqlite enrichment.", diff --git a/plugins/codexclaw/components/skill-search/package.json b/plugins/codexclaw/components/skill-search/package.json index 54012606..d0ba0f93 100644 --- a/plugins/codexclaw/components/skill-search/package.json +++ b/plugins/codexclaw/components/skill-search/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/skill-search", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "Remote dormant-skill search over cli-jaw-skills / Hermes / ClawHub / gh code search. Zero-dep, TTL-cached, adapter-preamble output. No local vendoring.", diff --git a/plugins/codexclaw/components/subagent-config/package.json b/plugins/codexclaw/components/subagent-config/package.json index 8beface2..517b1415 100644 --- a/plugins/codexclaw/components/subagent-config/package.json +++ b/plugins/codexclaw/components/subagent-config/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/subagent-config", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "Stores subagent model/prompt config; serves it to the GUI and an MCP tool.", diff --git a/plugins/codexclaw/components/subagent-config/test/mcp.test.ts b/plugins/codexclaw/components/subagent-config/test/mcp.test.ts index eb9138e5..4eb57a58 100644 --- a/plugins/codexclaw/components/subagent-config/test/mcp.test.ts +++ b/plugins/codexclaw/components/subagent-config/test/mcp.test.ts @@ -23,10 +23,17 @@ async function collect(cwd: string, messages: unknown[], expectedReplies: number const child = spawn(process.execPath, [serverJs], { cwd, stdio: ["pipe", "pipe", "inherit"] }); const out: any[] = []; let buf = ""; - // G23: the MCP stdio roundtrip can exceed a tight budget when this file runs - // alongside the rest of the suite (parallel node:test workers contend for CPU, - // and a cold spawn pays the type-strip cost). 8s was flaky under that load; 30s - // is still a real failure ceiling but absorbs scheduling jitter. + // G23 / C10. This ceiling is a HANG detector, not a flake absorber: the + // assertion is "the server answered", and any real answer arrives in + // milliseconds. The value is generous because a cold spawn under a loaded + // suite pays a type-strip cost, so a tight budget would fail on scheduling + // jitter rather than on the behavior under test. + // + // Naming that honestly matters, because raising a timeout until a test passes + // is exactly what TEST-FLAKE-RERUN-01 forbids. The underlying contention is + // NOT fixed here and is tracked as C10 in structure/30_contradiction_register.md; + // the honest fixes are build/test serialization or removing the real-process + // dependency from this assertion. const MCP_STDIO_TIMEOUT_MS = 30000; const timer = setTimeout(() => { child.kill(); diff --git a/plugins/codexclaw/gui/package.json b/plugins/codexclaw/gui/package.json index c01c6131..61e90620 100644 --- a/plugins/codexclaw/gui/package.json +++ b/plugins/codexclaw/gui/package.json @@ -1,6 +1,6 @@ { "name": "@codexclaw/gui", - "version": "0.2.12", + "version": "0.2.13", "private": true, "type": "module", "description": "codexclaw local dashboard (Vite + React) — subagent config, prompts, provider link bar.", diff --git a/plugins/codexclaw/inventory.json b/plugins/codexclaw/inventory.json index 3ac8184d..2080ec2d 100644 --- a/plugins/codexclaw/inventory.json +++ b/plugins/codexclaw/inventory.json @@ -2,8 +2,8 @@ "schemaVersion": 1, "plugin": { "name": "codexclaw", - "manifestVersion": "0.2.12+codex.20260818121334", - "packageVersion": "0.2.12" + "manifestVersion": "0.2.13+codex.260825074116", + "packageVersion": "0.2.13" }, "skills": [ { @@ -257,49 +257,49 @@ { "folder": "config-guard", "packageName": "@codexclaw/config-guard", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "cxc-ops", "packageName": "@codexclaw/cxc-ops", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "messenger-bridge", "packageName": "@codexclaw/messenger-bridge", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "pabcd-state", "packageName": "@codexclaw/pabcd-state", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "provider-bridge", "packageName": "@codexclaw/provider-bridge", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "recall", "packageName": "@codexclaw/recall", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "skill-search", "packageName": "@codexclaw/skill-search", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true }, { "folder": "subagent-config", "packageName": "@codexclaw/subagent-config", - "version": "0.2.12", + "version": "0.2.13", "hasTests": true } ] diff --git a/plugins/codexclaw/skills/dev-debugging/SKILL.md b/plugins/codexclaw/skills/dev-debugging/SKILL.md index ab932e72..35e07919 100644 --- a/plugins/codexclaw/skills/dev-debugging/SKILL.md +++ b/plugins/codexclaw/skills/dev-debugging/SKILL.md @@ -310,6 +310,10 @@ Root cause pattern: List endpoint lazy-loads related records per item (1 query + Root cause pattern: Test passes in isolation but fails in suite due to shared mutable state (database rows, global variables, uncleared mocks). Compare with stable tests that use transaction rollback in beforeEach/afterEach. Fix: add proper test isolation, then search for other tests missing cleanup. +Policy — what CI may do about a flake, when quarantine is permitted, and what +counts as closing one — is `dev-testing` `references/ci-pipeline.md` §5 +(`TEST-FLAKE-*`). This skill owns the diagnosis; that file owns the disposition. + --- ## When to Escalate vs When to Keep Digging diff --git a/plugins/codexclaw/skills/dev-devops/SKILL.md b/plugins/codexclaw/skills/dev-devops/SKILL.md index 0ecf2fc8..d1980c8d 100644 --- a/plugins/codexclaw/skills/dev-devops/SKILL.md +++ b/plugins/codexclaw/skills/dev-devops/SKILL.md @@ -163,6 +163,46 @@ jobs: --- +### §2.8 Freeze & GO/NO-GO Gates (STRICT) + +`DEVOPS-RELEASE-PROOF-01` governs the proof bundle for an artifact you already +published. This section governs the decision to publish at all — the readiness +report, and the gates it claims to have passed. + +Sources: the OpenCodex v2.32.1 hotfix train and the operator-visibility train +that followed it (`devlog/_plan/260824_v2_32_1_hotfix_train/`, +`devlog/_plan/260825_operator_visibility_train/`). A freeze audit rejected the +first GO report there on three counts — unresolved review threads on merged PRs, +a red gate argued into an exception, and missing frozen-head receipts +(`900_go_nogo_readiness_report.md`:37-49). + +| Rule | Severity | Statement | +|------|----------|-----------| +| `DEVOPS-FREEZE-SHA-01` | STRICT | Pin the readiness report to the code SHA its gates describe. If the head moved after the freeze, prove the delta is docs-only (`git diff --name-only `) and keep every gate receipt on the freeze SHA. | +| `DEVOPS-GATE-WEAKEN-01` | STRICT | A red named gate is never excused inside the report that gate failed. Make the original command green, or replace it with a pre-declared equivalent CI actually runs — and declare the swap **before** the verdict, not after the failure. | +| `DEVOPS-REVIEW-THREADS-01` | STRICT | Unresolved review threads on merged PRs are a GO blocker. Count them **after** merge: a thread opened minutes before merge still counts until it is fixed or explicitly dismissed. | +| `DEVOPS-GATE-OWNER-01` | STRICT | A mandatory GO gate needs an implementing work-phase and a recorded terminal outcome — pass, not-reproduced, or explicitly deregistered. A gate nobody implements is not a gate; it is a wish. | + +Per-rule sources: FREEZE-SHA `900`:3-7; GATE-WEAKEN `900`:47-49; REVIEW-THREADS +`900`:40-46 and `080_wp8`:47; GATE-OWNER `090_wp9`:6-8 and +`000_baseline_scope_and_roadmap.md`:243-245. + +**Why gate-weakening is the load-bearing rule.** In the case that produced it the +suite was red, the red tests were known to be load-sensitive, and the fix was +real — so the report explained the exception. The audit rejected that, correctly: +an exception argued *after* a gate fails is indistinguishable from an exception +argued *because* it failed. The honest move was to decompose the gate to match +what CI actually runs (`DEVOPS-SUITE-PARTITION-01`), which turned the general +suite green. One local `api-usage` failure remained and was waived separately, +on the `DEVOPS-BASELINE-DEFECT-01` triple — not by the partitioning. + +Operational mechanics — suite partitioning, baseline-versus-defect attribution, +instrument stability, and exact-head evidence — live in +`references/ci-cd-deploy.md` §6. Runtime and operator-signal evidence rules live +in `references/sre-foundations.md` §7. + +--- + ## §3 Kubernetes Basics ### §3.1 Minimum Viable K8s (DEFAULT) diff --git a/plugins/codexclaw/skills/dev-devops/references/ci-cd-deploy.md b/plugins/codexclaw/skills/dev-devops/references/ci-cd-deploy.md index 6a8772d8..9b755ab8 100644 --- a/plugins/codexclaw/skills/dev-devops/references/ci-cd-deploy.md +++ b/plugins/codexclaw/skills/dev-devops/references/ci-cd-deploy.md @@ -256,3 +256,85 @@ Phase 4: Drop old column | No prod approval | Unreviewed changes hit users | Environment protection | | `down()` migrations | Rollback breaks forward-deployed code | Expand-contract | | Deploy without smoke test | Silent failures | Post-deploy smoke in pipeline | +| Full-suite gate run as one process | Not the gate CI applies | `DEVOPS-SUITE-PARTITION-01` §6.1 | +| Red test waved as "environmental" | The most common way a real defect ships | `DEVOPS-BASELINE-DEFECT-01` §6.2 | +| New runner flags landed during a freeze | Red becomes indistinguishable from noise | `DEVOPS-VERIFY-INSTRUMENT-01` §6.3 | +| "It was green when I checked" | The head moved under you | `DEVOPS-EXACT-HEAD-01` §6.4 | + +--- + +## §6 Verification Evidence Rules + +Owner: this file. The GO/NO-GO decision rules that consume these live in +`dev-devops` SKILL.md §2.8. Sources are cited to the OpenCodex trains that +produced them, because each was learned by getting it wrong first. + +### §6.1 Suite partitioning (`DEVOPS-SUITE-PARTITION-01`, STRICT) + +A local one-process full-suite run is **not** the CI suite gate. Replay the +partition CI actually applies — the general shards plus each segregated job's +exact command — and record both forms. + +Why: OpenCodex excludes three load-sensitive path patterns from its general +batches (`scripts/ci/run-bun-test-batches.sh`:50) and covers them with two +dedicated jobs — `storage policy` runs six files together, `api usage` runs one +(`.github/workflows/ci.yml`). Running everything in one process therefore fails +tests that CI never runs together, and the resulting red is not the gate's +verdict. Decomposed, the same tree gave 14565 pass / 0 fail on the general suite +and 9/0 on the storage-policy job +(`260824_v2_32_1_hotfix_train/900_go_nogo_readiness_report.md`:54-58). + +Corollary: if you cannot state which CI job a local command corresponds to, that +command is not a gate. + +### §6.2 Baseline versus defect (`DEVOPS-BASELINE-DEFECT-01`, STRICT) + +A local red test is a **candidate defect** until all three hold: + +1. the identical failure reproduces on the untouched pre-change baseline SHA, +2. no merged unit in the change set touches that code, and +3. CI's matching job is green at the freeze SHA. + +Two out of three is not evidence. "It's flaky" and "it's environmental" are +conclusions, not observations, and they need the same proof as every other claim. +Remediation of a genuine flake is `dev-testing` `references/ci-pipeline.md` §5. + +Source: `260824_v2_32_1_hotfix_train/900_go_nogo_readiness_report.md`:69-72, where +a local `api-usage` failure was waived only after all three legs were shown. + +### §6.3 Instrument stability (`DEVOPS-VERIFY-INSTRUMENT-01`, STRICT) + +Do not change the verification instrument — runner flags, parallelism, shard +layout, timeouts, retry policy — while using it to certify a freeze. Change it +against a known-good baseline, or defer it. + +Stated at the source as: *a verification instrument gets changed against a +known-good baseline; it does not get used to establish one* +(`260824_v2_32_1_hotfix_train/070_wp2_pr2427_parallel_test_runner.md`:44-45). +That train deferred its parallel-runner PR for exactly this reason: across five +recorded runs, four different tests flaked, and the freeze gate was itself a +full-suite run — so landing the new runner would have made red indistinguishable +from noise (`070`:78-121). + +### §6.4 Exact-head evidence (`DEVOPS-EXACT-HEAD-01`, STRICT) + +Re-read the PR or branch head immediately before claiming exact-head evidence. A +contributor push mid-verification makes a recorded SHA stale; keep stale rows +labeled stale and never merge on them. A remembered pass is not evidence. + +Source: `070_wp2_pr2427_parallel_test_runner.md`:136-139 — a MERGE recommendation +was formed against a head the author had already replaced, and the run table keeps +the superseded rows explicitly marked stale (`070`:87-98). + +### §6.5 Stability counting (`DEVOPS-FLAKE-STABILITY-01`, DEFAULT) + +A flaky-capable suite is stable only after N consecutive greens at **one** head +plus the required CI matrix. Declare N in the GO report; OpenCodex set its bar at +N=3 plus Linux and Windows CI +(`260824_v2_32_1_hotfix_train/070_wp2_pr2427_parallel_test_runner.md`:125-126) — +and never collected it, which is why that PR was deferred rather than landed. +One green run is not a land signal: at the head under test the recorded runs were +green, green, then a failure (`070`:87-98). + +This rule counts greens. It does not tell you how to fix a flake: that is +`dev-testing` `references/ci-pipeline.md` §5, which owns `TEST-FLAKE-*`. diff --git a/plugins/codexclaw/skills/dev-devops/references/sre-foundations.md b/plugins/codexclaw/skills/dev-devops/references/sre-foundations.md index 949c66c3..6d329b83 100644 --- a/plugins/codexclaw/skills/dev-devops/references/sre-foundations.md +++ b/plugins/codexclaw/skills/dev-devops/references/sre-foundations.md @@ -264,3 +264,52 @@ N+M Root cause identified → permanent fix planned | Blame individuals | Engineers hide mistakes, no systemic improvement | Blameless postmortem culture | | No error budget policy | SLO violations have no defined response | Define 3-stage policy | | Postmortem without action items | Theater; same incidents repeat | Specific, owned, tracked items | +| Measuring a long-running process without proving what it is | You benchmarked the bug, not the fix | `DEVOPS-STALE-PROCESS-01` §7.1 | +| Flipping a true status bit to signal a missing one | The one honest signal now lies too | `DEVOPS-OBS-SIGNAL-01` §7.2 | + +--- + +## §7 Evidence & Operator Signals + +### §7.1 Process identity (`DEVOPS-STALE-PROCESS-01`, STRICT) + +A live long-running process is not candidate evidence until its start time, +binary, and config are proven to match the build under test. + +This exists because an OpenCodex train had a 100-call live canary as a GO +criterion, and the process on the expected port turned out to be PID 922, started +two days earlier: the reporter's own pre-fix proxy +(`260824_v2_32_1_hotfix_train/090_wp9_issue2472_mixed_sequence_regression.md`:21-23). +The canary was rejected as the wrong instrument BEFORE it was run, and the phase +substituted a deterministic mixed-sequence regression +(`080_wp8_freeze_verification_and_go_nogo.md`:24). The lesson is the identity +check, not a bad measurement — the measurement never happened, because the +identity check came first. + +Check, in this order: process start time versus the artifact's mtime, the +resolved binary path versus the one you built, and the config the process +actually loaded versus the one on disk. A matching port and a healthy `/healthz` +prove neither. + +Independently learned in this project's own history: a cli-jaw deployment proof +required comparing process start time against the installed `dist` mtime, because +a matching version string and a green health endpoint were both satisfied by a +process running the previous build. + +### §7.2 Operator signals (`DEVOPS-OBS-SIGNAL-01`, DEFAULT) + +When an operator surface is missing a signal, **add the missing signal**. Never +flip an already-true status bit to compensate. + +The instructive case: a status command showed a green proxy line, and users in a +degraded routing state complained it was misleading. It was not — the proxy was +up and answering `/healthz`, and the reporter proved that himself by curling it. +Turning that line yellow would have made the one honest signal lie in order to +cover for one that was never emitted. The fix is a second line, not a corrupted +first one +(`260825_operator_visibility_train/020_wp3_issue2411_status_routing_visibility.md`:16-22). + +The negative form of the same rule: a degraded, ineligible, or skipped verdict +that can reach an operator command must carry a message. A silent `ineligible` +is how `start`, `ensure`, and `repair` all reported success while doing nothing +(`260825_operator_visibility_train/001_current_state_inventory.md`:97-118). diff --git a/plugins/codexclaw/skills/dev-testing/SKILL.md b/plugins/codexclaw/skills/dev-testing/SKILL.md index 23a76411..1ee30a08 100644 --- a/plugins/codexclaw/skills/dev-testing/SKILL.md +++ b/plugins/codexclaw/skills/dev-testing/SKILL.md @@ -74,7 +74,7 @@ source-fetch and evidence-status rules. - Use factories / builders for setup; avoid repeated inline blobs. - A fast real dependency beats a mock. A mock beats an untested branch. - If the failure is mysterious, **delegate methodology to `dev-debugging`**, then return here for the regression harness. -- **STRICT (TEST-ANTI-FLAKE-01):** A time-based flake is a bug. Do not use sleep-based synchronization, retry-as-fix, or green-on-retry acceptance without a deterministic cause and harness correction. +- **STRICT (TEST-ANTI-FLAKE-01):** A time-based flake is a bug. Do not use sleep-based synchronization, retry-as-fix, or green-on-retry acceptance without a deterministic cause and harness correction. Full policy: `references/ci-pipeline.md` §5 (`TEST-FLAKE-*`). - Verification depth follows `dev` §3 `DEV-VERIFY-FLOOR-01`; CRUD per-operation negative coverage is owned by `references/core/crud-test-matrix.md`. --- ## Limited-Oracle / Score-Objective Evaluation @@ -209,13 +209,11 @@ See `references/ci-pipeline.md` for job dependencies, concurrency, matrices, sha Playwright dependencies, and full GitHub Actions/GitLab CI templates. Matrix only across supported runtimes, required OS behavior, or suites exceeding CI budget. ### 5.4 Flaky Test Remediation -| Symptom | First Fix | -|---------|-----------| -| passes locally, fails in CI | deterministic seeds, containerized deps, explicit waits | -| order-dependent failure | reset shared state in fixtures | -| green on retry only | remove wall-clock / random assumptions | -| screenshot noise | stable CI image, mask dynamic regions | -Protocol: detect → quarantine if blocking → assign owner → reinstate after repeated green runs. +A flake is a defect, not a category of test. `TEST-FLAKE-ELIMINATE-01`, +`TEST-FLAKE-RERUN-01`, `TEST-FLAKE-QUARANTINE-01`, and +`TEST-FLAKE-ATTRIBUTION-01` are canonical in `references/ci-pipeline.md` §5 — +read it before treating any flake, including deciding whether one is +"environmental". ### 5.5 CI-Green Loop **STRICT (TEST-CI-GREEN-01):** Latest HEAD is the source of truth. Inspect the failing job and artifacts before editing, make the minimal correct fix, run local @@ -488,7 +486,7 @@ Red flags that trigger escalation: - Deleted assertions without replacement - Snapshot updates without visual/behavioral verification - Coverage exclusions added in the same PR as the fix -- `@skip` or `.skip()` added to failing tests +- `@skip` or `.skip()` added to failing tests — unless it is a `TEST-FLAKE-QUARANTINE-01` quarantine carrying all four required fields (`references/ci-pipeline.md` §5.3) - Threshold reductions (e.g., coverage 80% → 60%) - Type assertion suppressions (`as any`, `@ts-ignore`) in test files diff --git a/plugins/codexclaw/skills/dev-testing/references/ci-pipeline.md b/plugins/codexclaw/skills/dev-testing/references/ci-pipeline.md index 1b1eddfb..29266224 100644 --- a/plugins/codexclaw/skills/dev-testing/references/ci-pipeline.md +++ b/plugins/codexclaw/skills/dev-testing/references/ci-pipeline.md @@ -93,20 +93,95 @@ pytest -n auto --dist=loadgroup - keep contract reports separate from unit coverage - fail the build when thresholds or diff coverage drop -## 5. Flaky Test Quarantine Strategy - -1. detect the flaky test by exact name -2. move it to a quarantine tag or job -3. keep quarantine non-blocking but visible -4. assign an owner and removal deadline -5. restore only after repeated green runs - -| Signal | Action | -|--------|--------| -| intermittent timeout | replace implicit timing with deterministic waits | -| order-dependent failure | reset shared state or fixture leakage | -| CI-only HTTP failure | remove live network dependency | -| snapshot variance | stabilize fonts, time, locale, and dynamic regions | +## 5. Flaky Test Policy (canonical — `TEST-FLAKE-*`) + +Canonical owner: `dev-testing`. Other skills carry pointer stubs only (see +`dev` `references/skill-ownership.md`). `dev-debugging` owns the diagnostic +method; this section owns the policy — what CI may do about a flake, and what +counts as closing one. + +A flake is a defect that has not been diagnosed yet. Mechanisms that make CI +green without diagnosing it ship that defect. + +### 5.1 Eliminate the nondeterminism (`TEST-FLAKE-ELIMINATE-01`, STRICT) + +A flaky test is a defect in the test or in the code under test. Diagnose the +source of nondeterminism and remove it. **A flake is closed when the cause is +named, not when the suite is green.** + +This is a CLOSURE rule: it governs the claim "this flake is fixed", not the +question of whether work may proceed meanwhile. A quarantine under §5.3 does not +violate it, because a quarantine explicitly does not close the defect. + +| Signal | Cause to remove | +|--------|-----------------| +| intermittent timeout | implicit timing — wait on an observable condition or a fake clock | +| passes locally, fails in CI | an unpinned seed, an uncontainerized dependency, or an implicit wait | +| order-dependent failure | shared mutable state or fixture leakage between tests | +| CI-only HTTP failure | a live network dependency | +| snapshot variance | unpinned fonts, time, locale, or dynamic regions | +| passes alone, fails in the suite | resource contention or global state — isolate, then fix the sharing | + +After a fix, search for other tests with the same missing cleanup or the same +timing assumption. One cause commonly has siblings, and closing only the +instance that failed leaves the rest to fail later. + +### 5.2 Re-running is not a resolution (`TEST-FLAKE-RERUN-01`, STRICT) + +Re-running a failed job or test to obtain green is **not** a fix and is never +recorded as one. Raising a timeout until a test passes is the same violation +wearing a config change. + +A re-run is permitted for exactly one purpose: measuring the failure RATE as +diagnostic input. When you use it that way, write the measurement down — "4 of 5 +runs, 4 different tests" is evidence; "passed on retry" is not. + +This is the CI-facing half of `TEST-ANTI-FLAKE-01` (`dev-testing` SKILL.md §1.5) +and `TEST-CI-GREEN-01` (`dev-testing` SKILL.md §5.5). + +### 5.3 Quarantine is an exception with a cost (`TEST-FLAKE-QUARANTINE-01`, DEFAULT) + +Quarantine is permitted only when the flake blocks delivery of work that does not +depend on the code under test — state which delivery, and why it is independent — +AND all four of these are recorded in the same change (**STRICT**: the four +fields are not waivable; only the decision to defer is DEFAULT): + +1. the exact test name, +2. a named owner, +3. a removal deadline, and +4. the suspected cause. + +A quarantine without a deadline is a deletion with extra steps. Quarantine never +closes the defect — it defers it, and the deadline is the receipt. + +A quarantine carrying all four fields is the one form of `.skip()` that is not a +`TEST-PATCH-INTEGRITY-01` red flag (`dev-testing` SKILL.md §8). An undocumented +skip still is. + +### 5.4 "Environmental" is a claim, not an observation (`TEST-FLAKE-ATTRIBUTION-01`, DEFAULT) + +Before calling a failure environmental or pre-existing, prove it: + +1. the identical failure reproduces on the untouched baseline, +2. no change in the current set touches that code, and +3. the matching CI job is green at the same SHA. + +Without the triple it stays a candidate defect. This is the test-side mirror of +`DEVOPS-BASELINE-DEFECT-01` (`dev-devops` `references/ci-cd-deploy.md` §6.2), +and it is DEFAULT rather than STRICT for one reason only: step 3 sometimes needs +CI access an agent does not have. In that case record the gap. **A recorded gap +is not a waiver** — the failure remains a candidate defect, and the claim +"environmental" remains unmade. + +**When both rules apply, the STRICT one governs.** A release or freeze decision +is covered by `DEVOPS-BASELINE-DEFECT-01` (STRICT), so an agent cannot reach the +weaker class by loading only `dev-testing`. + +### 5.5 Counting greens + +How many consecutive green runs make a flaky-capable suite trustworthy is a +release-gate question, not a remediation one: `DEVOPS-FLAKE-STABILITY-01` in +`dev-devops` `references/ci-cd-deploy.md` §6.5. ## 6. Recommended Job Order diff --git a/plugins/codexclaw/skills/dev/references/skill-ownership.md b/plugins/codexclaw/skills/dev/references/skill-ownership.md index 1678b7b7..672bdb26 100644 --- a/plugins/codexclaw/skills/dev/references/skill-ownership.md +++ b/plugins/codexclaw/skills/dev/references/skill-ownership.md @@ -11,6 +11,7 @@ Each rule area has exactly one canonical owner. Other skills may contain stubs b | Pre-write search | `dev` §1.5 | `dev-code-reviewer` | | Stacked pull requests (`DEV-STACK-*`) | `dev` `references/stacked-prs.md` | `pabcd`, `loop`, `dev-code-reviewer`, `dev-devops` | | Edge-first testing | `dev-testing` §6 | — | +| Flaky tests / CI re-run (`TEST-FLAKE-*`) | `dev-testing` `references/ci-pipeline.md` §5 | `dev-testing` §5.4, `dev-debugging` Scenario D, `dev-devops` `references/ci-cd-deploy.md` §6.2/§6.5 | | Manual surface QA / evidence matrix | `cxc-qa` | `dev-testing` §4.6 (tool routing stays there) | | Test-induced defense | `dev-testing` §6.7 | `dev-code-reviewer` | | Boundary-only defense | `dev-architecture` §4 | `dev-backend`, `dev-security` | diff --git a/plugins/codexclaw/skills/interview/SKILL.md b/plugins/codexclaw/skills/interview/SKILL.md index d78dfeb9..b75aa6b5 100644 --- a/plugins/codexclaw/skills/interview/SKILL.md +++ b/plugins/codexclaw/skills/interview/SKILL.md @@ -58,12 +58,30 @@ The loop that prevents it: question. This is also what makes Mind routing adaptive: `selectMinds` ranks by dimension level, so with an empty tracker all four tie and it degrades to a fixed order. -`--dim =` records an explicit assertion when coverage alone -understates what you know. It deliberately cannot set `max`: that level gates I -> P through -`isInterviewReady`, and the sanctioned way past an unready interview is the attested -`cxc orchestrate P --attest-file ` carrying `{"override":true,...}`, which -leaves a ledger row. (The file flag is required on Windows: PowerShell cannot pass -inline JSON as a single argument.) +**Readiness is reached through step 2, not through an assertion.** A dimension counts +toward I -> P when the session's interview ledger shows a question that was ASKED, an +answer that was RECORDED, and a `--map` attributing that question to that dimension. +That is why `--map` matters: an answered question nobody attributed proves nothing about +any dimension. + +`--known =` records a fact you already hold. It moves a dimension off +`low` and can carry it to `high`, but it can NOT make it count for readiness — a typed +fact is not an answered question, and four `--known` flags would otherwise be a complete +interview in one command. + +`--dim =` records an explicit level assertion when coverage alone +understates what you know. It deliberately cannot set `max`: that level bypasses the ledger +check entirely, so it stays out of the writer's reach. + +When the interview genuinely is not complete, the sanctioned way past the gate is the attested +`cxc orchestrate P --attest-file ` carrying +`{"from":"I","to":"P","did":"","override":true}`, +which leaves a ledger row. It is the exception now, not the only door — until 260825 the gate +demanded a level no writer could produce, so every interview spent an override and the row +stopped distinguishing anything. (The file flag is required on Windows: PowerShell cannot pass +inline JSON as a single argument.) `from`/`to` are not optional here — the parser +coerces them before the override is ever read, so `{"override":true}` alone is +refused (ATTEST-SHAPE-01 in `cxc-pabcd`). ## Show the state before asking (INTERVIEW-RENDER-01) @@ -124,11 +142,16 @@ work-phase (loop-engineering §11.4). - Run a contradiction rescan after every answer, AND one final rescan before any proceed/close decision — surface what still remains. (This final rescan is process discipline; the runtime does not encode scan recency.) -- Runtime readiness predicate (`isInterviewReady`): all dimensions at `max` + contradictions - empty + assumptions recorded + `scanRounds >= 1`. Treat readiness as a coverage claim on top of - that: each dimension has concrete knowns, no unresolved unknown changes scope, and every - contradiction has exited into an answer or a recorded assumption. Summarize the remaining OPEN - ASSUMPTIONS before claiming I -> P readiness. +- Runtime readiness has two halves. **Shape** (`isInterviewReady`): every dimension at `high` or + `max` + contradictions empty + assumptions recorded + `scanRounds >= 1`. **Provenance** + (the I -> P gate on the agent CLI path): every dimension counted at `high` must trace to a + question that was asked, answered, and attributed with `--map`. `max` needs no ledger backing + because no writer can produce it. +- The practical consequence: `--known` alone never opens I -> P. Ask the question, let the + `PostToolUse` hook capture the answer, then `cxc scan record --derive --map =`. +- Treat readiness as a coverage claim on top of that: each dimension has concrete knowns, no + unresolved unknown changes scope, and every contradiction has exited into an answer or a + recorded assumption. Summarize the remaining OPEN ASSUMPTIONS before claiming I -> P readiness. ## Closeout fork (INTERVIEW-FORK-01) @@ -141,7 +164,9 @@ and INTERVIEW-INDEPENDENT-01 governs batching by independence rather than count. There is no build/execute path out of Interview — the only forward move is Plan, normally after the readiness gate passes, unless the human explicitly overrides (override is recorded as an audit entry); the agent CLI path also supports override via -an attest carrying `{"override":true}` with equivalent ledger transparency. +an attest carrying `{"from":"I","to":"P","did":"","override":true}` with +equivalent ledger transparency (`from`/`to` are coerced before the override is +read, so `{"override":true}` alone is refused — ATTEST-SHAPE-01). `proceed` means "advance to Plan", not permission to implement; the evolving plan/devlog stay draft interview artifacts until then. A chosen `proceed` executes as a real transition — `cxc orchestrate P --session ` (or the diff --git a/plugins/codexclaw/skills/loop/SKILL.md b/plugins/codexclaw/skills/loop/SKILL.md index 14e39a6c..cdd397f3 100644 --- a/plugins/codexclaw/skills/loop/SKILL.md +++ b/plugins/codexclaw/skills/loop/SKILL.md @@ -28,7 +28,11 @@ for EVERY loop entry or re-entry: `cxc orchestrate --attest ` — or `--attest-file `, which is REQUIRED on Windows because PowerShell cannot pass inline JSON as one argument — carrying the phase's real artifact - (ORCH-ARTIFACT-01). Entry edges (IDLE→P, I→P) are explicit commands without an + (ORCH-ARTIFACT-01). Every attest names the edge it advances with `from`/`to`, + plus that edge's own keys (`planUnit` on P>A, `workPhaseId` on every gated edge + under a bound goalplan, `testReceiptPath` on C>D) — canonical table and + copy-paste objects: `cxc-pabcd` §"Required attest keys" (ATTEST-SHAPE-01). + Entry edges (IDLE→P, I→P) are explicit commands without an attest JSON — the shipped gate (`dist/attest.js` GATED_TRANSITIONS) gates exactly those four. A phase without its persisted transition did not happen — the footer/ledger is the only proof of phase. diff --git a/plugins/codexclaw/skills/pabcd/SKILL.md b/plugins/codexclaw/skills/pabcd/SKILL.md index 35f32034..487f3143 100644 --- a/plugins/codexclaw/skills/pabcd/SKILL.md +++ b/plugins/codexclaw/skills/pabcd/SKILL.md @@ -88,14 +88,35 @@ sentence: plan/devlog paths, changed files, commands with exit codes, and eviden ledger paths when present. The runtime gate remains form-only for `did`; this is the agent discipline that makes later audit possible. +**ATTEST-SHAPE-01 (STRICT):** every `--attest` object carries `from` and `to` +naming the edge it advances. The parser coerces before any gate runs +(`attest.ts` `coerceAttest`), so an attest without them is refused on EVERY +edge — including ungated entry edges — before `did`, `planUnit`, or +`workPhaseId` is ever examined. + | Edge | Required attest keys | Notes | |------|---------------------|-------| -| IDLE->P | none (entry command) | | -| I->P | none (entry command) | | -| P->A | `did` with plan pointer | | -| A->B | `did`, `auditOutput`, `auditVerdict` (`pass`/`near-pass`/`fail`); near-pass adds `auditResidual` | FAIL never advances | -| B->C | `did` with implementation delta | | -| C->D | `did`, `checkOutput`, `exitCode` (required, must be 0) | | +| IDLE->P | none — pass no `--attest` at all | If you pass one anyway it is still parsed, so it still needs `from`/`to` | +| I->P | none — unless overriding an unready interview, which needs `from`, `to`, `did`, `override` | | +| P->A | `from`, `to`, `did` with plan pointer, `planUnit` | `planUnit` must be a real `devlog/_plan/YYMMDD_slug/` holding numbered docs | +| A->B | `from`, `to`, `did`, `auditOutput`, `auditVerdict` (`pass`/`near-pass`/`fail`); near-pass adds `auditResidual` | FAIL never advances | +| B->C | `from`, `to`, `did` with implementation delta | | +| C->D | `from`, `to`, `did`, `checkOutput`, `exitCode` (required, must be 0) | a goalplan-bound session also needs `testReceiptPath` from `cxc receipt test` | + +**Every gated edge additionally requires `workPhaseId` whenever a goalplan is +bound to the session** — it must equal the active work-phase (LOOP-UNIT-CHAIN-01). + +Copy-paste objects. Replace the values; keep every key: + +```json +{"from":"P","to":"A","did":"wrote the diff-level plan at ","planUnit":"devlog/_plan/260825_slug","workPhaseId":"wp1"} +{"from":"A","to":"B","did":"folded 2 blockers, rebutted 1","auditOutput":"","auditVerdict":"near-pass","auditResidual":"GO-WITH-FIXES; blocker 1 folded, blocker 2 rebutted because ...","workPhaseId":"wp1"} +{"from":"B","to":"C","did":"implemented ; tests added","workPhaseId":"wp1"} +{"from":"C","to":"D","did":"verified at ","checkOutput":"","exitCode":0,"testReceiptPath":".codexclaw/evidence//test-receipt.json","workPhaseId":"wp1"} +``` + +Omit `workPhaseId` when no goalplan is bound, and `testReceiptPath` when the +session is unbound. Everything else is mandatory on that edge. These are edge contracts, not substitutes for phase work. Artifact pointers must name the evidence produced by the phase being advanced. diff --git a/plugins/codexclaw/test/hook-e2e.test.mjs b/plugins/codexclaw/test/hook-e2e.test.mjs index f21c94df..f45d6ea5 100644 --- a/plugins/codexclaw/test/hook-e2e.test.mjs +++ b/plugins/codexclaw/test/hook-e2e.test.mjs @@ -68,7 +68,12 @@ function snapshotEntrypoint(distAbs) { rmSync(snapDir, { recursive: true, force: true }); sleepSync(50); } - return null; // dist never settled within budget; skip rather than flake + // dist never settled within budget. Returning null makes the caller skip, which + // is a PRECONDITION failure, not flake avoidance: without a settled dist there + // is nothing to exercise, so the test would assert on a build artifact it never + // saw. TEST-FLAKE-QUARANTINE-01 does not apply — this skips a missing fixture, + // not a failing assertion. + return null; } // Resolve a hook JSON's first command string to its absolute dist entrypoint plus diff --git a/structure/20_pabcd_dispatch_doctrine.md b/structure/20_pabcd_dispatch_doctrine.md index 34e292d3..b5c9651d 100644 --- a/structure/20_pabcd_dispatch_doctrine.md +++ b/structure/20_pabcd_dispatch_doctrine.md @@ -69,7 +69,11 @@ only blocks premature termination (it never transitions phase). Verified against cli-jaw's `attestation.ts` requires forward transitions to carry evidence; narration is rejected. codexclaw mirrors this as the one place the agent path is genuinely gated: -- Forward transitions (P->A->B->C->D) require `--attest '{"from","to","did"}'`. +- Forward transitions (P->A->B->C->D) require an attest object carrying at minimum + `{"from":"P","to":"A","did":"..."}` — `from` and `to` name the edge and are + coerced BEFORE any gate runs, so omitting them is refused on every edge + (ATTEST-SHAPE-01). Per-edge extras and copy-paste objects: `cxc-pabcd` + §"Required attest keys". - A->B additionally requires a pasted `auditOutput` (the dispatched reviewer subagent's verdict tail — WP3) plus `auditVerdict` (`pass|near-pass|fail`, the MAIN agent's own judgment; `near-pass` also needs `auditResidual` naming each residual blocker's diff --git a/structure/30_contradiction_register.md b/structure/30_contradiction_register.md index 6a3bd0b1..91ca25d4 100644 --- a/structure/30_contradiction_register.md +++ b/structure/30_contradiction_register.md @@ -82,7 +82,7 @@ not the status-token diff (a phrase-scan there false-positives on L9/L12 whose r | C7 | RESOLVED (L18) | ~~`structure/INDEX.md` (pre-fix) "manifest wires five hook JSON files" vs `plugin.json:20-26` declares six~~ | FIXED 2026-06-30: INDEX says "six"; **locked** by `gate.mjs checkCounts` (manifest `hooks[]` length == `hooks/*.json` count) with a negative-control test. Drift now fails `npm test`. | | C8 | RESOLVED (L19) | ~~build compiles every `src/*.ts` -> `dist/*.js` and `.gitignore:2` ignores `dist/`; several runtime `dist/*.js` that `bin`/`hook` load are untracked~~ | FIXED 2026-06-30: force-added the 4 runtime-reached untracked dist files (`pabcd-state/dist/{interview-ledger,orchestrate-cli,orchestrate-grammar}.js`, `subagent-config/dist/cli.js`); `packaging.test.mjs` walks the import graph from the 6 runtime entrypoints (5 cli.js + mcp.js) and FAILS `npm test` if any reached dist file is untracked. `gui/dist` (dev-server only) + `rescan-coordinator.js` (not runtime-reached) intentionally excluded. Follow-up: a src↔dist freshness test (build.test proves only post-build idempotency). | | C9 | RESOLVED (L18) | ~~component test surface looks uniform~~ | DOCUMENTED 2026-06-30: `structure/INDEX.md` Quality Gate section records that the root `package.json` `test` glob is the single source of test discovery and component packages intentionally do NOT each carry a local `test` script (the glob already covers `provider-bridge`/`subagent-config`). Intentional asymmetry, not drift. | -| C10 | LOW (flaky) | `subagent-config/test/mcp.test.ts:57` MCP stdio roundtrip assumed reliable | times out (~8s) when the full `npm test` runs concurrently with `npm run build` (process/IO contention). Single-file + standalone `npm test` runs are green (5/5, 332/332). Real but environmental; candidate for an explicit timeout or build/test serialization in the L18 gate work. | +| C10 | LOW (flaky) | `subagent-config/test/mcp.test.ts:57` MCP stdio roundtrip assumed reliable | timed out at the original 8s ceiling (now 30s, a hang detector rather than a flake absorber — see the comment at `subagent-config/test/mcp.test.ts:26`) when the full `npm test` runs concurrently with `npm run build` (process/IO contention). Single-file + standalone `npm test` runs are green (5/5, 332/332). **Disposition updated 2026-08-25:** the old note offered "an explicit timeout" as a candidate fix, which `TEST-FLAKE-RERUN-01` now forbids — raising a timeout to make a test pass is not a fix. The remaining honest options are build/test serialization (removing the contention) or removing the real-process dependency from the assertion. "Environmental" here also needs the `TEST-FLAKE-ATTRIBUTION-01` triple before it is treated as settled. Canonical policy: `dev-testing` `references/ci-pipeline.md` §5. | Cluster verdict: the dead-code rows (C1-C6) are mostly the *same* L14 story — wrappers, minds/triage/rescan helpers, and the goal-activation directive were built as tested pure