docs(cli): untag exclusivity is per -t value, not per run; correct three more misstated claims

[andrei-hasna]

Wording-only corrections to claims landed by #34/#35, base main at 64d05ae. No behaviour change — the only non-comment source edit is one added list --help line, and removal semantics, exit codes and messages are untouched.

All exit codes below are unpiped (cmd >/dev/null 2>&1; echo $?). Every absence claim has a positive control.

---

1. "one shape per run" was measurably false — 4 sites

The continue that enforces untag's whole-value-before-split rule sits inside for (const raw of flags.tagRaw ?? flags.tag) (src/cli.ts), so the exclusivity is per raw -t value, not per run. Executed against a fixture holding both shapes, tags: ["a,b,c","a","b","c"]:

command	removed	remaining	exit
positive control — get --json	—	['a,b,c','a','b','c']	0
untag -t 'a,b,c'	1	['a','b','c']	0
untag -t 'a,b,c' -t a	2	['b','c']	0
untag -t 'a,b,c' -t a -t b -t c	4	[]	0

removed: 4 in a single run is "matching both shapes at once" — which four sites said untag deliberately does not do: README.md:323, README.md:417-418, src/cli.ts:1804-1805, src/cli.ts:1920-1921. All four now say per -t value.

The re-run contract itself is true and reproduces exactly, and is unchanged by this PR:

untag -t 'a,b,c'   ->  removed: 1   exit 0
untag -t 'a,b,c'   ->  removed: 3   exit 0
untag -t 'a,b,c'   ->  exit 1  ("No matching tag on k_both: "a", "b", "c" not in []")

Only the reason was misstated — and the reason is what a reader would reimplement from.

The mutation still fails

The wording change is documentation, so the guard that catches the literal "one shape per run" behaviour must still catch it. Re-applied the mutation (a run-level sawWhole flag suppressing later splitting) on top of this branch:

bun test tests/cli.test.ts -t "untag matches a tag value whole before splitting, and fails when nothing is removed"
  -> exit 1,  0 pass / 1 fail
  tests/cli.test.ts:713  expect(partialOut.not_found).toEqual(['nope'])
    - [ "nope" ]
    + undefined

Caught by name, at a named assertion. Mutation reverted; the committed tree rebuilds to the committed bundle byte-for-byte (see §4).

2. Fix 2's README cited an unreachable justification

README.md:434-435 (and the matching src/cli.ts comment) motivated quoting the partial-miss names with "(not found: p, q) cannot be told apart from one missing tag literally named p, q". That collision is unreachable. A comma-bearing value only enters remove through the whole-value branch, which requires stored.has(whole) — so it is found by definition and filtered out of notFound; every other entry is a comma-split part, which cannot contain a comma. Both branches of flags.tagRaw ?? flags.tag are comma-free (flags.tag is already dedupeTags(comma-split parts)).

12 probes, zero commas ever in not_found. The two that show the mechanism:

stored	-t args	not_found	note
["alpha","p, q"]	-t alpha -t 'p, q'	None (removed 2)	whole value matched → found, so never in not_found
["alpha"]	-t alpha -t 'p, q'	['p','q']	split → no comma

The reachable justification is whitespace trim() does not strip — corrected after review, see §5. A plain space is not an ambiguity: since no not_found entry can hold a comma, joining on ", " is injective (["p","q"] → p, q, ["p q"] → p q, distinct strings), so a space is a legibility problem only. The load-bearing case is a control character inside a name, which survives trim():

stored ["alpha"] + -t alpha -t $'p\nq'   ->  not_found: ["p\nq"]
  unquoted: 'Removed 1 tag from k_w (not found: p\nq)'    -> 2 lines
  quoted:   'Removed 1 tag from k_w (not found: "p\nq")'  -> 1 line

Joined raw, the name breaks the single-line message in two and the tail reads as separate output. Tab behaves identically. This is now pinned by a test assertion, not left as prose. The original narrowing was under-claimed: quoting has reachable value, just not the value the README gave.

3. An undocumented flag precedence on list

--archived wins over --include-archived (the else if), in either argument order. Documented in the flags table, the prose, a source comment and list --help. Behaviour untouched.

Fixture: 2 live + 1 archived.

