|
| 1 | +# #272 blockers 3 and 4 — implementation report |
| 2 | + |
| 3 | +Date: 2026-07-31 |
| 4 | +Base: 94cbf82 (verified ancestor before any write) |
| 5 | +Commits: `7b4dbf3` (blocker 3 + fold-in), `<blocker-4>` (blocker 4 + this report) |
| 6 | +Both unpushed. Consumer: the next session preparing the #272 response, and |
| 7 | +the dispatcher deciding the deployment boundary. |
| 8 | + |
| 9 | +## Blocker 3 — conversation-derived writes at ambient umask |
| 10 | + |
| 11 | +### What the reviewer asked for |
| 12 | + |
| 13 | +Fixed once as a series-wide pattern (#272 canon content, #275 request |
| 14 | +bodies, #280 system-prompt text are the same shape three times): explicit |
| 15 | +owner-only modes on every conversation-derived write, hashes instead of raw |
| 16 | +bytes where the bytes are not structurally required. |
| 17 | + |
| 18 | +### Scope criterion |
| 19 | + |
| 20 | +A write is conversation-derived when its payload is derived from live |
| 21 | +traffic: message bytes, request/response bodies, system-prompt text, **or |
| 22 | +the stable session identifiers that link a record back to a conversation**. |
| 23 | +The identifier clause is the reviewer's own — blocker 3 names |
| 24 | +`insertion-normalization.mjs:1076` ("telemetry logs stable session |
| 25 | +identifiers") as part of the finding, not just the canon content. Under that |
| 26 | +criterion every write site found qualifies, and all were converted. |
| 27 | + |
| 28 | +### Call sites — grep-established |
| 29 | + |
| 30 | +``` |
| 31 | +$ grep -rn "writeFile\|writeFileSync\|appendFile\|appendFileSync" proxy/extensions/ |
| 32 | +56 hits across 19 files |
| 33 | +``` |
| 34 | + |
| 35 | +27 of the 56 are real write sites, across 18 extensions; the rest are |
| 36 | +comments, imports, and test-seam declarations. All 27 converted: |
| 37 | + |
| 38 | +| extension | sites | what it writes | |
| 39 | +|---|---|---| |
| 40 | +| insertion-normalization | 2 | canon (`entry.m` raw bytes) + events | |
| 41 | +| prefix-diff | 2 | snapshot state + events | |
| 42 | +| deferred-tool-rewrite | 2 | state + events | |
| 43 | +| upstream-change-detection | 2 | baseline doc + events | |
| 44 | +| deferred-tools-restore | 1 | tool-definition state | |
| 45 | +| request-capture | 3 | full request bodies (the #275 case) | |
| 46 | +| rate-limit-log | 2 | jsonl | |
| 47 | +| usage-log | 2 | jsonl | |
| 48 | +| upstream-error-log | 2 | jsonl | |
| 49 | +| request-log | 1 | jsonl | |
| 50 | +| output-guard | 1 | guard events | |
| 51 | +| microcompact-stability | 1 | diagnostics | |
| 52 | +| overage-warning | 1 | jsonl | |
| 53 | +| bootstrap-defense | 1 | event log (sync) | |
| 54 | +| session-budget-breaker | 1 | event log (sync) | |
| 55 | +| image-retry-circuit-breaker | 1 | event log (sync) | |
| 56 | +| workflow-agent-id-synthesis | 1 | event log (sync) | |
| 57 | +| cache-telemetry | 1 | per-session telemetry (atomicWrite, 2 callers) | |
| 58 | + |
| 59 | +Post-change verification that nothing was missed: |
| 60 | + |
| 61 | +``` |
| 62 | +$ grep -rn "fs\.writeFile(\|fs\.appendFile(\|[^c]writeFileSync(\|[^c]appendFileSync(\|await appendFile(\|await writeFile(" \ |
| 63 | + proxy/extensions/ | grep -v "write-owner-only.mjs" | grep -v "append-queue.mjs" |
| 64 | +(no output) |
| 65 | +``` |
| 66 | + |
| 67 | +### The helper |
| 68 | + |
| 69 | +`proxy/extensions/write-owner-only.mjs`. No shared extension write utility |
| 70 | +existed — `append-queue.mjs` is a write primitive but had exactly one |
| 71 | +consumer, and `../claude-home.mjs` is a path resolver outside the extensions |
| 72 | +directory — so the brief's assigned new home was used. |
| 73 | + |
| 74 | +Two mechanisms, because neither covers the other's case: |
| 75 | + |
| 76 | +1. **`mode` at create.** Node applies the `mode` option only when the write |
| 77 | + actually creates the file. Passing it means a new file is never, not even |
| 78 | + briefly, group-readable. |
| 79 | +2. **A lazy chmod, once per path per process.** This is what fixes files |
| 80 | + written before the helper existed, and the rare umask that masks bits out |
| 81 | + of the create mode (chmod ignores umask; the `mode` option does not). |
| 82 | + Deliberately not a startup sweep, per the brief — a sweep would have to |
| 83 | + guess the file set and would touch state nobody writes again; binding the |
| 84 | + repair to the next write makes the repaired set exactly the live one. |
| 85 | + |
| 86 | +**Atomic writers (tmp + rename) need only mechanism 1** and carry no chmod |
| 87 | +call: the tmp file is always freshly created, so it is born 0600, and the |
| 88 | +rename carries that mode onto the final path — repairing a loose mode on an |
| 89 | +existing final file for free. Log rotation (`rename(path, path + ".1")`) |
| 90 | +preserves mode the same way. |
| 91 | + |
| 92 | +### Raw bytes vs hashes |
| 93 | + |
| 94 | +- **canon `entry.m` stays**, documented at the write site. Replaying those |
| 95 | + bytes *is* the pinning mechanism; a hash cannot stand in for them. That is |
| 96 | + exactly why the file must be owner-only. |
| 97 | +- **request-capture bodies stay** for the same structural reason — the |
| 98 | + corpus exists to be replayed. |
| 99 | +- **One candidate found and NOT converted** — see gap 1 below. |
| 100 | + |
| 101 | +### Verifier |
| 102 | + |
| 103 | +`test/write-owner-only.test.mjs`, four bites driving the real extension |
| 104 | +end-to-end against a real temp config root. A test double would report |
| 105 | +whatever the double chose; the wrongness lives in the filesystem, so the |
| 106 | +check runs at that altitude. The definition of "owner-only" is written above |
| 107 | +the assertions, before them, so the expected value comes from the invariant |
| 108 | +and not from the code meant to satisfy it — and it asserts **exactly** 0600 |
| 109 | +rather than "0600 or tighter", because a range assertion would pass on the |
| 110 | +very 0664 that motivated the check. |
| 111 | + |
| 112 | +- **Red-first**: all four failed against unmodified code, observing 420 |
| 113 | + (0644) and 436 (0664) against expected 384 (0600) — the reviewer's |
| 114 | + reproduced `-rw-rw-r--`. |
| 115 | +- **Green after**: 4/4. |
| 116 | +- **Mutation**: deleting only the lazy chmod from the append path turns |
| 117 | + exactly the repair bite red and leaves the other three green. The mutation |
| 118 | + removes the precise condition that bite names, and the two mechanisms are |
| 119 | + pinned separately rather than by one overlapping assertion. |
| 120 | + |
| 121 | +## Blocker 4 — the read-dedupe adjacency call |
| 122 | + |
| 123 | +**Verdict: the adjacency is not load-bearing. The assertion loosens; the |
| 124 | +order does not move. Already correct on fork-main — no code change needed.** |
| 125 | + |
| 126 | +The reviewer saw the PR branch merged onto upstream `origin/main`, where the |
| 127 | +test still pinned `cache-control-normalize` as read-dedupe's immediate |
| 128 | +successor. On this fork the assertion was already generalized at **60cb337** |
| 129 | +(the ttl-keepalive commit), independently of the insertion-normalization |
| 130 | +work, and `node --test test/proxy-read-dedupe.test.mjs` is 42/42 green at the |
| 131 | +base commit. |
| 132 | + |
| 133 | +The decision derives from what the adjacency is *for*, not from what makes |
| 134 | +the test pass: |
| 135 | + |
| 136 | +``` |
| 137 | +$ grep -rn -i "cache-control-normalize\|cache_control\|breakpoint" proxy/extensions/read-dedupe.mjs |
| 138 | +(no hits) |
| 139 | +``` |
| 140 | + |
| 141 | +read-dedupe has no reference to cache-control-normalize, to `cache_control`, |
| 142 | +or to breakpoints anywhere in its source. It rewrites duplicate `Read` |
| 143 | +tool_result bodies and reads nothing a later breakpoint pass writes. The |
| 144 | +adjacency was an incidental fact about the registry on the day the test was |
| 145 | +written, never a contract. What *is* load-bearing — read-dedupe's own order |
| 146 | +value (380) and that it is bracketed rather than at an end — is still |
| 147 | +asserted. |
| 148 | + |
| 149 | +Because no order moves, this stays clear of threat-matrix row 3. |
| 150 | + |
| 151 | +The code change is therefore only the recording of the determination in the |
| 152 | +test comment: the existing comment explained order-tolerance but never stated |
| 153 | +that the adjacency had been checked and found non-load-bearing, which is the |
| 154 | +deliberate call the reviewer actually asked for. |
| 155 | + |
| 156 | +## Gaps — surfaced, not settled |
| 157 | + |
| 158 | +1. **prefix-diff persists truncated raw message text.** `buildSnapshot()` |
| 159 | + (`proxy/extensions/prefix-diff.mjs:545`) stores `prefixMessages`, |
| 160 | + `tailMessages`, and `markerMessages` — truncated raw conversation |
| 161 | + content, alongside the hashes. It is the one payload found where a hash |
| 162 | + could plausibly replace bytes. It was **not** converted: the extension |
| 163 | + exists to diagnose *what* changed between requests, and a hash answers |
| 164 | + only *that* something changed, so the conversion trades diagnostic |
| 165 | + fidelity for reduced persisted sensitivity. That is a design decision |
| 166 | + above this tier, not a mechanical fix. 0600 covers it in the meantime. |
| 167 | + Not audited: whether the other payloads have byte-reducible fields — the |
| 168 | + pass was write-site-complete, not field-complete. |
| 169 | + |
| 170 | +2. **`docs/directives/proxy-read-dedupe.md` does not exist.** |
| 171 | + `proxy/extensions/read-dedupe.mjs:3` and |
| 172 | + `docs/extension-impact-guide.md:276` both cite it as read-dedupe's |
| 173 | + directive. It is absent from `docs/directives/`. The blocker-4 verdict |
| 174 | + did not need it (the artifact answered the question), but a doc-vs-artifact |
| 175 | + gap on a shipped extension's stated rationale is worth a decision: |
| 176 | + write it, or drop the two references. |
| 177 | + |
| 178 | +3. **The fold-in re-links sanitized fixture names to their capture keys.** |
| 179 | + As instructed, the corrected comments keep the old capture names beside |
| 180 | + the new ones ("captured as s-0d6f38ba" / "captured as s-dc3f8071"). Both |
| 181 | + prefixes were already in this file before the change, so exposure is |
| 182 | + unchanged — but the pairing now provides a *mapping* from the sha-derived |
| 183 | + fixture token back to the real capture prefix, which is a partial undo of |
| 184 | + what the fixture sanitization set out to break. This is a public repo. |
| 185 | + Flagging for a deliberate keep-or-drop rather than acting on it. |
| 186 | + |
| 187 | +## Deployment |
| 188 | + |
| 189 | +Touches `proxy/**` → dotfiles pin bump (`git rev-parse --short HEAD:proxy`) |
| 190 | ++ `systemctl --user restart cache-fix-proxy` ride the next boundary, |
| 191 | +dispatcher-owned. |
| 192 | + |
| 193 | +**Does not touch state KEYS or freeze logic.** 0600 is file metadata; no |
| 194 | +key derivation, no canon shape, no freeze path changed. The restart is |
| 195 | +cache-transparent (threat matrix row 3). No pipeline order changed either, |
| 196 | +since blocker 4 resolved without moving read-dedupe. |
| 197 | + |
| 198 | +After the restart the gate should run once |
| 199 | +(`systemctl --user start cache-fix-gate`) — extension write behavior changed |
| 200 | +even though extension *decision* behavior did not. |
| 201 | + |
| 202 | +## Checks run |
| 203 | + |
| 204 | +| check | result | |
| 205 | +|---|---| |
| 206 | +| `git merge-base --is-ancestor 94cbf82 HEAD` | BASE-OK, clean tree | |
| 207 | +| `node --test test/write-owner-only.test.mjs` (red-first, pre-change) | 0 pass / 4 fail — 420 and 436 vs expected 384 | |
| 208 | +| `node --test test/write-owner-only.test.mjs` (post-change) | 4 pass / 0 fail | |
| 209 | +| mutation: lazy chmod removed | 3 pass / 1 fail — exactly the repair bite | |
| 210 | +| `node --test` over 5 touched test files | 181 pass / 0 fail | |
| 211 | +| full suite minus proxy-integration/proxy-wrapper (117 files) | **1843 pass / 0 fail / 0 skipped** (baseline 1839 + 4 new bites) | |
| 212 | + |
| 213 | +## Not verified |
| 214 | + |
| 215 | +- **No live traffic.** Every mode observation comes from temp config roots |
| 216 | + under `node --test`, never from the serving proxy's real `~/.claude`. The |
| 217 | + lazy repair of *actually existing* production files is proven by |
| 218 | + construction and by the repair bite, not by observation on the real |
| 219 | + state directory. |
| 220 | +- **Port-bound suites** (`proxy-integration`, `proxy-wrapper`) not run, per |
| 221 | + the brief. |
| 222 | +- **umask sensitivity**: all runs were at `umask 022`. The 0664 case is |
| 223 | + covered by the repair bite's explicit `chmod 0o664` precondition rather |
| 224 | + than by running the suite under a second umask. |
| 225 | +- **`entry.m` byte-vs-hash** was accepted as structurally required from the |
| 226 | + brief and the code's own rationale; not independently re-derived. |
0 commit comments