diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 672fc83..324b9c0 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -4,14 +4,14 @@ "owner": { "name": "Ljferrer", "url": "https://github.com/Ljferrer" }, "metadata": { "description": "WAR — Work·Audit·Refine: Claude-native multi-agent execution of a multi-phase implementation plan.", - "version": "0.14.64" + "version": "0.14.65" }, "plugins": [ { "name": "work-audit-refine", "source": "./", "description": "Execute a multi-phase implementation plan with fresh worker agents, independent read-only auditors, and a serial refine/merge queue — phase by phase, gated by you, opening one PR at the end.", - "version": "0.14.64", + "version": "0.14.65", "author": { "name": "Ljferrer" }, "keywords": ["agent-teams","workflows","orchestration","code-review","merge-queue","multi-agent","gastown"], "category": "workflow" diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 0989c33..d36f62e 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "work-audit-refine", "description": "WAR — Work·Audit·Refine. A Claude-native Agent-Teams + Workflows orchestration skill that executes a detailed multi-phase implementation plan: it breaks phases into GitHub issues, then per phase spins up fresh worker agents in isolated worktrees, independent read-only auditors (severity-gated, unanimous), and a serial refine/merge queue, landing each phase on a working branch and opening one PR to the landing branch at the end.", - "version": "0.14.64", + "version": "0.14.65", "author": { "name": "Ljferrer", "url": "https://github.com/Ljferrer" }, "license": "MIT", "homepage": "https://github.com/Ljferrer/WorkAuditRefine", diff --git a/README.md b/README.md index e7427f3..c392a45 100644 --- a/README.md +++ b/README.md @@ -329,7 +329,7 @@ A version bump **must** update all four version slots across three files togethe ## Status -**0.14.64** — Four changes to the per-phase Workflow engine's dispatch surfaces: stage-time args embedding, completed submodule floor/note coverage, the two routing corrections that coverage makes reachable, and a drift guard for a header comment that was about to rot. **(1) `stage-workflow.mjs --args ` embeds over-size phase args (#1134).** At real campaign size the assembled phase args cannot ride the Workflow tool call inline — the measured class is ~104.5 KB (`args.memory` blocks + verbatim plan slices + the Commander's Intent), and phase 1 of the measuring campaign shipped an operator-approved hand edit of the staged copy. That is now mechanized. `--args` takes its value as a **separate following token**, reads the file, JSON-parses it, and requires a non-null, non-array object — the ADR 0034 predicate mirrored from the template's own entry guard — **before any write**; it then performs an **optional third** exactly-once substitution, rewriting the template's object-arm args fallback tail (a new exported `ARGS_FALLBACK_ANCHOR`, imported by the anchor-guard test, never a second hardcoded copy) so the staged copy falls back to a prelude carrying the validated payload. The embedding is strictly the **absent-args fallback**: dispatched args, when passed, **always win**, so an `--args`-staged script is dispatched with **no** Workflow `args` at all — a dispatched `{}` is truthy and would beat the embedded fallback. The prelude is inserted **immediately after** the `export const meta = { … }` statement, never prepended: the Workflow tool refuses any script whose first *statement* is not `export const meta`, dying `Invalid workflow script` before a single agent is dispatched (reproduced live 2026-07-27 — a leading *comment* is fine, a leading `const` is not), and a test pins the position rather than mere presence, since a "prelude is present" check would have passed the broken prepend. Injection is step (3) by construction — the payload goes in **after** every exactly-once count has run, so an args payload that string-quotes any anchor's bytes cannot fork the stage. **Without the flag neither the fallback rewrite nor the prelude injection runs and the staged output is byte-identical to a stage predating it** — the existing restore-roundtrip tests, which whole-file byte-compare via reverse substitution, pass unmodified. Validation is fail-loud and pre-write: malformed JSON, each non-object parse (array, scalar, `null`), an unreadable file, `--args` missing its value token, and a duplicate `--args` each exit non-zero with a named `stage-workflow:`-prefixed stderr error and write no staged file; `--args --force` consumes `--force` as the filename **verbatim** and dies at the named read error (the peel runs before the `--force` filter and the positional split, and that ordering is pinned by tests, never incidental). The **attached** `--args=` form is refused with the usage error, and that guard is load-bearing rather than decorative: without it the token is not a flag to this parser at all — it survives the `--force` filter and binds to the 5th positional `campaignOrdinal`, staging an args-**less** script at exit 0, which is precisely the incident this change exists to close. Write-if-absent is unchanged — an existing staged file is still reused byte-untouched at exit 0, with one added stderr warning that `--args` was ignored, and re-embedding **changed** args is a deliberate `--force` restage. This also closes the `resumeFromRunId` hazard: an `--args`-staged script needs **no** re-passed args on resume, because write-if-absent guarantees the same embedded bytes, where an args-less resume previously died at entry validation despite a healthy journal. **Two residuals are explicit:** a script staged **without** `--args` keeps today's behavior (its args-less resume still dies at entry, the fallback existing only when `--args` staged it), and no new resume path is sanctioned — `resumeFromRunId` stays `held:phase-incomplete`-only. `skills/war/SKILL.md`'s staging paragraph documents the whole flow — when to write the args JSON as a sibling of the staged script, the dispatch-with-no-args caveat, precedence, resume, the campaign-phases-default-to-`--args` decision rule, and the stage-failure runbook line (an exactly-once failure names the offending anchor label; `--force` cannot help) — while ADR 0037 decision 2 gains a dated **append-only** amendment and `CONTEXT.md`'s staged-phase-script entry is edited in place, each naming the optional third substitution; block-scoped assertions in `skill-doc-contracts.test.mjs` gate both, per medium. **(2) The submodule floor and note are complete across three dispatch families (#1114, landed as #1160).** Three sibling gaps left open when `2026-07-24-recovery-re-merge-dispatch-coherence` closed the MERGE-side retries are now closed, each converging the dispatched surface on what `agents/war-refiner.md` already declares ("always"). Both **re-land** prompts (`land:phase-:environment-proceed` and `:baseline-proceed`) omitted `submodLandNote`, so a submodule phase whose first land gate-failed recoverable was re-dispatched land-blind; both now carry it. The **polish merge** (`merge:p-polish`) named no submodule floor at all — it now carries the invocation **bare**, a coherence sweep never being a declared gitlink bump, and its skip sentence now says explicitly that the two skipped floors are skipped *because they are task-field-gated* while the submodule floor is unconditional and still runs. The **floor-retry re-merge** (`merge::floor-retry:r`) omitted the invocation its own adjacent comments already claimed it carried; it now carries it with the gitlink-bump `--declared` conditional shape. Every gain is a prose append reusing in-scope constants, and `submodLandNote`/`submodMergeNote` remain `''` off the submodule path — so no non-submodule phase gains a submodule NOTE; the two floor invocations are unconditional by design and do add one line to the polish and floor-retry prompts on every phase — every existing non-submodule prompt *test* passes unmodified. **(3) Two operator-visible routing corrections, both reusing existing enum members.** A floor-retry re-merge returning `submodule-blocked` now **HARD-holds the phase** through an explicit arm (`reason: 'escalate'`); it previously fell to the generic `reason: floorMr.status` fallback, and because `submodule-blocked` is not a `HARD_ESCALATION_REASONS` member that routed **soft** — the phase landed minus the task, silently. A re-land returning `submodule-pr` — newly reachable now that both re-land prompts carry the land note — now yields `landDecision: 'held:submodule-pr'` with `pr_number`/`pr_remote` on the escalated entry; it previously fell to the `held:land-failed` else, mislabelling the hold **and** losing the PR ref the Lead's `gh`-resume reads. Both arms reuse members that already exist: `land-decision.mjs` is **byte-untouched**, no `MERGE_RESULT` status was added, no `HARD_ESCALATION_REASONS`/`KNOWN_LAND_DECISIONS` member changed, and no floor script was edited (ADR 0005). The fail-open **polish** routing is deliberately left alone — a polish merge returning `submodule-blocked` still routes the existing DISCARD arm, demoting the queue to follow-up and landing the pre-polish tip. **(4) `classificationClause`'s consumer-site list is arbitered, not narrated (#1151).** Its header comment enumerated the mirror sites in prose — the exact shape that rotted for `gateCaptureClause` (#1034) — so it is repaired while still accurate and its site list delegated to a new drift guard in `workflow-template.test.mjs`, a sibling of `captureUses`, which counts the helper's call-paren occurrences in the template, asserts exactly **3**, and names the three sites in its failure message. The anchor is the bare call-paren, deliberately wider than a first-argument pin: a 4th site passing any other base variable still trips it, a line-wrapped argument list still matches, and the definition itself cannot match, having ` = (` between the name and the paren. The rewritten comment names that guard as the arbiter and stays count-free — writing the call-paren byte-run into it would red the guard itself. **No `agents/*.md` or `hooks/` edit rides this release:** the standing refiner card already stated the target behavior, so the dispatched surfaces converged on it rather than the reverse. +**0.14.65** — Three correctness fixes to the shared memory CLI (`skills/_shared/war-memory.mjs`), each bound at its own verb's boundary, plus the doc and lesson cascade they force. **(1) `query`'s `--top-k` and `--budget` refuse a bad value instead of silently binding a degraded one (#1145).** Both flags now carry the three-way argv-boundary resolution #1059 ratified for `tighten-plan`'s `--target`: flag absent ⇒ the default (the flagless path stays byte-identical), a positive finite string binds, anything else exits **1** with stderr naming the flag and the token it received. The pre-fix read was a truthy ternary — `argv['top-k'] ? Number(argv['top-k']) : DEFAULT_TOP_K` — so every bad **space-separated** value still bound *something*: a **bare** flag became `1` via `Number(true)`, quietly cutting a seat's prefetch to a single lesson; a non-numeric or zero `--top-k` emptied the block outright through `selectForBudget`'s `records.slice(0, topK)`; and a non-numeric `--budget` left it **uncapped**, because `bytes + size > budget` is never true against `NaN`, so the break never fires. The guards sit **above** the `walkCorpus` call, so a refused invocation writes nothing at all — no query-log line, no stdout block. No integer check: `--top-k 3.5` still binds and `slice` truncates it, mirroring `--target`'s predicate exactly (D3). **The `=`-attached form is a named, still-silent, deliberately frozen residual (D10)** — `parseArgv` is untouched, so `--top-k=abc` is keyed whole as `{'top-k=abc': true}`, `argv['top-k']` stays `undefined`, the guard never sees it, and the query runs on defaults. Three comment-coupled sites — `--target`, `--top-k`, `--budget` — and deliberately no shared helper. **(2) The redaction-lint walk is recursive (#1135) — the one change here that can turn a previously green tree red on upgrade.** `lint ` now enumerates with `fs.readdirSync(dir, { recursive: true })`, so `.md` files in subdirectories — the committed `archive/` subtree above all — are linted like any other. A learnings tree whose top level is clean but which holds a redaction violation under `archive/` exited **0** before this release and exits **1** now, naming the file and the pattern, with the reported hit path carrying its subdirectory prefix: a real new failure for anyone whose archive had never been walked, and the remedy is to redact the named file, not to pin the old walk. Because every surface shares the one invocation, CI (`.github/workflows/memory-audit.yml`) and the gate wrapper (`skills/_shared/war-memory-lint.test.sh`) inherit the recursion with **zero edits** and stay byte-unchanged. The `catch { continue }` fail-open on an unreadable entry is preserved. **(3) `archive`'s concept-hub WARN counts slug-deduped citers (#1154).** The advisory WARN that fires when a hub with ≥ 2 hot inbound citers is about to lose its index row previously counted *records*, so a slug hot in **both** memory roots counted twice — the same lesson read as a hub at the `archive` verb while the `inbound` verb and `tightenPlan`'s cross-root Floor 2, both already slug-deduped, read it as one citer. `new Set(hotInbound.map((r) => r.slug)).size` now drives the ≥ 2 predicate **and** the printed number, so a cross-root twin alone no longer warns, and a twin plus one distinct citer prints **2**, not 3. The count noun reads **`inbound citers`**: the deduped number counts slugs, so naming it after records would be false. Still advisory, still non-blocking, still exit 0. **The cascade rides with the fixes rather than trailing them.** ADR 0028 quotes the WARN verbatim in its decision list, so it gains a dated **append-only** amendment recording the deduped counting and the new noun, while the original quote stands as the historical record — superseded, never rewritten. `/lessons-learned`'s Phase-3 hub-check thresholds and one doc-contract comment drop the record-flavored noun they had inherited: they document `war-memory inbound`, which was already slug-deduped, so a cross-root twin was being described as two of something when the tool counts it as one citer and no hub at all. And the two committed lessons behind this work are trued up — the archived `tighten --target` lesson replaces a paraphrased failure mode with the measured fuse mechanism (zsh does not word-split the unquoted `${TIGHTEN_TARGET:+…}` expansion, so the replacement arrives as one fused argv word, `--target 2000`, which `parseArgv` keys as `{"target 2000": true}` while `argv.target` itself stays `undefined` — the same silent outcome as never setting the variable, reached by a different route), and the `cmdQuery` truthy-ternary lesson carries a `RESOLVED` stamp citing this fix and the regressions that pin it. ## License diff --git a/docs/adr/0028-memory-store-integrity-tool-enforced.md b/docs/adr/0028-memory-store-integrity-tool-enforced.md index cb327d9..1cbb3b9 100644 --- a/docs/adr/0028-memory-store-integrity-tool-enforced.md +++ b/docs/adr/0028-memory-store-integrity-tool-enforced.md @@ -91,6 +91,18 @@ judgment — which hub to keep, which lesson to compress — stays human.** Conc reach — the sweep greps the current tree; the drift-guard/`/lessons-learned` posture is the standing backstop. +## Amendments + +- **2026-07-27 — the hub WARN counts slug-deduped `inbound citers`.** Decision item 2 above quotes the + WARN as first shipped: an undeduped record count with the noun `inbound refs`. A slug hot in *both* + roots is two records but one citer, so that count read `2` where the `inbound` verb (and `tightenPlan`'s + cross-root Floor 2) read `1`. As of + [`docs/plans/2026-07-26-war-memory-cli-correctness.md`](../plans/2026-07-26-war-memory-cli-correctness.md) + Task 1.1 (#1154), `cmdArchive` counts `new Set(hotInbound.map((r) => r.slug)).size` for both the ≥2 + predicate and the printed number, and the noun reads **`inbound citers`** — the deduped count counts + slugs, so the old noun would be false. Advisory, non-blocking, exit 0: unchanged. The quote in item 2 + stands as the historical record (superseded, never rewritten). + ## References - Design spec: diff --git a/docs/learnings/archive/tighten-target-flag-has-three-independent-silent-degradation-paths.md b/docs/learnings/archive/tighten-target-flag-has-three-independent-silent-degradation-paths.md index 66eacb9..58810de 100644 --- a/docs/learnings/archive/tighten-target-flag-has-three-independent-silent-degradation-paths.md +++ b/docs/learnings/archive/tighten-target-flag-has-three-independent-silent-degradation-paths.md @@ -64,8 +64,10 @@ none of which raise a diagnostic: 3. **Shell-dialect word-splitting**, at the doc layer: `skills/lessons-learned/SKILL.md`'s step-1 fence threads the flag as `${TIGHTEN_TARGET:+--target "$TIGHTEN_TARGET"}` inside a ` ```bash ` block. Bash splits the `:+` replacement text into two argv words as expected; **zsh does not** - (no `SH_WORD_SPLIT` by default), so under zsh the same line collapses to one argv token and the - flag is dropped entirely — same silent outcome as never setting `$TIGHTEN_TARGET`. + — lacking `SH_WORD_SPLIT` by default, it does not word-split the unquoted expansion, so the + whole replacement fuses into one argv word (`--target 2000` arrives as a single token), which + `parseArgv` keys as `{"target 2000": true}`; `argv.target` itself stays `undefined` — same + silent outcome as never setting `$TIGHTEN_TARGET`. **Why this matters:** all three degrade toward one of two silent states (default 17,000 B advisory, or the "verdict always warn" pre-select) with zero diagnostic — an operator who typos `--target` @@ -126,12 +128,12 @@ Method explicitly froze as a shared blast-radius fence — not reopened by this it at `cmdTightenPlan`'s own boundary (e.g. also refuse when any argv key starts with `target=`), never by widening `parseArgv`. -**Not yet extended to siblings:** `cmdQuery`'s `--top-k`/`--budget` resolve via the identical -pre-fix truthy-ternary shape (`argv['top-k'] ? Number(...) : DEFAULT_TOP_K`, same for `budget`) at -`skills/_shared/war-memory.mjs` lines 703-704 — this task's Method deliberately bound the fix to -`cmdTightenPlan`'s boundary only ("bind each fix at its own boundary, never a shared one") and did -not extend it to `cmdQuery`. See [[cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape]] -for the standalone gotcha and its failure-direction difference (NaN `--top-k` empties the seat -prefetch instead of over-selecting). +**Extended to siblings (war-memory-cli-correctness/1.1, #1145):** `cmdQuery`'s `--top-k`/`--budget` +now carry the same three-way `typeof`-gated resolution described above for `--target` — a shape +copy at `cmdQuery`'s own boundary, not a shared helper (`parseArgv` stays frozen). See +[[cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape]] for the RESOLVED +sibling gotcha and the failure-direction difference that made it worth its own lesson (a bare +`--top-k` had silently selected ONE lesson and a NaN/zero/negative value emptied the seat +prefetch, unlike `--target`'s over-select failure mode). > archived 2026-07-27: resolved — moved to archive diff --git a/docs/learnings/cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape.md b/docs/learnings/cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape.md index 53b9040..f50552c 100644 --- a/docs/learnings/cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape.md +++ b/docs/learnings/cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape.md @@ -1,6 +1,6 @@ --- name: cmdquery-topk-budget-share-tighten-targets-pre-fix-truthy-ternary-shape -description: "cmdQuery's --top-k/--budget resolve via argv['top-k'] ? Number(...) : DEFAULT — the exact truthy-ternary silent-degradation shape #1059 closed for tighten-plan's --target, left open here on purpose (fix bound to cmdTightenPlan's boundary only); a bare/NaN --top-k silently empties the seat memory-prefetch block instead of over-selecting" +description: "RESOLVED (war-memory-cli-correctness/1.1, #1145): cmdQuery's --top-k/--budget now carry the same three-way typeof-gated resolution #1059 ratified for tighten-plan's --target, closing the truthy-ternary shape (argv['top-k'] ? Number(...) : DEFAULT) this lesson flagged as deliberately left open; a bare --top-k no longer silently selects one lesson and a NaN/zero/negative value no longer empties the seat memory-prefetch block — both refuse loud instead" metadata: node_type: memory type: project @@ -69,3 +69,24 @@ three-way `typeof argv[x] === 'string'` resolution (undefined → default; strin Related: [[tighten-target-flag-has-three-independent-silent-degradation-paths]] (the sibling fix this gotcha was found alongside, in the same audit pass). + +## RESOLVED (war-memory-cli-correctness/1.1, #1145) + +`cmdQuery` now carries the same three-way `typeof`-gated resolution ratified for +`cmdTightenPlan`'s `--target` — a shape copy at `cmdQuery`'s own boundary, not a shared helper +(`parseArgv` stays frozen): flag absent → the unchanged default; `typeof === 'string'` and +`Number.isFinite(t) && t > 0` → the value binds; anything else (bare flag, non-numeric, zero, +negative) → `war-memory query: --top-k requires a positive count (got '')` (resp. +`--budget requires a positive byte count`) on stderr, exit 1 — placed above the `walkCorpus` call, +so a refusal appends no query-log line and prints no block. This closes exactly the gap this +lesson flagged: a bare `--top-k` no longer takes `Number(true) === 1` into a one-lesson block, a +`NaN` `--top-k` no longer reaches `selectForBudget`'s `slice(0, topK)` to silently empty the block, +and a `NaN` `--budget` no longer silently uncaps it (`bytes + size > NaN` never breaks the loop). + +Covered by the `refusal (#1145): --