args	total	ids	exit
(none)	2	k_live1, k_live2	0
--archived	1	k_arch1	0
--include-archived	3	all three	0
--archived --include-archived	1	k_arch1	0
--include-archived --archived	1	k_arch1	0

4. verify:generated was cited for something it does not check

scripts/verify-generated-artifacts.mjs only (a) git diff --exit-code -- bin/knowledge-mcp.js dist and (b) greps four files for two stale Windows-path regexes. It never compares bin/knowledge.js against src/cli.ts. Proven, not read:

tree state	verifier exit
clean	0
src/cli.ts diverged from bin/knowledge.js	0 ← blind
bin/knowledge.js itself corrupted (+30 junk bytes)	0 ← blinder than expected: it does not require that file to be byte-stable at all, only bin/knowledge-mcp.js and dist
bin/knowledge.js with a planted file://${x}	1 ← negative control: the check it does have works

All probes reverted; git status --porcelain empty afterwards.

The citation that does establish bundle/source sync, and the one the CHANGELOG now points at — rebuild with the build script's command to a temp outfile:

rebuilt   1045262 bytes
committed 1045230 bytes   (bin/knowledge.js at 64d05ae)
after per-line trailing-whitespace strip -> 1045230 bytes, sha256 IDENTICAL
  0bfceb0969392f18605c05995af02907ef3905753bacfd6a750785c08ab77e6d

The 32-byte delta is exactly what scripts/strip-generated-trailing-whitespace.mjs removes.

I did not add the missing check to the verifier. That would be a code change needing its own review, so it is called out rather than slipped in — see the "Left undone" section.

---

Artifacts

bin/knowledge.js is rebuilt, and the line-level diff against 64d05ae is exactly the one added help string — 1376 → 1377 lines, 3 changed lines, all the same sentence:

+  If both --archived and --include-archived are passed, --archived wins (archived items only).

Comment-only source edits do not reach the bundle (the minifier drops them), so this is the entire behavioural surface of the PR. Verified after committing: rebuilding the committed src/cli.ts to a temp outfile and stripping reproduces the committed bin/knowledge.js byte-for-byte (sha256 0630038c6ab4c2…). bin/knowledge-mcp.js, bin/knowledge-serve.js and dist are untouched, so bun scripts/verify-generated-artifacts.mjs exits 0. Not bun run verify:generated — that exits 1, on main as well as here, because package.json:58 rebuilds first and the committed bin/knowledge-mcp.js predates #33's package.json files entry that gets inlined into it. Pre-existing, not this PR (git diff --exit-code 64d05ae 05ee01c -- bin/knowledge-mcp.js dist/index.js → 0); tracked as its own task.

Verification

Hermetic runs need three env vars unset: KNOWLEDGE_API_URL, HASNA_KNOWLEDGE_API_URL, HASNA_KNOWLEDGE_API_KEY.

check	exit	detail
bun test tests/cli.test.ts --timeout 60000 — base 64d05ae	1	72 pass / 1 fail / 73
bun test tests/cli.test.ts --timeout 60000 — this branch	1	72 pass / 1 fail / 73
bun test tests/package-release.test.ts	0	5 pass / 0 fail
bun scripts/verify-generated-artifacts.mjs	0	(see §4 for what this does and does not prove)
bun run verify:generated	1	pre-existing on main — rebuilds first, and the committed mcp bundle is stale since #33. Not this PR.
bunx tsc --noEmit	1	8 errors, ANSI-stripped — same 8 as base, none in a touched hunk
mutation re-applied (§1)	1	caught by name

The single failure is identical on both sides, by name: context pack and proposal context commands return bounded agent JSON — pre-existing, also fails on base 2531d0d. No branch-only failing test names.

tsc's 8 errors are src/cli.ts:1229 ×2 (activeEnv on StorageStatus), tests/knowledge-db.test.ts ×4, tests/sync.test.ts ×2. The src/cli.ts pair is ~570 lines from the nearest hunk I touched.

Secrets: staged-diff pattern sweep over 120 added lines → 0 findings; secrets scan workspace → 22 findings, 0 in any of the four staged paths (all in untouched test fixtures, docker-compose.yml, src/serve.ts, bin/knowledge-serve.js).

Left undone, deliberately

