1684 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
xarmian	80be76a3ce	docs(mcp): the detectFieldConflicts header stated the pre-round-14 reach (BUG-2850) ... Comment-only. The lead caught it in the package review. The "SCOPE:" paragraph still said the pass runs only when a `fields` object is present, and that a top-level-vs-`field:[]` collision without one is outside it. Round 14 falsified both halves — the pass runs from both action entry points on every call — and the body's own comment said so while the header contradicted it. On the one boundary this loop spent eighteen rounds on, the header is what a future reader trusts. CONVE-23 is exactly this and I missed it: the round-14 commit swept the version.go prose and the test comments, and left the header of the function it had just changed. The paragraph now states the actual reach: both entry points regardless of `fields`; alias collisions adjudicated always and refused even on equal values; same-name collisions adjudicated only when the `fields` object carries THAT key, which is a per-key question; the round-7 exemption as the sole carve-out, itself narrowed to keys the CLI can express (the compat IDs refuse, but only when a top-level compat value is present), with padded entries outside it. It closes with the instruction the loop earned: say a new condition's QUANTIFIER out loud before writing it. Rounds 15, 16, 17, 19, 20 and 21 were each that question answered by assumption. gofmt clean · go vet clean · go test ./internal/mcp/ ./cmd/pad/ green	2026-09-03 20:44:01 +00:00
xarmian	a1b8e63a29	fix(mcp): a nil top-level value is absence, for every key (BUG-2850) ... Codex round 21, one P2 and no P1 — a false refusal, and the finding named a strict subset of it. topLevelValueProvided returned true unconditionally for the compat IDs, and fell through to true for everything else, so a nil counted as a supplied value and refused against a `fields` entry for the same key. Nothing writes a nil: the HTTP mapper's `.(string)` assertion drops it and BuildCLIArgs has no flag value to emit, so both doors resolve to the `fields` value. The finding named `assigned_user_id` / `agent_role_id`. Probing the population first — the habit this unit has been beating into me — showed all five top-level keys behaving identically, because the non-compat path fell through to `return true` as well. Fixing the named pair alone would have left `status: null` refusing. Mutation matrix, three directions: remove the nil check -> all five key legs fail fix ONLY the named compat pair -> status / priority / parent fail treat the empty compat clear as absence too -> both clear-semantics tests fail The middle mutant is the population-versus-instance distinction made executable: a fix that satisfies the reviewer's example and nothing else is red, by name, in three legs. Gates: gofmt clean · go vet clean · go test ./... green (29 packages) · contract-drift gate green	2026-09-03 19:59:16 +00:00
xarmian	a9f2405903	fix(mcp): the compat exception turns on a top-level value, not on the key (BUG-2850) ... Codex round 20, one P2 and no P1 — another false refusal from a reason of mine applied past the source it was verified on. Round 15's reason was specific: a TOP-LEVEL compat param has no CLI flag, so BuildCLIArgs drops it while HTTP reads it, and the doors receive different writes. I then keyed the exception on the KEY being a compat one, which caught `field:["assigned_user_id=A", "assigned_user_id=B"]` — two array entries, no top-level value, no asymmetry: both doors keep the last and lift the same column. Refused a call that resolves deterministically. The gate now asks whether a top-level compat value is actually present, which is the condition the reason describes. Mutation matrix, both directions: broaden it back to any compat-keyed contribution -> only the two-entry legs fail drop the exception entirely -> only the top-level legs fail (round 15's defect returns) Round 15's own case is a leg of the new test deliberately: without it this pin would pass on a build that dropped the compat exception altogether, which is the defect round 15 existed to fix. Gates: gofmt clean · go vet clean · go test ./... green (29 packages) · contract-drift gate green	2026-09-03 19:27:32 +00:00
xarmian	052850f613	fix(mcp): both gates ask the per-key question; compare like with like (BUG-2850) ... Codex round 19, two P2 and no P1. Both were FALSE REFUSALS my own fixes introduced — the first is round 17's mistake in the sibling gate. [P2] THE SAME-NAME GATE WAS STILL PER-REQUEST. Round 17 made the padded gate per-key and left this one asking whether the request has any `fields` object. With `fields:{"other":"x"}` and `field:["effort=l","effort=s"]`, `effort` is not in the object, nothing arbitrates it but the doors themselves, and both keep the last entry — so a call that resolves deterministically was refused. The predicate is now `canonicalized`, the same per-key question the other gate asks. `fieldsPresent` no longer exists anywhere in the pass, and its absence is commented as the fix's shape: a future gate reaching for "does the request have a fields object" is almost certainly this mistake a third time. [P2] TRIMMED AND UNTRIMMED VALUES WERE COMPARED. Entry values are trimmed for comparison because ingestFieldKVP trims them; the `fields` object's value was compared raw. `fields:{"note":" x "}` with `field:["note= x "]` read as " x " vs "x" and refused, though both doors write " x ". Only the COMPARISON key is trimmed now — `raw` and the re-emitted wire value keep the caller's whitespace. Mutation matrix: same-name gate back to per-request -> only the unrelated-key leg fails stop trimming for comparison -> only the whitespace-equal leg fails trim the EMITTED value too -> only the re-emission leg fails THE THIRD MUTANT SURVIVED AT FIRST, and it was unreachable rather than unobserved: the whitespace test's entry is already canonical, so the re-emission path never ran and a mutant trimming the emitted value changed nothing it could see. Added a leg whose KEY is padded, which forces the re-emission, and asserted the emitted value still carries the caller's whitespace. Third time this loop that asking "is the mutant faithful" before "is the test weak" found a real hole (CONVE-28). Control legs, both directions: a `fields` object that DOES carry the key still refuses differing values, and genuinely different values are still refused however they are padded. Gates: gofmt clean · go vet clean · go test ./... green (29 packages) · contract-drift gate green	2026-09-03 19:17:31 +00:00
xarmian	21a3057389	fix(mcp): keep per-entry multiplicity in the conflict pass (BUG-2850) ... Codex round 18, one P2 and no P1 — and the fix is upstream of the rules rather than another rule. parseFieldArray indexes by NORMALIZED key, so two entries naming one key collapsed into a single index slot, and this pass walked that index. Its own input was lossy: `field:["effort=l", " effort=l"]` arrived as ONE contribution, fell under the len < 2 early exit, and passed unchecked — HTTP trims both to `effort` while stdio writes `effort` AND a junk `" effort"`. The pass claims to adjudicate one canonical key offered by multiple sources; two array entries ARE multiple sources, and it could not see them. It now walks the raw entries, so multiplicity survives and the existing rules apply unchanged — no new branch. The index is deliberately discarded here and the discard is commented, because reaching for it is the natural thing to do next. I NEARLY CHANGED THE CODE TO SATISFY A WRONG TEST. The third leg was first written asserting that two canonical entries with DIFFERING values are refused. The code disagreed, and the code was right: ingestFieldKVP (HTTP) and the --field loop in cmd_item.go (CLI) both do `map[key] = val` in entry order, so each door keeps the LAST entry and they agree. That is the round-7 boundary exactly — a visible duplicate with a resolution the caller can predict — and refusing it would have contradicted the boundary the lead confirmed, on two doors pinned to agree. Verified by reading both loops before touching anything. The leg is kept, inverted, because it is the one that stops a future "refuse every repeated key" simplification from looking correct. Mutation: restoring the collapse (walk one contribution per key) fails exactly the padded-twin leg, and leaves the two control legs green. Gates: gofmt clean · go vet clean · go test ./... green (29 packages) · contract-drift gate green (go test ./internal/mcp/ -run 'CoversEveryCatalogAction|VersionMatchesToolSurface')	2026-09-03 18:55:15 +00:00
xarmian	130c854052	fix(mcp): canonicalization is a per-KEY property, not a per-request one (BUG-2850) ... Codex round 17, one P1, and it is the third consecutive round where my own fix generalized a property verified on one subset to the whole. Round 16 gated the padded-entry refusal on "no `fields` object", reasoning that a `fields` object makes reshapeItemFields re-emit the entry canonically. That holds for keys IN that object. With `fields:{}`, or a `fields` carrying some OTHER key, nothing canonicalizes `field:["status = done"]` and it reaches the doors padded exactly as it does with no `fields` at all — HTTP writes `status`, the CLI writes a junk `"status "` beside it. The predicate is now the actual question: will anything canonicalize THIS key. The pattern is worth naming because it is now a habit rather than an accident: round 15 — a premise true of schema-declared params, applied to the compat IDs, which are undeclared precisely so it cannot hold round 16 — the check placed below the exemption, so it covered one key class (caught by my own test's control leg) round 17 — a per-key property read as per-request Each time the fix was correct for the case in front of me and wrong for its siblings, which is the same shape as the defects this unit started with. The canonical restructure removed it from the CODE; it evidently did not remove it from how I reason about the code. Mutation matrix, both directions: revert to the per-request gate -> only the two uncovered-key legs fail refuse even when canonicalized -> only the canonicalized control fails The third leg of the new test is the control that makes the distinction real: with the key present in `fields` the entry IS canonicalized, so the call must still succeed. A fix that refused whenever anything was padded passes the first two legs and fails this one. Gates: gofmt clean · go vet clean · go test ./... green (29 packages) · the contract-drift gate ran green (go test ./internal/mcp/ -run 'CoversEveryCatalogAction|VersionMatchesToolSurface')	2026-09-03 18:41:00 +00:00
xarmian	3696686377	fix(mcp): a padded entry colliding with a param is not an equal duplicate (BUG-2850) ... Codex round 16, one P1, and its placement is the whole lesson. The conflict index is normalized — that is what lets a padded entry be recognized as a collision at all — so `field:["k = A"]` compared EQUAL to a top-level `k:"A"` and the pair was accepted while the entry stayed padded on the wire. HTTP trims it and writes `k`; the CLI does not, and writes a junk `"k "` key instead. The normalization that makes the collision VISIBLE is exactly what made accepting it wrong: equality on the normalized form licensed a collapse of the RAW forms, which are not equal at all. So equality only licenses a collapse when both doors receive the same write, and this check now runs BEFORE the same-name exemption. MY FIRST DRAFT PUT IT AFTER, which is round 15's mistake repeated one round later. Round 15 was a premise verified for declared params and generalized to two keys it did not hold for; placing this below the exemption meant only the compat IDs were checked, when padding breaks the exemption's own premise — both doors resolve the duplicate identically — for EVERY key class. The declared-param leg of the new test caught it, and the mutation matrix pins the placement rather than just the behaviour. Scope held: a padded entry standing ALONE, with no colliding param, is untouched. That is BUG-2870, ruled out of this PR, and fixing it changes what every CLI caller receives rather than only callers who supplied one key twice. There is an executable test for that boundary, so growing into BUG-2870's territory without a ruling goes red. Mutation matrix, three directions: remove the check -> both padded legs fail restrict it to compat IDs -> only the declared-param leg fails (the first draft) extend it to a lone entry -> the BUG-2870 scope-boundary test fails gofmt clean · go vet clean · go test ./... green (29 packages) context: 56.4% (session-shape)	2026-09-03 18:26:10 +00:00
xarmian	dae7bbf233	fix(mcp): the same-name exemption holds only where the doors agree (BUG-2850) ... Codex round 15, one P1, and it lands on the boundary I defended at round 7 and the lead confirmed. The exemption's premise is "both doors resolve a same-name duplicate identically". That is TRUE for a schema-declared param: the CLI has a real flag, so stdio receives BOTH forms (`--status open --field status=done`) and its overlay order resolves them exactly as the HTTP mapper does. I verified that per door and pinned it. It is FALSE for the v0.16 compat IDs, and being undeclared is precisely why. BuildCLIArgs emits the CLI's real flags; there is none behind `assigned_user_id`, so the top-level value is DROPPED and stdio sees only the field entry while HTTP reads the param. `assigned_user_id:"A"` with `field:["assigned_user_id=B"]` assigns two different people depending on transport — with no `fields` object anywhere, which is why the canonical pass's `fields`-gated half never saw it. So I generalised a premise from the params I had verified to the two whose whole nature is being unverifiable that way. The exemption is now narrowed to keys the CLI can express; the compat pair refuses. Swept for the prose this falsifies (CONVE-23): the v0.27 changelog entry said the no-`fields` same-name case is "NOT refused, deliberately", full stop. It now states the narrower rule and why the compat IDs are outside it — the entry is the artifact a consumer reads to decide what this version does, and leaving it broader than the code would have been worse than never writing it. Mutation matrix, both directions: restore the blanket exemption -> only the two compat legs fail remove the exemption entirely -> only the declared-param control fails The over-narrow mutant did NOT compile on the first attempt (`declared and not used: fieldsPresent`), which proves nothing about the tests, so it was rewritten to compile before being counted. A compiler-killed mutant is a failed experiment, not a passing one. The declared-param control is in the same test deliberately: without it this pin would pass on a build that abandoned the round-7 boundary outright, which is the opposite defect and just as wrong. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 18:08:06 +00:00
xarmian	4b6e321924	feat(mcp): bump ToolSurfaceVersion to 0.27 (BUG-2850) ... The contract this branch changes is advertised in the handshake under capabilities.experimental.padToolSurface, and agents branch on it. It still said 0.26. Caught by reading the artifact a CONSUMER reads rather than the code — and the repo already had the instrument: tool_surface_drift_test.go fails when instructions.md or README.md drift from the constant, so the bump immediately named both documents. They are updated with what actually changed, not just re-titled. Bump grounds are v0.26's own, and v0.25's, v0.16's, v0.10's and v0.9's: no tool name, action enum or parameter shape changed, and the behaviour did. This branch REFUSES calls 0.26 accepted: - two names for one target in a single call (parent/plan, assign/assigned_user_id, role/agent_role_id), refused even when the values match, because the names address one thing through incomparable vocabularies and the doors resolved them differently; - the same key through the `fields` object and another source with differing values (equal ones collapse); - a non-string `assign`/`role`, which one door dropped silently and the other rejected; - an empty hierarchy value inside `fields`, which promoted onto a param both doors read as "not supplied" and so reported success having detached nothing. Every one of those replaced a call that SUCCEEDED while doing something other than what it said, so the break is the fix in each case. The additive halves are in the same entry because they are one contract change: server-side coercion (a declared number/json field was unwritable from the remote transport at all), the `fields` object carrying native types, and `warnings.undeclared_fields` on write responses. Deliberately NOT refused, and stated in the entry so it reads as a decision: a top-level param colliding with a `field:[]` entry under the same name with no `fields` object. Both doors resolve that identically and it is visibly a duplicate. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 17:59:40 +00:00
xarmian	bb62cfdc95	test(mcp): derive the conflict property's population from the declared schema (BUG-2850) ... The lead's finding after round 14, and it is a sharper statement of what went wrong than mine was. The property test enumerated its sources by hand, and that hand-written list came from the same head as detectFieldConflicts. A property whose input list mirrors the implementation cannot see a source the implementation forgot — which is exactly how the round-14 defect survived it: the property SKIPPED param-vs-array pairs as "out of scope", which was the implementation's assumption restated as a test assumption. So the population now comes from the DOOR'S DECLARED CONTRACT — the live pad_item ToolDef's parameter list, which is what agents read — and every declared param must be either classified by the conflict machinery or explicitly excluded with a reason. The two lists have genuinely different origins (the tool schema vs the four key sets), which is the whole point: a test that derives its expectations from the thing it checks cannot fail. It earned its place immediately by naming ten declared params I had not classified. Each is now excluded WITH its reason, because a bare list would let a future field-writing param be silenced by adding one word to it — the round-14 mistake in miniature. The interesting group is summary/details/decision/rationale. Those DO change item state, so excluding them is a real claim rather than a shrug: they write implementation_notes / decision_log through their own actions, and those exact keys are REFUSED through `field` and `fields` (BUG-2627 / BUG-2675), so they cannot reach one key by two routes — which is the only thing this pass adjudicates. The reverse direction is checked too: every key the machinery classifies must be reachable through the declared schema, or be a documented undeclared form (the v0.16 compat IDs, and `plan`, a fields_patch pseudo-key with no top-level param). That fails if a key set goes stale against the schema. gofmt clean · go test ./internal/mcp/ green	2026-09-03 17:55:51 +00:00
xarmian	eb37c0e53c	fix(mcp): give the canonical pass full reach; equal structures collapse (BUG-2850) ... Codex round 14, two P1 — both in the round-13 restructure, and the first is the restructure repeating the mistake it was built to end. [P1] THE CANONICAL PASS DID NOT REACH THE NO-`fields` CASE. It ran from inside reshapeItemFields, which returns early without a `fields` object, so an ALIAS pair arriving through the top level and the `field` array alone slipped past: `assigned_user_id:"B"` with `field:["assign=dave"]` applies the compat ID over HTTP while stdio drops it and sends only the generic field. Two different people assigned, from one call. The tell is that round 7 had ALREADY built an always-run alias guard — for the hierarchy pair only. So the restructure meant to end guard accretion had itself left two alias mechanisms with different reach, and the pair the older one covered is exactly the pair that kept working. detectFieldConflicts now parses its own inputs and runs from both action entry points regardless of `fields`; checkHierarchyAliasAmbiguity is deleted as subsumed. One mechanism. The alias half is ungated; the SAME-NAME half stays gated on `fields`, which is the round-7 boundary the lead confirmed and is unchanged. Last-write-wins is defensible when both sources name one key and indefensible when two names address one target through different vocabularies — a slug and a UUID cannot be compared, so co-occurrence is ambiguous however it arrives. [P1] EQUAL STRUCTURES WERE REFUSED. `tags:["a"]` plus `fields:{"tags":["a"]}` is one unambiguous value, and scalarEqual had always collapsed it; the round-13 pass refused whenever either side was structured. A regression I introduced, that nothing in the suite caught because every existing tags test passes a structure on one side only. Equal structures now collapse, differing ones still refuse. The property test is widened rather than merely extended: it previously SKIPPED param-vs-array pairs as out of scope, and that exclusion is precisely where the round-14 defect lived. It now covers every source pair — 72 combinations, up from 48. Mutation matrix: re-gate the alias half on `fields` -> property fails on the param×array legs revert equal-structure collapse -> only EqualStructuredDuplicateCollapses fails make the collapse unconditional -> only DifferingStructuredDuplicateRefused fails The last two are the both-directions pair: under-apply and over-apply each fail exactly one leg, so the pins bracket the behaviour instead of agreeing with it from one side. Control kept explicit: the round-7 same-name boundary still passes on BOTH doors (internal/mcp + cmd/pad SameNameDuplicate tests), so widening alias detection did not quietly swallow the case that resolves. gofmt clean · go vet clean · go test ./... green (29 packages) context: 45.1% (session-shape)	2026-09-03 17:52:19 +00:00
xarmian	1de4fd48dc	refactor(mcp): one canonical view, one conflict check (BUG-2850) ... Codex round 13 found a fourth consecutive defect in a prior round's fix, which fired the lead's restructure trigger. The finding and the ruling are the same observation from two directions. THE FINDING. `assign`/`assigned_user_id` and `role`/`agent_role_id` are two names for one target, exactly like `parent`/`plan` — and none of the five guards standing at round 12 compared them. The alias guard knew only about hierarchy; the compat guard only about same-name collisions. So `assigned_user_id:"B"` with `fields:{"assign":"A"}` was accepted and the doors disagreed: resolveAssignName gives the explicit ID precedence over HTTP, while BuildCLIArgs drops the compat ID and emits `--assign A`. One call, two different people assigned. THE RULING. Stop adding guards. Conflict handling had accreted one at every site that noticed a problem — generic path, promoted block, alias check, compat block, canonicalization predicate — and each covered only the sources its author thought about. Round 13's finding is that shape's signature, not a new one. WHAT CHANGED. `detectFieldConflicts` resolves every source (the `fields` object, the `field:[]` entries, the promoted params, the compat IDs) to a canonical key via `fieldAliasGroups`, then refuses on the resulting map. Alias collision refuses even when the values match — the names address one target through different vocabularies (a slug vs a UUID), so "equal" is not a question this layer can answer. Same name from two sources keeps the old rule: equal collapses, differing refuses. The five guards are gone; the per-key branches now do emission only, and they run knowing the input is unambiguous. A new alias pair is one line in a map rather than a sixth guard. SCOPE, unchanged and stated: this runs only when a `fields` object is present, because reshapeItemFields does. A top-level param colliding with a `field:[]` entry and no `fields` object keeps its documented last-write-wins resolution — the round-7 boundary the lead confirmed, still pinned on both doors. EVIDENCE. All 30-odd existing case tests pass unchanged against the new structure; they are the regression net the ruling asked to keep. Added: the round-13 case tests over both directions of both pairs, and a PROPERTY test derived from `fieldAliasGroups` itself — for every alias class, every ordered pair of member names, and every pair of distinct sources, the call refuses. It covers 48 combinations and a future alias pair extends it with no edit. Mutation matrix: drop assigned_user_id from the alias map -> property fails unwire detectFieldConflicts entirely -> 54 tests fail remove the alias-collision branch -> property fails (equal-value legs) The third mutant SURVIVED at first, and the reason is the useful part: my property used differing values, so the ordinary same-canonical-key comparison refused anyway and a build with alias detection wholly removed still passed. The equal-value leg is the one only alias detection catches — exactly the semantics the parent/plan ruling established — so the property now drives both value shapes. CONVE-28 again: on SURVIVED, the mutant was faithful and the test was weak. gofmt clean · go vet clean · go test ./... green (29 packages) context: 45.1% (session-shape)	2026-09-03 17:42:42 +00:00
xarmian	af686c350d	fix(mcp): a blank top-level param does not block the fields answer (BUG-2850) ... Codex round 12, one P1 — and the fix is bigger than the finding, twice over. THE FINDING NAMED ONE KEY. `{status: "", fields: {status: "done"}}` was refused, because the duplicate check treated a present-but-empty top-level param as a competing value. `""` is "not supplied" everywhere else on this surface — promotedParamValue treats it as absent, the CLI's `status != ""` guards do, `assign: ""` is documented inert — so a client that zero-fills its optional params was refused for asking one question. Driving the whole class the key belongs to (CONVE-18: the reviewer names an instance, the fix owes the population) turned `status` into all six promoted keys, which behave identically. Round 10 fixed exactly this for the hierarchy keys and I never asked whether the same reasoning covered their siblings; it did. Third time in this unit that a guard was written for the path in front of me, and this time the guard was mine. THE SAME PROBE FOUND THE EXCEPTION, which matters more than the fix. For `assigned_user_id` / `agent_role_id` an empty string is NOT absence — it is a CLEAR to NULL, the deliberate v0.16 semantics (dispatch_http_advanced.go forwards "" verbatim for exactly these two). Applying the finding uniformly, as its wording invites, would have discarded a clear in favour of the `fields` value: a spurious refusal traded for a silent wrong write, which is the worse half. They stay conflicts, with their own pin. AND THEN A SURVIVING MUTANT CAUGHT A FALSE COMMENT OF MINE. Deleting the compat carve-out failed nothing — because the carve-out lived in a helper that only the PROMOTED block called, while the compat keys were checked in a separate block that never consulted it. Dead code, and the comment I had just written called it "the whole reason this is a function". CONVE-28's rule is what caught it: on SURVIVED, ask whether the mutant is faithful before blaming the test. The mutant was faithful; the code was redundant and the prose was wrong. Both call sites now consult the predicate, so the rule has one home and the carve-out is live — re-running the same mutant against the corrected code fails BlankCompatIDIsAClearAndStillConflicts, as it always should have. Mutation matrix: revert the blank-param exclusion -> only BlankTopLevelParamDoesNotBlockFields fails (6/6 subtests) delete the compat carve-out -> only BlankCompatIDIsAClearAndStillConflicts fails (2/2) [survived before the redundancy was removed — see above] gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 17:27:34 +00:00
xarmian	c2bb1bad34	fix(mcp,server): close three codex round-11 findings, one of them my own bad refutation (BUG-2850) ... Two P1 and one P2. The P2 is the important one, because I had already dismissed it in round 10 and was wrong. [P2 — CORRECTION] Artifact imports DO drop undeclared-field warnings, and the case is reachable. Round 10 refuted this on the grounds that artifact.Decode populates Fields only from FieldKeysForKind, so no undeclared key could arrive. That check was real, and it was the WRONG SIDE of the comparison: UndeclaredFieldKeys compares the field map against the DESTINATION COLLECTION'S SCHEMA, not against the artifact format's key list. The destination schema is editable, so a canonical artifact key can be undeclared THERE while being perfectly legal in the artifact. Verified before reinstating, not argued: narrow the conventions collection's schema to declare only `status`, import an ordinary convention carrying trigger/scope/priority — the blob stores all three and UndeclaredFieldKeys names all three. The merge is back, and the comment now records the correction rather than the refutation, so the next reader inherits the right reason. What I got wrong is worth naming exactly: I verified a true fact and then drew a conclusion one step wider than it supported, because I never asked what the OTHER operand of the comparison could be. "No key outside the artifact's list arrives" does not imply "no undeclared key arrives" unless the destination declares every key on that list — an assumption I never stated and never checked. The test I wrote at the time could not pass, and I read that as confirming the refutation instead of as the setup being wrong. [P1] An empty hierarchy value inside `fields` was a silent no-op. `fields:{"parent":""}` promotes onto the top-level `parent`, where both doors treat empty as NOT PROVIDED — so the call reported success and detached nothing. Refused now, pointing at clear_parent and the raw `field:["parent="]` form. Not silently promoted to a clear: that decides what this door MEANS, and v0.19 already made clear_parent canonical so the empty string would not have to carry it. [P1] The v0.16 compat ID params were not conflict-checked against `fields`. Never schema-declared by design, they are invisible to padItemPromotedFieldKeys and took the generic path, where the check only consults the `field` array. `assigned_user_id:"A"` with `fields:{"assigned_user_id":"B"}` made the doors disagree outright — the remote mapper reads A, stdio emits only `--field assigned_user_id=B`, because the top-level form has no CLI flag behind it. One call, two different people assigned. Conflicting values refuse; equal ones collapse to one form. Also corrected, per CONVE-23: the round-10 test asserting that `fields:{"parent":""}` conflicts with the plan alias now refuses for a DIFFERENT reason and its stated rationale had become false. The case moved to the new test with the right reason, and the field-array clear — which really is an effective directive — stays where it was, with a control leg proving the new refusal did not swallow it. Mutation matrix, each mutant from a file backup: revert the empty-hierarchy refusal -> only EmptyHierarchyValueInFieldsRefused fails (both subtests) revert the compat-ID check -> only CompatIDConflictRefused fails (both subtests) revert the import warning merge -> only the narrowed-schema import test fails gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 17:12:20 +00:00
xarmian	0a71ad3aa9	fix(mcp): an empty parent param is not a hierarchy directive (BUG-2850) ... Codex round 10: three P2, no P1. One fixed, one already filed, one refuted and reverted. [FIXED] An empty top-level `parent` was counted as an alias directive, so `parent: ""` with `fields:{"plan":"X"}` refused a perfectly good call. Every declared string param on this tool treats "" as NOT PROVIDED — it is why promotedParamValue does, and why `assign: ""` is deliberately inert — so a client that fills declared optional params with their zero value rather than omitting them got a refusal for asking one hierarchy question. My own round-9 snapshot carried this forward from the out[]-based check it replaced. Deliberately NOT applied to the other empty forms: `field:["parent="]` and `fields:{"parent":""}` are the documented CLEAR signal (BUG-2013 / BUG-2078), so they are semantically effective and still conflict. One is a param left blank, the other is an instruction that happens to look like one. Both directions are pinned, and the mutation matrix drives both: remove the empty-param exclusion -> only EmptyParentParamIsNotAnAliasConflict fails extend it to the field-array clear -> only EmptyClearFormsStillConflict fails [ALREADY FILED] The transport-dependent whitespace finding is BUG-2870, ruled out of this PR's scope. Round 10 did add something the filing missed and BUG-2870 now records it: the divergence covers VALUES too (`--field "cost= 3"` stores the number 3 remotely and the string " 3" over stdio), which is worse than the key half because both doors report success and only the stored type differs. [REFUTED, REVERTED] "Artifact imports discard createItemChecked's undeclared-field warnings." True as a code reading — this handler builds its own response shape and ignores item.Warnings — but the condition is unreachable. artifact.Decode populates Fields exclusively from FieldKeysForKind via a closed switch over a typed frontmatter struct, so a key outside that per-kind list never enters the map. Verified both ways before reverting, because the import door takes raw bytes and the hand-written case is the one that mattered: Encode drops extra keys, and Decode of a hand-written artifact carrying extra frontmatter keys drops them too. I had written the merge and a test for it before checking; the test could not pass through the public door, which is what exposed the finding rather than my fix. Reverted to a comment recording the mechanism, so the next reader — or the next round — does not re-find it. A branch nothing can enter is not defence in depth, it is a claim that something is handled when it never happens. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 16:53:14 +00:00
xarmian	56ee3a7e95	fix(mcp): require strings for fields.assign/role; fix the alias refusal's mechanism (BUG-2850) ... Codex round 9: one P1 and one P2. The P1 was REFUTED on inspection and the P2 confirmed; both produced a change, for different reasons. [P2, real] `fields.assign` / `fields.role` accepted a number, and the two doors then disagreed about it. The HTTP dispatcher's `rawAssign.(string)` turns a float64 into "" and treats it as NOT PROVIDED, silently dropping the write; stdio emits `--assign 123` and the CLI fails loudly on the lookup. Same call, one door silent and one red. Refused now at the door-independent layer, which is what stops them drifting apart again rather than teaching each dispatcher separately. Deliberately narrow: this does NOT walk back round 6's decision to accept non-string promoted values in general. `priority` may legitimately be a number in a custom schema and create has always passed such values through — a control leg pins that. `assign` and `role` are references that NAME something, where a number has no meaning at all. [P1, refuted] `fields:{"parent":"A","plan":"B"}` was already refused. But it was refused by ACCIDENT: keys process in sorted order, so `parent` was promoted into out["parent"] and `plan` collided with it one iteration later. Right answer, wrong mechanism — the refusal depended on `parent` sorting before `plan` AND on `parent` being a promoted key, and it told the caller their value conflicted with "the top-level parent param" when no such param was passed. The fields-vs-fields case is now checked against `obj` directly, and a snapshot of the original top-level params keeps that message honest. Reported as verified rather than as agreement: the finding's mechanism was wrong, and shipping "fixed" against a refuted claim would have put a false statement on the trail. Mutation matrix, from file backups: revert the identity-ref requirement -> only NonStringIdentityRefRefused fails (both subtests) revert to the out[]-only alias check -> only BothAliasesInOneFieldsObject fails Plus a PROBE that is not a mutant of the fix: removing `parent` from padItemPromotedFieldKeys leaves the alias pair still refused. Under the old code that mutation made the guard go silent, since nothing would write out["parent"] — which is the latent coupling this change removes. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 16:32:51 +00:00
xarmian	49e533d478	test(mcp,cli): pin same-name duplicate precedence on both doors (BUG-2850) ... The lead's condition on the round-7 boundary. checkHierarchyAliasAmbiguity refuses parent+plan — two NAMES for one target, which a caller can collide without knowing — but deliberately does NOT refuse a same-name duplicate (`--status A --field status=B`), because those are visibly duplicates and both doors resolve them identically. "Both doors resolve them identically" is the load-bearing half of that argument and nothing enforced it. Two tests now do, one per door, asserting the SAME outcome: the `field` entry overlays the named param, because cmd_item.go and dispatch_http_advanced.go both apply named flags first and overlay --field after. Per-door mutation matrix, run this turn from file backups: make the named param win on the HTTP door -> only the mcp test fails make the named flag win on the CLI door -> only the cmd/pad test fails Neither mutant reddens the other door's test, which is the property worth having: the doors cannot drift apart again without exactly one of these going red and the boundary getting re-examined rather than silently becoming untrue. Also filed, per the lead's ruling: BUG-2870, the padded-`field`-key divergence with NO `fields` object (`--field " effort=l"` stores an undeclared " effort" key on the CLI door and writes `effort` on the remote one). Out of scope here — it predates this PR's claim rather than defending it — and its fix is a policy call on the CLI's input contract, so it wants a ruling, not a quick patch. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 16:22:00 +00:00
xarmian	4937fd84f6	fix(mcp): canonicalize when ANY entry for the key is padded (BUG-2850) ... Codex round 8, one P2 and no P1 — the first round of this unit that did not turn up a correctness defect on the fields-vs-field seam. Round 7's canonicalization asked whether a canonical entry was PRESENT and left the array alone if one was. So `field:["effort=l", " effort=l"]` with `fields:{"effort":"l"}` kept the padded twin, and the doors then disagreed about it: HTTP trims and writes `effort`, the CLI does not and writes an undeclared `" effort"`. Transport divergence out of a call both doors accept — the shape this unit exists to remove, reintroduced one round earlier by the fix for its sibling. The predicate is now "any entry for this key is non-canonical", so the key is re-emitted once and cleanly. Collapsing the duplicate pair is not lossy: parseFieldArray already indexes both to a single value, so two entries for one key were never two writes. Mutation: restoring the round-7 predicate verbatim fails only MixedCanonicalAndPaddedDuplicatesCollapse. Round 7's two pins still pass under that mutant, which is correct — neither exercises the mixed case, and that is exactly why the new one was owed. NOT fixed here, and named so it is not mistaken for an oversight: a padded entry with NO `fields` object at all (`field:[" effort=l"]` alone) still reaches the CLI door untrimmed. That predates BUG-2850, is unrelated to the fields merge, and normalizing every entry unconditionally changes what the CLI receives for every caller — a policy change, not a defect fix. Flagged to the lead on the trail. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 16:17:49 +00:00
xarmian	13892fecf7	fix(mcp): close two codex round-7 findings on the same seam (BUG-2850) ... Both are consequences of round 6's own fixes, which is the tell that the seam — a guard written for one key shape, and the keys that do not take that path — is still the thing to keep hitting. [P1] The alias guard did not fire without a `fields` object. Round 6 put it inside reshapeItemFields' per-key loop, and reshapeItemFields returns early when `fields` is absent — so `field:["parent=A","plan=B"]` walked straight past it and extractParentLink's no-early-exit loop applied `plan` while the caller had every reason to believe `parent` was what they set. A guard a caller can step around by moving the same two values into a different param is not a guard. checkHierarchyAliasAmbiguity now runs on create and update regardless of `fields`, over the merged input. SCOPE, stated rather than smuggled: the pure-`field` form was accepted before BUG-2850 too, so this closes a pre-existing silent mis-write, not a regression. Fixed here rather than filed because shipping round 6's guard without it would advertise a refusal that any caller bypasses in one edit. Deliberately NOT extended to same-name duplicates (a `parent` param plus `field:["parent=B"]`) — those resolve last-write-wins identically on both doors, which is documented behaviour, and widening the refusal to cover them is a policy change rather than a defect fix. [P2] A padded equal duplicate was retained raw. Round 6 normalized the conflict INDEX so ` effort=l` matches `fields:{"effort":"l"}` — correct, and it closed the padded-key bypass — but the raw entry stayed in `field`, and the CLI door does not trim. Over stdio that stored an undeclared `" effort"` key and left `effort` untouched: the normalization that made the duplicate visible is what made the retained entry wrong, so the fix belongs at the same place. The entry is now re-emitted canonically, and only when the raw form actually differs, so a well-formed array keeps its contents and its order. Mutation matrix, run this turn, each mutant from a file backup: unwire checkHierarchyAliasAmbiguity -> only the round-7 alias test fails (4/4 subtests) revert the canonical re-emission -> only PaddedEqualDuplicateIsCanonicalized fails Neither mutant touches round 6's in-loop alias test, which is right: that one enters through the `fields` object and is a different path — the distinction this finding exists about. Control legs: a lone hierarchy key through the array still dispatches, an already-canonical duplicate is left byte-identical and unreordered, and the padded-entry case asserts exactly one --field is emitted. gofmt clean · go vet clean · go test ./... green (29 packages) · 18 files	2026-09-03 16:11:01 +00:00
xarmian	dfee13896d	fix(mcp): close four codex round-6 findings in the fields-object merge (BUG-2850) ... All four sit on the same seam this unit keeps failing at: a guard written for one key shape, and a class of keys that does not take that path. [P1] parent/plan alias conflicts bypassed every guard. extractParentLink resolves the hierarchy link with `for _, key := range {"parent","plan"}` and no early exit, so when both arrive the LATER key wins — but every check in reshapeItemFields matched on the SAME key name. So `fields:{"parent":"PLAN-12"}` with `field:["plan=PLAN-9"]` passed and relinked the item to PLAN-9 while reporting PLAN-12. The same alias bypass BUG-2078's round-1 review found on clear_parent, reached through a different door. Refused now in both directions and against the top-level param — and refused even when the two values are EQUAL, which is what v0.19 already does for parent + clear_parent "including via the plan alias". [P1] The conflict index was not normalized the way the door normalizes. ingestFieldKVP TrimSpaces both halves of a `key=value` entry; parseFieldArray indexed the raw halves, so `field:[" status=cancelled"]` sat under " status", missed the guard against `fields:{"status":…}` and then silently overrode it. Trimming the value fixes the mirror-image false refusal (`status= done` vs `done`). ONLY the index is normalized — `entries` stay verbatim, because the CLI door does not trim and must keep receiving exactly what the caller sent. [P1] A non-string promoted value silently no-op'd on remote update. reshapeItemFields promotes `fields:{"priority":3}` with its type intact, but hasFieldChanges and the patch loop both read `.(string)` — so the dispatcher skipped the fields_patch branch entirely and answered SUCCESS having sent no PATCH. A silent no-op reintroduced by the fix for silent no-ops, and asymmetric with create, which has always passed non-strings through. promotedParamValue now accepts any scalar; empty string still means "not supplied". [P2] Equal promoted duplicates did not collapse. `fields:{"role":"x"}` plus `field:["role=x"]` resolved the role to agent_role_id AND wrote a literal `role` key into the fields blob that no schema declares — one value, two writes, one of them an undeclared field with a warning naming it. The array entry is now dropped so the value applies once through its dedicated param. Mutation matrix, run this turn, each mutant applied and reverted from a file backup (never `git checkout` — the tests were uncommitted): drop the alias block -> only HierarchyAliasConflictRefused fails (4/4 subtests) revert index normalization -> only FieldArrayKeysNormalizedForConflicts fails (both legs) revert the duplicate drop -> only the two EqualDuplicate tests fail revert promotedParamValue -> only NonStringPromotedValueIsNotDropped fails No cross-talk: each mutant is the defect at the site its test targets, and each kills exactly that test. Control legs included on purpose — a lone hierarchy key is still accepted, padding-only value differences are not conflicts, an unrelated `field` entry survives the duplicate drop, and an empty promoted string still produces no fields_patch. gofmt clean · go vet clean · go test ./... green (29 packages)	2026-09-03 13:07:36 +00:00
xarmian	7f25283a41	test(server): pin the last three coercion call sites (BUG-2850) ... Five of the eight `CoerceFields` sites had a test that goes red if that site alone is dropped. Move, bulk move and bulk update did not, so the PR's "typed on every door" claim rested on reading the code — CONVE-19 and the shape rounds 2-5 of this unit kept finding. Why the move pins are faithful rather than green-for-free: migrateValue already permits text->number (migrate.go:190), but it returns `value`, the ORIGINAL, not a parsed float. So a text field holding "42" reaches a number-typed destination as the STRING "42", and only CoerceFields at the move site turns it into a number before validation. Assertions are on the STORED NATIVE TYPE, re-read from the item rather than taken from the mutation's own response, so a handler that answered 200 and stored the string is still red. Bulk update merges request STRINGS (status, priority), so it is observable only where the schema declares one of those keys as a non-string type; a collection declaring `priority` as a number is unusual but legal and is the honest way to reach that site. Bulk ops answer 200 with per-item failures in the envelope, so the pins read the envelope too — a status-code-only assertion would pass on a dropped coercion. Per-site mutation matrix, run this turn against these tests, each mutant applied and reverted from a file backup (never `git checkout`, which would have taken the uncommitted tests with it): drop coercion at handlers_items.go:2314 -> only TestItemFieldsCoercedOnMove fails drop coercion at handlers_items_bulk.go:683 -> only TestItemFieldsCoercedOnBulkMove fails drop coercion at handlers_items_bulk.go:499 -> only TestItemFieldsCoercedOnBulkFieldUpdate fails Each mutant is the defect at the site the test targets — not a call-site patch next to a still-correct function (CONVE-28) — and each kills exactly one test, which is the per-site discrimination the PR claims. Files restored and verified identical after the matrix; suite green.	2026-09-03 12:49:33 +00:00
xarmian	baaa236d83	fix(mcp): apply the field-array conflict guard to promoted keys too (BUG-2850) ... Codex round 5, one P1. The `fields` object vs `field: ["k=v"]` conflict guard lived on the generic path, which a promoted key never reaches: `status`, `priority`, `category`, `parent`, `role`, `assign` and `tags` all return from the promoted branch above it. So the one ambiguity this function did not refuse was the one on the keys that matter most. It did not fail closed either. The promoted branch writes the top-level param (`out["status"]`) while the array entry stays in `out["field"]`, and both the HTTP mappers and the CLI overlay `--field` entries AFTER the named flags — so the array silently won. `fields:{"status":"done"}` with `field:["status=cancelled"]` cancels the item. The same shape on `parent` relinks or detaches it. The guard now runs first in the promoted branch, before the existing top-level-param check, with the generic path's semantics: differing values refuse, an equal duplicate falls through and still promotes, and a structure against a string entry (the `tags` case) refuses as "one key cannot be both". This is the fourth consecutive round where the defect was a guard written on the generic path only, and round 4's test is why: it pins `effort`, a key that takes the generic path, so it vouched for the path the guard is on rather than for the class of keys that skips it (CONVE-19). The new test drives all four promoted shapes plus an equal-duplicate control leg. Negative control: all four conflict cases fail on the unfixed tree (run before the fix, not against a synthetic mutant); the equal-duplicate leg passes both before and after, so it discriminates refuse-on-ambiguity from refuse-on-agreement. gofmt clean · go vet clean · go test ./internal/mcp/ ok	2026-09-03 12:46:25 +00:00
xarmian	b23c0dbc6f	fix(mcp): apply the null and hierarchy guards to promoted keys too (BUG-2850) ... Codex round 4, and both findings are the same defect in my own round-3 fix: ORDERING. The null guard and the hierarchy-key guard sat BELOW the branch that promotes status/priority/category/parent/role/assign/tags onto dedicated params, so every promoted key walked around both. - `fields: {"tags": null}` reached the promoted branch and became a silent no-op, instead of the refusal the null rule documents. - `fields: {"parent": 42}` was accepted here and dropped later by the handler — the same silent-drop shape this whole bug is about, reintroduced by a guard I added to prevent a different instance of it. Both guards now run before that branch, so they apply to every key. A guard a whole class of keys bypasses is not a guard. Pinned separately from the generic-path cases, because the generic-path tests passed throughout: they never exercised a promoted key, which is exactly why the hole survived round 3. Reverting the hoist fails the new test. Well-formed promoted keys still promote — tested, so the hoist did not break promotion while closing the bypass. Gates: gofmt clean, go vet clean, go test ./... 29 packages ok. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-03 01:41:11 +00:00
xarmian	f15f60831e	fix(mcp): guard hierarchy pseudo-keys and correct the fields description (BUG-2850) ... Codex round 3. 1. [P1] A structured `plan` could silently DETACH an item. `plan` is not in padItemPromotedFieldKeys, so it fell to the generic path — and once this branch stopped refusing structures, fields:{"plan":{…}} reached the server natively. There extractParentLink reads any PRESENT non-string plan/parent as a hierarchy directive, drops the key, and on update clears the existing parent link. Lifting the nested refusal quietly opened a path where a malformed value detaches an item from its parent. Guarded specifically: these keys take a string ref and nothing else. Both `parent` and `plan` are listed, so the guard does not depend on which other set a key happens to belong to. A string ref still works — tested, so the fix did not re-refuse the normal case while closing the hole. 2. [P2] The MCP `fields` param description still told agents that multi_select and json non-scalars are "refused, not written". This diff makes that false, and a schema an agent reads is the artifact that decides what it attempts — the reporter's agent rewrote seven playbooks after believing exactly this kind of line. Rewritten to state what is true now, including the two remaining refusals (null, structured parent/plan) and that structured values need the remote transport. Gates: gofmt clean, go vet clean, go test ./... 29 packages ok. Removing the hierarchy guard fails its test. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-03 01:33:54 +00:00
xarmian	2cf9f0035a	fix(mcp,server,cli): three codex round-2 findings (BUG-2850) ... 1. [P1] The structured-value refusal was in the wrong place and killed the fix. It went into BuildCLIArgs, which env.Dispatch runs for BOTH transports before handing off to whichever Dispatcher is configured — so it blocked the remote /mcp door too, and the native-field handling that is the whole point of this change was never reached. Moved into ExecDispatcher, which IS the stdio door. My own test could not see this: it called mapItemCreate directly, so it vouched for the mapper and not for the path that reaches it — CONVE-19's exact shape, in a unit where I had already written binding tests for the other half. The tests are now split along the two claims the first version conflated: nested values REACH the dispatcher (the remote door is unblocked), and refuseStructuredFieldsOverCLI refuses them at the CLI door naming the transport. 2. [P2] The CLI warning sat after the `--format json` early return, so the caller most likely to have sent a mistyped key — one piping stdout into a parser — was the one caller who never saw it. Moved above the return, and out of the `ref != ""` branch it was also trapped in. Still stderr. 3. [P2] A nil value in fields_patch DELETES the key (store/items.go), so reporting it as an undeclared field told the caller a field was stored that the same request removed. Filtered at the patch site, not inside UndeclaredFieldKeys, because nil means "store JSON null" on the full-fields path where reporting it is correct. Gates: gofmt clean, go vet clean, go test ./... 29 packages ok. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-03 01:07:45 +00:00
xarmian	dc3fc2d50e	feat(server,cli): name undeclared field keys on the write response (BUG-2850) ... Undeclared keys are ACCEPTED — the census found 168 live values under 14 such keys, and refusing them would break read-modify-write on items nobody edited wrongly. But once stored, a typo and a deliberate extra field are indistinguishable, so the write now says which keys it did not recognize. - models.Item gains `Warnings *ItemWriteWarnings` with `undeclared_fields`, omitempty and additive. NEW API SURFACE: item write responses carried no warnings element before. Wrapping the response as {item, warnings} was the alternative and would have broken every existing parser; a clean write is byte-identical to before. - items.UndeclaredFieldKeys consults models.IsReservedItemField rather than re-listing the reserved set — that set exists so callers ask, and its doc comment records what re-listing cost last time. So a write carrying implementation_notes or github_pr reports nothing. - fields_patch reports only the PATCHED keys. A stray key already on the item is not something this write introduced, and naming it on every touch would train the reader to ignore the field. - The CLI prints one line to STDERR. Never stdout: `--format json` output is piped into scripts, and a warning there would corrupt the JSON they parse. - CLAUDE.md documents the element as new surface. Controls: never attaching the warnings fails the pin; reverting the HTTP mapper's native overlay fails the remote-door type test; dropping the reserved-key exclusion fails its own test. Two coverage gaps the controls FOUND rather than confirmed, both now closed: the remote door's native overlay was covered by no MCP test at all (a revert left the package green), and the reserved-key exclusion had no test either. Both were written after the control survived, which is the only reason they exist. Gates: gofmt clean, go vet clean, go test ./... 29 packages ok. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-03 00:54:56 +00:00
xarmian	dd919dd0a9	feat(mcp): carry the fields object with its JSON types intact (BUG-2850) ... Second half of BUG-2850, ruled after a census: undeclared keys stay accepted on every door, and a value's native JSON type is preserved wherever the encoding carries one. Only two doors carry a type at all. The remote /mcp `fields` OBJECT param does — and the catalog was destroying it, flattening every value into `field: ["key=value"]` before dispatch. The direct HTTP API does. The other three (remote `field:[…]`, CLI `--field`, stdio MCP, which dispatches through the CLI) are string-by-construction: `key=value` carries no type to preserve, and a string is the correct and complete representation of `cost=42` typed at a shell. So this does not "make every door preserve types" — it stops destroying the types the object form already had. - The catalog merge now emits the native map alongside the string entries, so each transport takes what it can use. Both forms describe the same input; they differ only in fidelity. - The HTTP create and update mappers overlay the native map LAST, so it wins over the stringified copy of itself. - hasFieldChanges consults the native map. Without that a NESTED-ONLY update emits no `field` entry at all, so it reported success and wrote nothing — the silent-drop shape this bug is about, arriving through the fix for it. PR #1159's blanket refusal of nested values is LIFTED, as ruled: its precondition (server-side coercion) landed in ae793e6f, and an object cannot be preserved through a door that refuses it. The refusal moves rather than disappears — BuildCLIArgs now refuses a structured value naming the TRANSPORT limit, because stdio builds `--field key=value` and would otherwise discard the key silently. The message says which door cannot express it rather than implying Pad cannot store it: the reporter's agent rewrote seven playbooks' argument specs after reading the old refusal as a verdict on the data. NULL stays refused, deliberately and against the letter of "preserve native types". "Store JSON null" and "clear this field" are both readable from it, Pad already has an explicit clear vocabulary (clear_parent, clear_assigned_user), and giving null a silent meaning inside a bug fix would be inventing semantics. If a clear-by-null is wanted it should be ruled. Gates: gofmt clean, go vet clean, go test ./... all packages ok. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-03 00:32:50 +00:00
xarmian	b451fb50de	test(server): bind the coercion to its call sites, and enforce copy/preflight agreement (BUG-2850) ... CONVE-19: wiring is a claim. The previous commit threaded CoerceFields through eight validate sites; a test at the items package vouches for the function, not for any of those bindings. - Three HTTP-door tests (create, update, fields_patch) assert the stored NATIVE TYPE, not that the request returned 201 — a test that only checked the status passes on an implementation that stores the string, which is the shape the reporter described. - A text field holding "42" must stay a string in every one of them. Fixing this bug by coercing anything that parses would retype real data. - An un-coercible value must still be REFUSED with the validator's existing message, so coercion is not quietly widening what the server accepts. - TestCopyAndPreflightCoerceIdentically makes the cross-package invariant real. The preflight validates in internal/server and the copy in internal/store; both files carry a comment saying they must match, and a comment protects nobody. The assertion is agreement FIRST — whatever they do, they must do the same thing — and only then that both accept and the copy stores a number. Controls, each run against the mutated tree: - CoerceFields reduced to the identity function (the unfixed build) fails the two door tests and the items typing test. - Dropping the call at CREATE alone fails only the create test; dropping it at FIELDS_PATCH alone fails only that one. The bindings are individually covered, not covered in aggregate. - Coercing in the copy but not the preflight FAILS, and so does the reverse. Both drift directions are caught. BOUNDARY, stated rather than implied: four of the eight sites — move, bulk update, bulk move, and the migrated-schema paths they share — are wired identically but have no test that fails if that specific wiring is dropped. They are covered by the existing suites for their own behaviour, not for coercion. A follow-up should extend the door tests to them. Gates: gofmt clean, go vet clean, go test ./... 29 packages ok. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 23:38:26 +00:00
xarmian	ae793e6fa6	fix(server,store,items): coerce field values to their declared types server-side (BUG-2850) ... The write doors disagreed about what `key=value` means. The CLI has coerced by schema type since BUG-1125, and local stdio MCP inherits that by shelling out to the binary — but the remote /mcp transport builds its field map in ingestFieldKVP with `dst[key] = val`, so every value arrives as a string. validateFieldType then correctly refuses a string for a declared number or json field, and the net effect was that an MCP agent on that transport could not write those fields AT ALL: every attempt a 400, not a mis-typed value. Measured before writing anything (repro table on BUG-2850's trail): CLI and stdio MCP store 42 and an array; the HTTP door 400s on both; an UNDECLARED key is stored as a string on every door. items.CoerceFields(fields, schema) converts strings to the declared type — number via ParseFloat (NaN/±Inf refused, because json.Marshal cannot encode them and the ignored downstream error would silently drop the whole payload), json/multi_select via Unmarshal, checkbox via ParseBool — and is applied immediately before every Validate* call. Three deliberate non-behaviours, each with a test: - A value that will not parse is left as the string for the validator, so the existing "must be a number" error still fires. Coercion invents no error path, and cannot turn a currently-PASSING write into a failure. - Non-string values pass through untouched; an int stays an int. - Text-typed fields holding "42" stay strings. Coercing anything that parses would retype real data while fixing the bug. Not folded into ValidateFields, though that would be the single call site: a function named Validate that mutates its input is a trap, and two callers re-marshal the map they pass. THE POPULATION IS 8 CALL SITES, and finding them took two sweeps. The first was scoped to internal/server and found 7; the copy path validates in internal/store (items_cross_workspace_copy.go), which only a repo-wide sweep sees. The preflight and the store-side copy now carry cross-references to each other: the preflight exists to PREDICT the copy, they live in different packages, and that is exactly how they would drift unnoticed. The undeclared-key half of BUG-2850 is untouched and marked as a decision point in CoerceFields — refuse/warn/keep is with Dave. A test pins today's keep behaviour so the ruling lands as a deliberate change. The CLI's parseFieldFlag deliberately STAYS: it is why two of four doors are correct today, and removing it alongside its replacement would put all four at risk of one mistake. Retiring it is a follow-up. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 23:30:26 +00:00
xarmian	70099c2724	Merge pull request #1239 from PerpetualSoftware/fix/bug-2848-pane-jk-anchor ... fix(web): capture the pane-follow target at keypress, not when the timer fires (BUG-2848)	2026-09-02 18:43:14 -04:00
xarmian	af24997c72	fix(web): re-resolve the follow target by id at fire time (BUG-2848) ... Codex round 1, P2, and a real latent bug in the previous commit. Capturing the item OBJECT and reusing it 140ms later keeps a stale snapshot: a rename during the debounce changes the slug, `openItemPane` builds the URL from that slug, and the id-only existence check passes happily on the way to a dead URL. What is captured is now the IDENTITY — `targetId` — and the callback re-resolves the current row from `filteredItems` before opening it. That still follows a row that MOVED, which is the whole point of the fix, and still skips one that was DELETED, while picking up any change to the row itself. Also answering the round's second P2 in the spec rather than in code: the race is probabilistic and cannot be made deterministic without a seam in the page. The asymmetry is what makes that acceptable, and it is now written down — a round that misses the 140ms window still PASSES on a correct build, because the cursor moves, the pane follows and the intended row is where it should be. So missing costs power, not correctness; the failure mode is a false green, never a false red. Three rounds put a false green around 1 in 1700 against a build that loses the keypress 11 times in 12. pane-follow-live-list + pane-controller: 44/44 across both projects.	2026-09-02 22:07:46 +00:00
xarmian	1a76531eb3	fix(web): capture the pane-follow target at keypress, not when the timer fires (BUG-2848) ... The list is SSE-live and the pane-follow is debounced 140ms. The callback re-read `filteredItems[focusedIndex]` when the timer fired, which made a keystroke depend on the list holding still for those 140ms. It does not. The failure was silent, and that is what made it hard to see. `j` advanced `focusedIndex`; an item arriving during the debounce shifted every index below it, sliding the PANED item down onto that very index; the callback read it back, found "the focused row is already the paned item", and returned through its own guard. No cursor move, no re-target, no error — a discarded keystroke. The target is now captured BY IDENTITY at keypress time. The callback still re-checks pane state and that the row still exists — identity, not position, so a row that MOVED is followed correctly and only a row that was DELETED is skipped. MEASURED, because the first two explanations were both wrong. The trail's diagnosis was that an insert leaves `focusedIndex` behind so `j` lands on the already-open row. A snap-back $effect re-syncs the cursor to the open item on every `filteredItems` change and prevents exactly that; a pin that waited for the row to settle passed every candidate assertion. So the second hypothesis was that the snap-back undoes the cursor move during the debounce, and the fix was to suppress it while a follow is in flight. Measured: 12 of 12 failures, WORSE than the 11 of 12 baseline. The stale index lands on the paned item by itself; the snap-back was never the culprit. Capture-at-keypress, same harness, same sweep size: baseline (unfixed) 11/12 lost the keypress suppress snap-back 12/12 lost the keypress capture target at keypress 0/12 across all three measured properties — the pane re-targeted, the cursor moved, and the pane landed on the row that was actually below the cursor. The new spec CAUSES the race rather than waiting for it: it seeds a row above the cursor, then lands a second insert across the keypress inside the debounce. Three rounds per run, because one round caught the unfixed build 11 times in 12 and three make a false green not worth reasoning about. Counterfactual against the unfixed controller: 3 of 4 desktop runs fail with the bug's signature — `Expected: "DOC-16"` (the intended row) versus `Received: "DOC-15"` (the row that was already open). It asserts three things and none is redundant: `retargeted` alone passes if the pane wanders anywhere; `cursorMoved` alone passes if the cursor moves and the pane ignores it; `intended` is what pins the actual contract. A fourth that suggests itself — "the pane agrees with the focused row" — passes VACUOUSLY on the bug, since cursor and pane are then both stuck on the opened row. It was measured doing that and is deliberately absent. pane-controller.spec.ts is unchanged and still green (21/21). Its intermittent failure was this defect, not the shared-workspace pollution it was filed as — it just could not cause the race, so it only caught it when a sibling test's seed happened to land in the window.	2026-09-02 22:02:11 +00:00
xarmian	58f50909af	Merge pull request #1234 from PerpetualSoftware/feat/idea-2843-composer-quote-handle ... feat(web): comment on a selection, with comments back under the item content (IDEA-2843)	2026-09-02 17:39:27 -04:00
xarmian	e92f6f235d	fix(web): the sidebar footer's Settings label wrapped once the GitHub link joined the row (BUG-2844) ... MEASURED, not eyeballed. The desktop sidebar is 260px wide, less 24px of .sidebar-inner padding, so the footer row has 235px. Four 32px controls and four 8px gaps are fixed cost; .settings-btn is the only flex:1 item and got what was left — a 75px box, 51px of content after its 12px padding. The label "⚙ Settings" needs 56.3px. Five pixels short, so the gear and the word landed on separate lines and the row grew from 33.9px to 51.7px. The five pixels arrived with the GitHub link (IDEA-2711, PR #1229): a 32px control plus a fifth gap took 40px out of a box that had about 22px of slack. space-2 -> space-1 on the row returns 16px and the Settings padding another 8px, all of which lands in the one shrinkable item: a 75px content box against 56.3px of text, a 33% margin rather than the 5% either change alone would have left. Measured after: one line box, row height back to 33.86px. ONLY THE DESKTOP ARM WAS BROKEN, which the dispatch's "every width the layout supports" is what surfaced. Below 768px the sidebar is 280px AND the collapse button is gone, so the row carries four controls in 255px and the label had 135px to itself. It measured one line box before this change and still does — and both mobile legs of the new spec PASS against the unfixed CSS, which is what makes the desktop failures mean something. .github-btn also gains `flex-shrink: 0`, which every other control in the row already had. Harmless today — its automatic minimum size equals its 32px content box — but it made the row's one shrinkable item ambiguous, and .settings-btn is meant to be that item. THE TEST IS AN E2E SPEC BECAUSE NOTHING ELSE CAN HOLD IT. jsdom performs no layout, so a vitest render of Sidebar.svelte reports identical geometry with and without the bug. It asserts LINE BOXES rather than row height: height grows for other reasons and could stay put through a wrap, while Range.getClientRects() returns one rect per line, so the count is the question itself. Counterfactual run against the reverted declarations: desktop fails "Expected: 1, Received: 2", the shrinkable-control leg fails on the extra item, mobile stays green. Gates: web unit tests 115 files / 1978 tests green; svelte-check 0 errors (the 6 warnings are pre-existing, in files this does not touch); go build and go vet clean. The full Go suite was NOT re-run locally — no Go file changed — and CI covers it; naming the narrowing rather than reporting a leg I did not run.	2026-09-02 17:37:29 -04:00
xarmian	0900be6241	chore(deps): bump golang.org/x/crypto to v0.56.0 (BUG-2851) ... Two advisories published 2026-09-02 19:12Z (GO-2026-6354, GO-2026-6355; DoS in golang.org/x/crypto/ssh, fixed in v0.56.0) made govulncheck fail the Nix job on runners whose vulnerability database had them — intermittently across runners, not as a threshold: main at 704ba874 and a PR tip based on it passed while another failed on an identical dependency tree, four minutes apart. x/crypto/ssh is not linked into pad (go list -deps ./cmd/pad shows no crypto/ssh; go mod why: bcrypt), so this is a CI unblock, not an exposure. The bump beats an accepted-advisories entry: an exception would encode "not linked today" as permanent and would sit beside a check that disagrees with itself. go.mod one line, go.sum two lines, nix/package.nix vendorHash one line. No nix on the build box, so the hash was lifted from CI's own mismatch on a lib.fakeHash placeholder, which is why it is a build-sourced value and not a guess: specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= got: sha256-8L7gH7Yy5+Fig3wK2SPLYSJjcY9nF/jumQ7PATJ3RIE= Squashed so the placeholder commit (fails to build by design) never enters main's history. Gates on the tip: Nix green, Go suite green on SQLite and Postgres uncached (PG legs verified by timing), lint 0, vet clean; CI 7/7 on 51a0efd2. Claude-Session: https://claude.ai/code/session_01TkxKnJpLgk5UxKS8T896dk	2026-09-02 16:52:22 -04:00
xarmian	7a7e9d669b	fix(web): the selection toolbar is a row again (IDEA-2843) ... Codex round 7. `.bubble-menu` had no layout of its own. Its buttons are themselves `display: flex`, so they are block-level and STACK — invisible while the menu held one action, wrong the moment Comment joined Extract. It also falsified the dimensions `positionMenu` clamps against, so the menu drifted over the text it points at. A row layout on the container; the expanded state opts out, since the extract form lays itself out. Layout is not observable in jsdom, so the assertion lives in the e2e: the two buttons share a row (y within 4px) and Extract sits to the right of Comment. Removing the row layout fails it in a real browser — verified. Declined, with the reason recorded in the code: "Comments 1+" can appear when the only unfetched entries are activity or versions. `+` reads as a LOWER BOUND, and a lower bound of 1 over exactly one comment is true. Knowing whether more comments exist means fetching the rest of the feed, so the alternative trades a true imprecise count for a confident wrong one. Gates: 119 files / 2005 unit tests, svelte-check 0 errors, e2e 2/2 on the selection spec against a rebuilt binary. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	4ebad409a2	fix(web): gate the composer's item identity during A→B navigation (IDEA-2843) ... Codex round 6. 1. [P1] During an A→B navigation `item` still holds A while loadData fetches B, so ItemTimeline received A's itemId/collectionId beside B's itemSlug — and an attachment dropped in the composer inside that window is associated with the WRONG item. The wiring is PRE-EXISTING and identical on main. What changed is the exposure: the composer used to sit behind the Activity tab, and tabs reset to Details on an item switch, so reaching it inside the load window took a deliberate tab click. It is now on the tab you land on. Widening a latent hole is the same as opening one, so it is fixed here. Fixed by feeding honest inputs rather than adding a gate: the host passes itemId/collectionId only while `itemMatchesRef`, and ItemTimeline's canEdit already derives false without them, so the composer hides until the identities agree. 2. [P2] A load failure's banner outlived it — `loadMore` set `error` and never cleared it, so a successful retry left the failure sitting beside the entries it claimed had not loaded. Newly visible because the error is mirrored to the tabs now. Test boundary, stated: the new test covers ItemTimeline's half of the gate (no identity ⇒ no composer), verified by a control that flips its default to permissive. The host's half — the `itemMatchesRef ? … : undefined` — is not unit-testable here, since ItemDetail cannot be mounted in jsdom. Gates: 119 files / 2005 unit tests, svelte-check 0 errors. E2E: 37 passed across the five affected specs. An earlier run of that same set had one failure — capstone's "stale back-settle" nav test — which did not reproduce alone or in an identical re-run, and sits outside this diff's surface (history back-settle and drill targeting; nothing here touches either). Recorded rather than dropped. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	09a183844d	fix(web): a submit no longer erases what arrived mid-flight; empty states wait for the last page (IDEA-2843) ... Codex round 5. 1. Data loss, in the handle I added. `doSubmit` clears the composer on success, and a quote pushed in through `appendMarkdown` during the round trip was cleared with it — the quote simply vanished. The clear now requires the composer to still hold what was SENT. This also fixes a PRE-EXISTING loss by the same mechanism: text the user typed while a submit was in flight was erased too. It is the same class as the item-identity capture already guarding this path (PLAN-2105 / TASK-2112) — that one asks "is this still the same item", this one asks "is this still the same content". 2. Empty states appeared while more pages remained. A first page carrying only other kinds made a filtered view say "No versions yet." before the pages that would have contradicted it were fetched — a claim about the item made from one page of a feed. Both views now wait for the last page; until then the "Load more" button is what the reader sees. Gates: 119 files / 2004 unit tests, svelte-check 0 errors. The mid-flight fix has a negative control — restoring the unconditional clear fails the new test. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	b82c689eda	fix(web): pagination honours the caller's filter; the host's Load more is styled (IDEA-2843) ... Codex round 4. 1. Filtered pagination, fixed properly this time. Round 1 gave the TABS a host-side retry wrapper and left the same defect in the owner's own button: the Comments view could also page without showing a new comment. Fixing the instance and not the class, twice on the same defect. `loadMore(forKinds?)` now takes the caller's view filter and the hop loop counts only entries that filter admits, defaulting to the component's own `visibleKinds`. Whoever pressed the button says what progress means. The host wrapper is deleted — MAX_EMPTY_HOPS already bounds the walk, so the six-round loop on top of it was compensation for the missing filter. Caught while wiring it: the owner's own button was `onclick={loadMore}`, which passed the MouseEvent as `forKinds`. svelte-check found that; no test would have. 2. The host's "Load more" was unstyled. Its class name matches ItemTimeline's, but Svelte scopes styles per component, so the copied NAME got browser defaults and nothing warned — CSS is the part no test here asserts. Styles copied over with the reason recorded. Swept the other class names the split moved across that boundary (entry-list, compose, timeline-header, entry-count, empty): none is used in the host, so the population is this one. Gates: 119 files / 2003 unit tests, svelte-check 0 errors. The pagination fix has a negative control — restoring the unfiltered break fails the new test, which needed descending fixture timestamps to avoid passing for the unrelated "cursor did not move" reason. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	abe8d93c10	fix(web): per-view titles, counts and empty states; a failed quote is not silent (IDEA-2843) ... Codex round 3. 1. The split left every view describing the WHOLE feed. The comments section was headed "Timeline" with a count of every entry, so an item with three activity entries and no comments read "Timeline 3" over an empty list — a count of things the reader cannot see. And `showEmpty` was computed over the whole feed, so a view whose own slice was empty rendered nothing at all: no entries, no explanation. Title, count and empty state now describe what the view RENDERS. `title` and `emptyLabel` are props: "Comments" / "No comments yet." on Details, "No changes yet." / "No versions yet." on the tabs. The deliberate choice this reverses is mine — I passed showEmpty over the whole feed on the grounds that a filtered-out tab must not claim the item has no history. Right premise, wrong fix: the answer is to say something true about the slice, not to say nothing. 2. A failed quote was silently discarded. `appendMarkdown` returns false precisely so "did nothing" is distinguishable from "inserted" — and the only caller threw the boolean away and hid the menu, putting the silent no-op back exactly where the handle was built to remove it. A false now keeps the menu and the selection and reports it. Gates: 119 files / 2002 unit tests, svelte-check 0 errors, 14 e2e in desktop-chromium against a binary rebuilt from this tree. The discarded-return fix has a negative control — restoring the bare `onComment(...)` call fails the new test. (A first attempt at that control failed on shell quoting and silently ran against UNMUTATED code; it was re-run properly before this claim.) Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	8d7fdc22f3	fix(web): consult the error state everywhere success was assumed (IDEA-2843) ... Codex round 2, and both findings are follow-through misses on my own round-1 fix: I added an `error` to the mirrored feed and then left the code that assumes a load succeeded reading only `entries` and `hasMore`. - A failed load rendered the error AND "No timeline entries yet." One is a statement about the ITEM; a failed load knows nothing about the item, so the pair says something false beside something true. `showEmpty` now consults `error`. - The filtered "load more" wrapper retried a dead server up to six times per click. `loadMore()` catches and resolves, leaving `hasMore` true, so the loop had no reason to stop. It bails on `error` now. CONVE-18 sweep rather than the two named instances. The population is every consumer of the mirrored feed — six read sites in the markup, four in the wrapper. Two were defects (both above). One is a deliberate non-change: the "Load more" button still renders while an error is showing, because that is the retry affordance. The class also reaches the OWNER, where the same empty-beside-error contradiction is PRE-EXISTING on main and is fixed here too, since leaving it would mean the comments view kept the bug the tabs just lost. The regression test asserts the mounted owner's DOM. A first version recomputed `entries.length === 0 && !loading && !error` and asserted that — which passes whatever the component actually renders — so it was replaced. Gates: 119 files / 2001 unit tests, svelte-check 0 errors. Reverting the owner's guard fails the new test. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	5c0d8e9d34	fix(web): three codex round-1 findings on the timeline split (IDEA-2843) ... All three are consequences of the two-view split that the split's own tests did not reach. 1. Multi-paragraph selections lost their paragraph breaks. The bubble menu builds `selectedText` with `textBetween(..., ' ')` because Extract uses it as an item TITLE, where newlines would be wrong — so quoting through it flattened two paragraphs into one run-on line. Worse, it made `toBlockquote`'s blank-line handling unreachable from production: that behaviour had a passing test and no call site that could produce it. The quote now re-extracts with a paragraph separator, and a test asserts the blockquote's blank line end to end. 2. Activity and Versions rendered a FAILED load as "No timeline entries yet." Loading and error were the owner's states and did not cross the mirror, so an unreachable server and an empty timeline looked identical on the tabs that only render entries. `error` joins the mirror; both states render. 3. "Load more" could visibly do nothing on a filtered view. The owner's hop loop stops as soon as a page adds an entry of ANY kind, so a page of pure comments ends it having added nothing to the changes view. Pre-existing on Versions; widened to Activity when comments moved off it. The host now pages until THIS view's list grows, bounded at six rounds — not fixed in the owner, which would have to know what the other view is rendering. Gates: 119 files / 2000 unit tests, svelte-check 0 errors. Fixes 1 and 2 have negative controls: reverting to the space-joined text fails 1, dropping `error` from the mirror fails 1. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	346a5c92a9	feat(web): Comment action on the selection toolbar, quoting into the composer (IDEA-2843) ... GitHub #1228. Selecting a passage in an item's content now offers Comment beside Extract; it quotes the selection as a markdown blockquote into the comment composer under the content, appending after a blank line so an in-progress draft survives. The selection is NOT consumed — unlike Extract, which replaces it with a wiki-link — so a reader can quote the same passage twice or keep reading. - toBlockquote() prefixes EVERY line including blank ones. An unprefixed blank line ends a blockquote in markdown, so quoting two paragraphs without it silently drops the second out of the quote and leaves it looking like the commenter's own words. - The action renders when the host supplies `onComment`. A composer to quote into IS the capability; a flag that is always true beside a callback that is always supplied would be two ways to say one thing. - The button's accessible name is "Comment on selection". The composer's submit button is also named "Comment", and two identically-named buttons with different effects is a real ambiguity for name-based navigation — found by the first end-to-end run failing on a locator, not on behaviour. A NEGATIVE result, measured and kept. The action was briefly gated peek-independently, reasoning that a peeking master keeps a live composer (BUG-2263) but could not act on a selection. That state does not exist: a drag-selection in a peeking master RE-ACTIVATES it (focus-follows-editing, PLAN-2179 DR-2), so a selection and a frozen master never coexist. The gate is back on `mutationsEnabled`, and e2e/selection-comment-peek.spec.ts asserts the re-activation so a future change that makes selections survive the freeze turns red there instead of quietly reopening the question. Gates: 119 files / 1998 unit tests, svelte-check 0 errors, and 37 e2e in desktop-chromium — the 2 new ones plus the 35 in the four specs the comment relocation touched, run against a binary built from this tree. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	7aef246dcd	feat(web): comments move under the item content; Activity keeps changes (IDEA-2843) ... GitHub #1228. Reviewing an agent-written doc meant many small comments, and every one cost a trip to the Activity tab and back. TASK-2294's own spec put an activity preview on the Details panel; it never shipped, and the comments being tab-only is the half that was left. Dave ruled the full move. One component cannot render in two DOM locations, so ItemTimeline stays the SINGLE owner — one fetch, one SSE subscription, one composer — mounted under the content on Details rendering comments, and mirrors its feed out through a new bindable `feed` prop. The Activity and Versions panels render that same feed through a second TimelineEntryList. - The mirror publishes the WHOLE feed, not the owner's rendered slice. The owner renders comments only, so publishing `visibleEntries` would leave both tabs permanently empty with nothing to report. Tested, and the one-word mutation fails it. - `loadMore` rides in the mirror: pagination is a property of the ONE feed, and a tab that can show older entries but not ask for them is a dead end. - The kind partition is three shared constants with an exhaustiveness check, not literals at the mount sites. A kind in none of them renders NOWHERE — which is how note/decision shipped invisible the first time (BUG-2301). Adding a kind to TimelineEntry without routing it is now a build error or a failing test rather than a silent hole. - The comments section carries its own {#key itemSlug}: it left the block that used to provide that remount, and dropping the guard would have been invisible. It wraps only the timeline — the collab editor must never be keyed. Five e2e specs asserted comments behind the Activity tab and are updated. attachment-lifecycle's tab round-trip is preserved deliberately: its claim is that the panel is CSS-hidden rather than unmounted, so it now goes out to Activity and BACK rather than asserting against a hidden panel. Gates: 117 files / 1987 tests pass, svelte-check 0 errors. Both new properties have negative controls — publishing the rendered slice fails 1, unrouting a kind fails 2. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	f932469380	refactor(web): extract TimelineEntryList from ItemTimeline, no behaviour change (IDEA-2843) ... Comments move under the item content on Details while Activity keeps changes and Versions keeps versions, so the one feed has to render in two DOM locations. One component instance cannot be in two places, and the constraint on this work is ONE subscription and ONE composer — so the rendered list becomes its own presentational component and ItemTimeline stays the single owner of fetching, SSE, pagination, the attachment probe, the paint fence and every mutation. This commit is the extraction only. Nothing moves location and no behaviour changes; the second mount site is the next commit. - TimelineEntryList.svelte: the entry loop, rail chrome, the five card branches and their CSS, lifted verbatim. `listEl` is bindable because the owner's delegated lightbox listeners and imperative image-a11y pass attach to the container and stay with the owner. `showEmpty` is passed rather than derived from the rendered entries, preserving the owner's condition over the WHOLE feed — a tab that filters everything out must render an empty list, not claim the item has no history. - The comment-card callbacks are optional here with no-op defaults: a list rendering no comments has nothing to hand them, and a card that could call one only renders when the owner supplied the real handler. Evidence, and the reason it counts: the existing suite passes UNCHANGED — 116 files, 1983 tests — and svelte-check reports 0 errors. A refactor whose only claim is "nothing changed" is exactly where an untouched suite is the right instrument, but only if it actually exercises the moved markup. It does: rendering the list over an empty array instead of `entries` fails 64 tests across 7 files. Claude-Session: https://claude.ai/code/session_011Q4b1iHtJtSyMs7BA2ySxo	2026-09-02 19:42:50 +00:00
xarmian	a2bdd904a5	docs,test(web): correct two claims the mutation matrix refuted (IDEA-2843) ... Both corrections are to MY OWN rationale, not to behaviour. - The setContent-over-insertContentAt comment read as a defect avoided. Measured: swapping to insertContentAt leaves all five tests green, so both routes preserve the blockquote today. Restated as what it is — a preference for not depending on normalizeInline's leading-<p> rule — and marked explicitly unenforced. - The explicit `empty = editor.isEmpty` was inert: dropping it leaves the suite green, because setContent emits an update by default and onUpdate maintains the flag. Removed. The test asserting submit becomes enabled is the real guard, and it goes red if a tiptap bump flips that default. The test file's claim that its blockquote assertion catches an insertContentAt implementation was false for the same reason; it now states what the assertion does catch (a genuine flatten, verified) and what it does not.	2026-09-02 19:42:50 +00:00
xarmian	9363dbb749	feat(web): imperative appendMarkdown handle on CommentEditor (IDEA-2843) ... The selection toolbar's forthcoming Comment action needs to drop a blockquote of the reader's selection into the ALREADY-MOUNTED composer. The obvious route is a silent no-op: CommentEditor reads `content` once, inside `new Editor({...})` in onMount, and has no $effect syncing it, so writing the prop on a live composer drops the text with no error. - appendMarkdown(markdown): appends after a blank line when a draft exists, never replaces; returns false when there was nothing to insert or no live editor, so a caller can tell 'inserted' from 'did nothing'. - setContent (block parse) rather than insertContentAt: tiptap-markdown overrides insertContentAt with { inline: true }, where a blockquote survives only incidentally. - A {#key} remount was the ruled-out alternative: it would destroy an in-progress draft, which is what doSubmit's identity capture (PLAN-2105 / TASK-2112) exists to protect. Tests assert the quote TEXT and its blockquote tag, not the composer's visibility — the broken version opens the composer too.	2026-09-02 19:42:50 +00:00
xarmian	704ba874d4	Merge pull request #1233 from PerpetualSoftware/fix/bug-2810-nul-repair ... fix(store,server,cli): count and repair the legacy NUL population (BUG-2810)	2026-09-02 15:29:10 -04:00
xarmian	e4415ddd04	fix(cli): name the skipped-table suspects instead of counting them (BUG-2810) ... Codex round 12, polish rather than a defect. The advisory reported how many values in non-migrated tables mention a NUL escape, and then made the operator run `pad db scan-nul` to learn which — when the rows were already in hand. They are listed now, in the same shape as every other row this command prints. The test asserts the table.column appears rather than only the surrounding phrase, so a regression to a bare count fails it.	2026-09-02 18:42:36 +00:00
xarmian	d9f3fe3881	fix(cli): filtering suspects out of the check also filtered them out of the report (BUG-2810) ... Codex round 11. Round 10 stopped probing suspects from tables the migration does not copy, which was right — but it also dropped them from the output, while the comment two lines below still claimed "the others are still REPORTED". A legacy shadowed-NUL in activities.metadata produced no warning at all. They are now COUNTED and named, pointing at `pad db scan-nul` for detail. Counted rather than probed on purpose: whether one is actually fatal can only be answered by the destination, and asking would put them back inside the fail-closed rule the filter exists to keep them out of. The test captures stderr and asserts the advisory appears, and is mutation-verified: suppressing the notice fails it with "the suspect was filtered out of the check AND out of the report". THIS IS THE THIRD TIME on this branch that the same shape has appeared — a filter that is right about what to ACT on quietly becoming a filter on what to SAY. The first was the scan dropping suspects entirely; the second was the preflight refusing on tables it does not copy and then, fixing that, going silent about them. Each fix was correct about the action and wrong about the reporting, and each time the comment stayed true while the code stopped being. Worth naming as the pattern rather than as three unrelated defects.	2026-09-02 18:29:55 +00:00