• The verifier still has no bundle/source check. Adding one is a code change; it wants its own PR and its own review. The shape that works is in §4 (temp-outfile rebuild + strip + compare), and it is now written down in the CHANGELOG so the next person does not have to rediscover it.
• ok_untag MCP parity — untouched, tracked separately.
• Not touched at all: the store divergence, config.mode, the HASNA_KNOWLEDGE_API_KEY exposure, hasna-agent-comms-envelope.

Two measurement traps worth recording

Both were hit while verifying this, and both return a confident wrong answer rather than nothing:

• grep -c $'\x00' bin/knowledge.js returns 1376, which is the file's line count. Bash cannot put NUL in argv, so the pattern degrades to '' and matches every line. The pattern was not eaten, it was emptied. True NUL count is 0 (python3, and len(d) - len(d.replace(b'\x00',b'')), agree). For byte-level absence never use a shell-quoted control character.
• grep -F 'notFound' bin/knowledge.js exits 1 while src/cli.ts has 4 hits — the minified identifier is K. Correct grep, correct file, wrong assumption. Judge bundles by behaviour or a minification-tolerant pattern; the bundle does carry the fix: (not found: ${K.map((P)=>JSON.stringify(P)).join(", ")}).

Note on the toolchain

This repo has no committed lockfile (git ls-files | grep -i lock → only tests/store-lock.test.ts), so bun install --frozen-lockfile exits 0 but pins nothing. @hasna/events resolved to 0.1.14 against a ^0.1.3 range. Worth a separate decision; not changed here.

---

5. Reconciliation with the adversarial review (commit 05ee01c)

An independent adversarial reviewer measured 59d1d65 on its own worktree, reproduced all four corrections and both traps, and returned FIX-FORWARD with five findings. All are now fixed in 05ee01c, kept as a second commit rather than an amend — 59d1d65's message contains the wrong verify:generated sentence, and rewriting history to hide it would be the same mistake as F5 below.

#	finding	disposition
F1	the corrected claim survived at tests/cli.test.ts:722-725 — 4 sites repo-wide, not 3	fixed; I had self-found this too, independently
F2	"verify:generated still exits 0" is false — the npm script exits 1	fixed in the body above and in 05ee01c's message; root cause is pre-existing main staleness, now its own task
F3	the replacement wording was itself an overclaim, and the truly load-bearing case was missed	fixed — this is an upgrade, see §2
F4	the rebuild-and-compare check is dependency-state-dependent	caveat added to the CHANGELOG; lockfile is its own task
F5	editing merged PR #34's body left no audit trail	reverted — body restored byte-for-byte, correction re-posted as a comment
F6/F7	scope judgements + one wording nit	nit fixed; scope calls stated below

F3 is the substantive one. My replacement claim — "one tag named p q reads exactly like a pair" — was false, because a pair renders p, q. The reviewer's diagnosis is right: the unquoted join is injective over comma-free entries, so a plain space is legibility, not ambiguity. It then supplied the case that genuinely is load-bearing (a newline or tab surviving trim()), which makes the word "ambiguity" true. Corrected wording plus a new assertion; that assertion is the only added test in this PR, and its premise is measured, not asserted: unquoted the message is 2 lines, quoted it is 1.

F5: gh api repos/hasna/knowledge/issues/34/timeline showed commented 3, committed 6, closed 1, merged 1 and no body-edit event — GitHub keeps no PR-body revision history, so my "read back identical" was unverifiable by anyone else. Body restored to its exact pre-edit 10736 chars (verified by readback), correction re-posted as an attributed comment; the timeline now reads commented 4.

On F6(b), the one scope call I would defend rather than reverse: adding the precedence to list --help forced the bin/knowledge.js rebuild, which is the widening the brief warned against. I kept it because the help string is the surface an agent actually reads, and the cost is bounded and verified — the reviewer independently confirmed the bundle delta is exactly one string via --word-diff=porcelain. F6(c) is reversed: the 9-line bullet I added to the unrelated "inventory paths block fix" section is back to a 4-line pointer.

Re-verified at 05ee01c (unpiped): bun test tests/cli.test.ts --timeout 60000 → exit 1, 72 pass / 1 fail / 73, same sole pre-existing failure; the untag test alone → exit 0, 1 pass, 35 expect() calls; dropping the JSON.stringify quoting → exit 1; bunx tsc --noEmit → exit 1, same 8 errors; bun scripts/verify-generated-artifacts.mjs → 0. Every source edit in 05ee01c is a comment, so bin/knowledge.js is not rebuilt — proven by rebuilding the tip to a temp outfile and reproducing the committed bundle byte-for-byte (sha256 0630038c6ab4c200).

Secrets on the follow-up diff: 114 added lines, 0 credential findings (my app_password heuristic fired once on the English prose case that does make — four consecutive four-letter words). secrets scan workspace reports 5 findings in tests/cli.test.ts, all pre-existing redaction fixtures present at base with line numbers shifted by exactly my +22, none inside my hunks.

Filed as separate tasks, not fixed here

• main ships a stale bin/knowledge-mcp.js — bun run verify:generated is red on main at 64d05ae, independent of this PR.
• No committed lockfile — @hasna/events is bundled (not --external) and declared ^0.1.3, so the rebuild-and-compare check this PR documents is dependency-state-dependent: 0.1.14 reproduces the committed bundle, 0.1.13 does not. The shared main checkout on this box has 0.1.13 installed right now.

Claims the reviewer could not verify either way

Its worktree is /home/hasna/.hasna/repos/worktrees/knowledge/hc-00133-adversarial-review (clean, detached at 59d1d65). It did not re-run secrets scan workspace, did not exercise --archived precedence in self_hosted/cloud mode (owner-held config.mode), and could not establish cross-machine bundle determinism — F4 shows it is dependency-sensitive even on one box. Determinism within a fixed tree is solid: two processes, identical sha256.

[andrei-hasna]

Adversarial review — APPROVE WITH FIXES APPLIED (fix pushed as 3f8fb58)

Independent re-derivation on my own worktree /home/hasna/.hasna/repos/worktrees/knowledge/pr36-adv-review (detached at 05ee01c), with a base comparison worktree at 64d05ae. @hasna/events resolved to 0.1.14 in both. All exit codes unpiped; every absence check carries a positive control.

One defect found and fixed on the branch

All three of README.md:330-334, src/cli.ts:1812-1817 and tests/cli.test.ts:825-828 stated the consequence of a split-only list -t as a universal: it "returns a DIFFERENT item that carries the three names separately, at total: 1 and exit 0". It only does that when the corpus also holds an item carrying the three names separately.

Measured by removing the whole-value branch from the list predicate (a real mutation, not a simulation with three separate -t values):

corpus	filter	rc	result
[k_glued("a,b,c")]	union -t 'a,b,c'	0	total=1 ids=[k_glued]
[k_glued("a,b,c")]	split-only -t 'a,b,c'	0	total=0 ids=[] ← a miss, not a swap
+ k_split("a","b","c") + k_ctrl("a","b")	union -t 'a,b,c'	0	total=2 ids=[k_glued,k_split]
+ k_split + k_ctrl	split-only -t 'a,b,c'	0	total=1 ids=[k_split] ← the swap

Both outcomes are silent, so the case for the union is untouched. What was wrong is that the swap was presented as what the defect does, when it is what the defect does only if a discriminating item happens to exist — which is also why a fixture without one makes union and split-only look identical, and is plausibly how earlier readings of this same paragraph survived review. The test's fixture does supply k_control, so its comment now says it constructs the worse case deliberately.

Also corrected: the CHANGELOG preamble said "one added test assertion"; the added block is one test case with six expect() calls (measured 860 → 866 between 64d05ae and this branch).

Every source edit in 3f8fb58 is a comment, verified rather than assumed: bun run build on this tree leaves bin/knowledge.js byte-identical to the committed file (git status --porcelain -- bin/knowledge.js empty after a full rebuild), so the bundle is deliberately not rebuilt.

Everything else in the PR reproduced

• untag exclusivity is per raw -t value. Fixture ["a,b,c","a","b","c"], positive control get --json returns all four: -t 'a,b,c' → rc=0 removed=1 remaining ["a","b","c"]; -t 'a,b,c' -t a → removed=2; -t 'a,b,c' -t a -t b -t c → removed=4, remaining []. Re-run contract on one store, not reseeded: removed=1 → removed=3 → rc=1 No matching tag on k_both: "a", "b", "c" not in []. The continue at src/cli.ts:1945 is inside for (const raw of flags.tagRaw ?? flags.tag) at :1943.
• list -t is a union — src/cli.ts:1840. Discriminating fixture: -t 'a,b,c' → total=2 [k_glued,k_split], k_ctrl("a","b") correctly excluded.
• --archived beats --include-archived in both orders. 2 live + 1 archived: none→2; --archived→1 [k_arch1]; --include-archived→3; both orders of the pair→1 [k_arch1]. The new list --help line prints identically from src/cli.ts and from the committed bundle (diff of the two help outputs → rc=0).
• list narrows on repeated -t: 3 → 2 → 1, exactly as the source comment says.
• No comma can reach not_found: 14 probes, none produced one. Detector positive-controlled by planting an unconditional remove.add(whole), which produced not_found=["p, q","p","q"].
• Newline case: -t alpha -t $'p\nq' → rc=0, not_found=["p\nq"], message 1 line; with JSON.stringify removed the message is 2 lines.
• §4 verifier claims, all four: standalone rc=0; bin/knowledge.js +30 junk bytes rc=0; src/cli.ts diverged rc=0; planted file://${x} rc=1 (negative control). bun run verify:generated → rc=1 on this branch and rc=1 on base 64d05ae, both on bin/knowledge-mcp.js + dist/index.js → pre-existing, as the PR says.
• Bundle scope of 05ee01c: one changed line opcode, base[1344:1345] → head[1344:1346]; the character-level insert is exactly .\n If both --archived and --include-archived are passed, --archived wins (archived items only) = 95 bytes = 1045325 − 1045230.
• Tests / typecheck: 05ee01c and 64d05ae both 72 pass / 1 fail / 73, rc=1, same name (context pack and proposal context commands return bounded agent JSON); 3f8fb58 the same. bunx tsc --noEmit rc=1, 8 errors on both sides (src/cli.ts ×2, tests/knowledge-db.test.ts ×4, tests/sync.test.ts ×2).
• CI is pre-existing red on main. 3f8fb58 fails test (macos-latest, bun) and test-matrix (macos-latest, bun) with 254 pass / 2 fail — the same two test names that fail on 64d05ae: context pack and proposal context commands return bounded agent JSON and sync export redacts local file and workspace refs from bundles. No branch-only failure. The cli.test.ts#3019 → #3044 line shift is exactly this PR's +25 insertions.
• Root cause not regressed: src/cli.ts:201 still accumulates via collectTagFlag(flags.tag, …) and spreads into flags.tagRaw.

Filed separately, not fixed here

• list -t's union has no MCP counterpart — measured over JSON-RPC against bun src/mcp.js: ok_list tag:["a,b,c"] → total=1 [k_glued], tag:["a","b","c"] → total=1 [k_split], while the CLI returns both. src/mcp.js:1551-1556 is exact-match only. Same class as the tracked ok_untag gap (src/mcp.js:1680 is a plain set-difference). Todos bf0cd7b7.
• README.md:411-415 / src/cli.ts:1921-1925 say a split-only untag "would remove nothing while still exiting 0"; today removed === 0 throws (rc=1). Describes the pre-#34 world, reads as current behaviour. Pre-existing on main. Todos 95b0d5aa.

One correction to this PR's body, recorded here rather than by editing it

The §2 sentence "Both branches of flags.tagRaw ?? flags.tag are comma-free" is false for tagRaw: src/cli.ts:201 stores the raw argv value, which is precisely the comma-bearing branch — it is flags.tag alone that is pre-split. The reasoning the body then gives (a comma-bearing value only reaches remove via the whole-value branch, so it is found by definition) is correct and is what carries the claim, and the committed README/source wording does not contain the error. Left as a comment rather than a body edit, per the same rule that produced F5 last round.

Could not verify either way

--archived precedence in self_hosted/cloud mode (owner-held config.mode); cross-machine bundle determinism (dependency-sensitive per F4); secrets scan workspace was not re-run — I ran the staged-diff pattern sweep over my four staged files (rc=1, clean; detector positive-controlled against a planted ghp_ string).