Repository navigation
Independent audit P1 fixes: F04, F05 and F06 (ADRs 0200-0201) - #442
Merged
Merged
Conversation
…(independent audit 2026-11-04, F04) What changed: fetchproof/lib/quotes.mjs - anchorFound carries the placements of a quote's fragments as a set of where they end, fragment by fragment, instead of trying each chain of occurrences by recursion: an occurrence of the next fragment extends a placement when it starts at or after the nearest end before it and no more than maxGap after that end, and each distinct fragment text's occurrences are found once. The verdict is the recursion's for every input: the first fragment may start anywhere, each later one within MAX_QUOTE_GAP (300 compared characters) of the one before, and every occurrence of every fragment is a candidate (ADR-0162), overlapping ones included. Its JSDoc says why. Two new tests. Why: the independent audit of 532c696 (Codex, F04, P1) showed the recursion trying every chain of occurrences before it could fail: a quote of n copies of `alpha beta gamma` ending in a fragment the page lacks, over a page repeating the phrase 2n times, took time exponential in n - twenty copies over 680 characters did not finish in 3 s - and the citations check, which preflight and the commit gate run, waited on it. Set aside: memoizing the recursion on (fragment, start), the audit's other remedy - the same bound, with a memo of up to fragments x occurrences entries and a call depth of the fragment count, for no difference in what it decides; matching each fragment at its first reachable occurrence, linear and wrong - ADR-0162 rejected it because a word earlier on the page hides the passage meant, and the new kept-placement test pins the same for a middle fragment; a time or step budget, which turns a verdict into a give-up; a cap on fragments per quote, a new rule under the freeze (ADR-0117). One remedy keeps every verdict, so no ADR (ADR-0075). What it touched: fetchproof/lib/quotes.mjs (anchorFound and its JSDoc); fetchproof/test/quotes.test.mjs (+2 tests: twenty and four hundred repeated fragments, with the page lacking the last one and holding them all, run in a child process with a 10 s timeout so a regression fails instead of holding the suite; a middle fragment whose first occurrence leaves the next out of reach, found through a later occurrence and not found when that one is past the bound; the node:child_process import); docs/ARCHITECTURE.md (row quotes.mjs); CHANGELOG.md (Unreleased); fetchproof/README.md (2277 -> 2279 tests). No new check, flag, rule or format. What you verified: verified - red first: with the two tests and without the library change, `selftest quotes` failed exactly the bounded-time test (41 passed, 1 failed, ETIMEDOUT at 10 s); the kept-placement test passed on the old code too, as a pin of kept behaviour should; verified - green: `selftest quotes` 42 passed, 0 failed; verified - the same verdict as the recursion it replaces: a scratch differential run (not committed) over 200,000 random bodies, fragment lists and gaps of 0 to 11 characters, drawn from an alphabet with overlapping occurrences, case and emphasis, agreed on all 200,000 (70,100 found); verified - the audit's own repro, `gate-review/repro-quotes.mjs --worker 20` pointed at this tree (the worker mode writes nothing): found false in 3.7 ms; verified - `selftest quote checks measure preflight gate brief` (every group that calls anchorFound, through citations, the measure or the quote recheck) 362 passed, 0 failed; expected - the audit's runner passes its F04 case (run after the P1 fixes); expected - the full suite, run by this commit's hook (ADR-0120): the commit exists only if it is green. What you got wrong: mistake - the first scripted edit of quotes.mjs anchored on a JSDoc line I quoted from memory (" * is skipped, and an anchor ...") while the file reads " * nothing is skipped, ..."; where - the replacement step for anchorFound; impact - none: the script checks its anchor before writing and threw, leaving the file untouched, and nothing delivered is affected; cause - wrote the anchor from memory instead of copying it from the read; fix - re-anchored on the JSDoc's last line, copied from the file, and checked that it occurs once; verified - `selftest quotes` 42 passed on the edited file, and the diff of quotes.mjs shows only anchorFound and its JSDoc changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… or whose bytes miss their CRC-32, is refused (independent audit 2026-11-04, F05) What changed: fetchproof/lib/artifact-zip.mjs - a new inspectLocalHeaders(buf, entries), run by openZip beside inspectEntries before anything is inflated, reads every entry's local header, directories included. A local header missing where the directory says it is, or running past the end of the file, and a body that at the directory's compressed size would run past it (named in readEntry's words, before anything is compared), are ZIP-READ problems; otherwise the local header must agree with the central directory on what an extractor takes from it - the name as stored bytes, the compression method, the encryption bit, the UTF-8 name bit and, unless the local header's bit 3 defers them to a data descriptor, the CRC-32 and both sizes - and one ZIP-READ problem per entry names every field that disagrees, with both values. readEntry checks each body's CRC-32 (zlib.crc32) after its length and refuses a mismatch with ZIP-READ. readCentralDirectory keeps each entry's name bytes. fetchproof/lib/artifact-validator.mjs - manifest.json and manifest.sha256 are read under a catch: a ZipError there is FAIL with its code, as it is for a payload file, where it had reached the outer catch as BLOCKED. ADR-0200. Five new tests; the tests' ZIP writer gains a data-descriptor option. Why: the independent audit of 532c696 (Codex, finding F05, Medium, confirmed; assertions F05a and F05b) built two variants of an approved package - a local header naming README-ALTER.md where the directory lists README-FIRST.md, and the two CRC-32 fields of README-FIRST.md zeroed - and both validated PASS with buildAuthorized true, while Python's zipfile.testzip() names README-FIRST.md as bad in each. The reader checked a local header's signature and a body's length and took everything else from the directory: it never compared the local fields and never computed a CRC-32. The manifest's SHA-256s still matched the bytes, so this is not an integrity bypass; it is an approved package an ordinary extractor refuses, or a streaming one reads as another package. Set aside (ADR-0200): comparing every local field (versions, timestamps, extra fields, every flag bit - writers legitimately differ there, and none changes what is extracted); comparing the local CRC-32 and sizes whatever bit 3 says (it refuses every streaming writer's archive); reading the data descriptor; no CRC-32 because the SHA-256 covers the payload (an extractor checks the CRC-32, and manifest.sha256 is covered by no hash); computing the CRC-32 in the structural pass (it inflates every entry twice, or holds them all); comparing decoded names (two invalid sequences decode alike); new error codes; the writer's crc32 imported into the reader. Not covered, and named here so it is not taken as fixed: the data descriptor itself is not read; the reader is stricter than Python's zipfile on the method, the two flag bits, the CRC-32 and the sizes, on purpose, because a streaming extractor reads them from the local header. What it touched: fetchproof/lib/artifact-zip.mjs (the header comment, readCentralDirectory's nameBytes, a new "local headers" section with inspectLocalHeaders, readEntry's CRC-32 check, openZip); fetchproof/lib/ artifact-validator.mjs (the reads of manifest.json and manifest.sha256); fetchproof/test/artifact-fixtures.mjs (rawZip's descriptor option); fetchproof/test/artifact-validator.test.mjs (+5 tests: the renamed local header, refused by the container pass and by the validator; each agreed field refused when it disagrees, and three fields that are not compared left alone; a wrong CRC-32 in a payload file and in manifest.json, both FAIL ZIP-READ, the manifest's no longer BLOCKED; stored and deflated entries behind data descriptors read, with their bytes still checked against the directory's CRC-32; Python's zipfile as an independent reader of the same fixtures, UNSUP without Python); docs/adr/0200 and its index row; docs/ARCHITECTURE.md (rows artifact-zip.mjs and artifact-validator.mjs, ADR-0200 in both ADR cells); CHANGELOG.md (Unreleased); fetchproof/README.md (2279 -> 2284 tests). No new error code, flag, check or file format. What you verified: verified - red first: with the five tests and the writer's new option, and the two library files put back at HEAD, `selftest artifact-validator` failed exactly those five (65 passed, 5 failed): the container pass did not see the renamed header, a local method of 0 gave no problem, a zeroed CRC-32 validated PASS, a directory CRC-32 zeroed behind a data descriptor read without error, and the reader matched Python's zipfile on neither refused fixture - Python (3.14.7) named README-FIRST.md in both and nothing in the approved package or the descriptor archive; verified - green: `selftest artifact` (artifact-cli, artifact-producer, artifact-validator) 137 passed, 0 failed; verified - the neighbours: the first run, `selftest dispatch fi- hardening mcp audit bundle release collect`, was red (452 passed, 1 failed, cwd E:\Claude\fp-p0-fixes; mistake 5 below); after its fix, `selftest artifact fi- dispatch hardening audit evidence-context` (every group that reads a ZIP through artifact-zip.mjs) 346 passed, 0 failed, and `selftest mcp collect` (the other two that build ZIPs) 207 passed, 0 failed; verified - the external facts ADR-0200 rests on: nodejs.org's zlib page gives zlib.crc32 "Added in: v22.2.0, v20.15.0", inside the kit's floor (runtime.mjs: "install Node 22.22.1 or newer on the 22 line, or Node 24"); Python 3.14.7's ZipFile.open (zipfile/__init__.py, lines 1743-1771) refuses a bad local signature and a local name that differs from the directory's, and takes the method, sizes, CRC-32 and encryption from the directory record; Info-ZIP's extrafld.txt (libzip.org copy) gives the extended-timestamp field (0x5455) modification, access and creation times in the local header and the modification time only in the central one - both pages fetched 2026-11-06 through Firecrawl, not captured in a corpus; expected - the Codex runner's F05a and F05b now pass (run once F06 is in, at the end of this task); expected - the full suite, run by this commit's hook (ADR-0120): the commit exists only if it is green. What you got wrong: five mistakes, each fixed before this commit. (1) mistake - ADR-0200's draft cited PKWARE's APPNOTE by section numbers (4.3.9, 4.4.4) I had not checked; where - ADR-0200, decision 2; impact - a reader might have looked up the wrong sections, nothing delivered affected; cause - wrote the numbers from memory; fix - it names "PKWARE's APPNOTE, general-purpose bit 3" with no section number; verified - grep finds neither number in ADR-0200. (2) mistake - the draft said writers differ between the two headers on the version and the deflate bits, with no source for it; where - ADR-0200, first rejected alternative; impact - an unsupported reason for a rejection, nothing delivered affected; cause - generalised from the audit's warning about overstrict comparison; fix - the reason is now Info-ZIP's extended-timestamp field, longer in the local header than in the directory; verified - Info-ZIP's extrafld.txt, as above. (3) mistake - the draft said Python's zipfile compares only the name; where - ADR-0200, Consequences; impact - it understated the independent reader (it also refuses a bad local signature), nothing delivered affected; cause - read the name comparison and stopped; fix - "it checks a local header's signature and compares its name with the directory's, and nothing else"; verified - Python 3.14.7's ZipFile.open, as above. (4) mistake - my first insertion of the five tests went through a shell heredoc whose backslashes the tool rewrote; where - the step that added the tests to artifact-validator.test.mjs; impact - bash refused the script ("unexpected EOF") and nothing was written, nothing delivered affected; cause - typed backslashes into a tool call known to decode them; fix - inserted with the editor instead, with no backslash typed; verified - the five tests are in the file once each and ran, red then green, as above. (5) mistake - the first inspectLocalHeaders compared the local fields before checking that the body, at the directory's compressed size, lies inside the file, so a directory size pointing past the end was reported as the two headers disagreeing; where - fetchproof/lib/artifact-zip.mjs, inspectLocalHeaders; impact - fi-validator-adapter's "#318 a truncated workbook (an entry running past the end of the file) is refused, not read as short sheets" went red: the workbook was still refused (WB-PACKAGE-UNTRUSTED), but for "the compressed size (71 there, 398 in the directory)", a symptom, instead of its body running past the end of the file; nothing delivered affected, the change was uncommitted; cause - I did not ask what the new pass reports for an archive whose directory is itself out of bounds, and the artifact group I ran for green has no such case; fix - that body is named as running past the end of the file before anything is compared, and the local header's own bound now includes its extra field (ADR-0200, decision 1; the map row); verified - the #318 test unchanged and green in the re-run above, with the rest of its groups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l in flight (independent audit 2026-11-04, F06) What changed: fetchproof/lib/dispatch.mjs - waitForRun holds one AbortController per watch, aborted by one timer at timeoutMs (re-armed past the 2^31-1 ms a setTimeout holds) and cleared in a finally however the watch ends. Its signal goes through getRun's new optional `signal` to fetch, and a new unlessAborted races the fetch, the JSON body and a 403's text against it, so whatever is still unanswered at the deadline is cut off, by a fetch that honours the signal or not. callApi and readJsonBody rethrow an abort as it is, so a cut-off poll is never NETWORK or BAD_BODY, which the watch would retry. A poll cut off ends the watch with TIMEOUT naming the run, the status it had at the poll before, its page and --run-id - exit 4 in collect-remote, as every TIMEOUT already is. A wait is the smaller of the interval and the time left, and no poll starts at or after the deadline; a failure being retried when the deadline comes is thrown as itself, as before; a completed answer in hand is returned. ADR-0201. Four new tests, and one test now expects no poll after the deadline. Why: the independent audit of 532c696 (Codex, finding F06, Medium, confirmed) held a status poll's fetch open under a 25 ms deadline: the watch was still pending after 150 ms, and when the fixture then released a completed run, the watch returned it. waitForRun checked its deadline only once a poll had answered; callApi awaited fetch with no signal and readJsonBody awaited the body the same way, so a proxy that takes the connection and never answers, or a body that stalls, held collect-remote past its --timeout for as long as the request hung. The wait between polls was also a full interval whatever was left, and the poll after the wait that crossed the deadline still ran. Set aside (ADR-0201): a fixed timeout per request (it bounds a poll, not the watch, and is one more number below the operator's); AbortSignal.timeout (past 2^31-1 ms it fires at once, and it cannot be cleared); the signal to fetch only, with no race (an injected fetch, or a dispatcher that ignores the signal, hangs as before); refusing a completed answer read after the deadline; one last poll after the deadline (the old behaviour); TIMEOUT for a failure being retried at the deadline (it would move collect-remote's exit for a case the audit did not raise); deadlines for the other requests (outside the confirmed finding, a dispatch that may have been accepted must not be retried blindly, and a new limit under the freeze); a clock seam for the timer. Not covered, and named here so it is not taken as fixed: the dispatch POST and the artifact list and download have no deadline; the MCP server's getRunSummary passes no signal and is unchanged; a deadline under a second is still reported rounded ("after 0s"). What it touched: fetchproof/lib/dispatch.mjs (unlessAborted; readError, rateLimitError, readJsonBody and callApi pass the signal on; getRun's signal; MAX_TIMER_MS; waitForRun and its doc comment); fetchproof/test/ dispatch.test.mjs ("retrying a failing poll does not outlast the deadline" now expects one poll, not two; +4 tests: a poll still unanswered at the deadline cut off three ways - no response headers, a JSON body that never finishes, a 403's text that never finishes - each ending in TIMEOUT that names run 7, its last status, its page and --run-id, with the request's signal aborted and no third poll; the wait capped at the deadline and no poll after it, on the fake clock; an answer late inside the deadline taken; a watch of 2^31 ms not cut off at its start); docs/adr/0201 and its index row; docs/ARCHITECTURE.md (the dispatch.mjs row, ADR-0201 in its ADR cell); CHANGELOG.md (Unreleased); fetchproof/README.md (2284 -> 2288 tests). No new flag, error code, exit or format. What you verified: verified - red first: with the new dispatch.test.mjs and dispatch.mjs at HEAD, `selftest dispatch` 44 passed, 3 failed - exactly the changed retry test (two polls where one is now expected: "a poll started after the deadline had passed"), the cut-off test ("no response headers: the watch was still pending 2 s after its 50 ms deadline") and the wait test (waits of 10000, 10000, 10000 ms where the last should end at the deadline, 5000); the other two new tests passed on the old code, as they should: the answer taken late in the deadline is the cut-off test's control, and the 2^31 ms watch is the guard against a timer that overflows; verified - green: with dispatch.mjs changed, `selftest dispatch` 47 passed, 0 failed, and no TimeoutOverflowWarning in its output; verified - the neighbours, `selftest dispatch cli collect hardening mcp panel proxy` (every group whose tests name waitForRun or collect-remote) 518 passed, 0 failed, cwd E:/Claude/fp-p0-fixes; verified - the Node facts ADR-0201 rests on, on Node 24.20.0 with a scratch script (not in the repository): setTimeout given 2^31 ms fired after 3 ms with a TimeoutOverflowWarning ("Timeout duration was set to 1"), and AbortSignal.timeout(2 ** 31) aborted after 11 ms; against a local HTTP server, a fetch aborted 100 ms in rejected at 113 ms when no headers had come, and its body read at 112 ms when the body had stalled half-way, both with the signal's own AbortError; expected - the Codex runner's F06 now passes (run at the end of this task); expected - the full suite, run by this commit's hook (ADR-0120): the commit exists only if it is green. What you got wrong: two mistakes, each fixed before this commit. (1) mistake - ADR-0201's draft said collect-remote ends within its --timeout whatever the network does; where - ADR-0201, Consequences, first item; impact - it overstated the fix, since the dispatch POST before the watch and the artifact list and download after it are still unbounded, as the same draft's Not covered says; nothing delivered affected; cause - wrote the command where I meant its watch; fix - "collect-remote's watch ends within its --timeout ...; the dispatch before it and the download after it are not bounded"; verified - read against fetchproof/bin/collect-remote.mjs: dispatchCollection at line 225, waitForRun at 253, fetchCorpus at 285. (2) mistake - a line-ending check with grep and a CR typed into the pattern counted every line of each file, which read as if the docs were CRLF; where - the step that applied the docs snippets; impact - none: the edits were made by a script that refuses an anchor matching other than once, and nothing was changed on that reading; cause - the CR typed into the tool call did not reach grep as a CR; fix - counted with node and String.fromCharCode(13) instead; verified - 0 CRs in CHANGELOG.md, docs/ARCHITECTURE.md, docs/adr/README.md, fetchproof/README.md, dispatch.mjs, the ADR and the snippets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d a ZIP64 local header is refused as ZIP64 (review of #442) What changed: fetchproof/lib/artifact-zip.mjs - readEntry's CRC-32 now goes through a new crc32Of(body): zlib.crc32 where this Node has it, looked up on each call, else the same sum from a 256-entry table the reader now carries (CRC_TABLE). inspectLocalHeaders: a local header whose bit 3 is clear and whose compressed or uncompressed size is 0xFFFFFFFF is one ZIP-READ problem, 'entry "<name>" uses ZIP64 size fields in its local header, which this reader does not implement' - the words the directory's ZIP64 sizes are already refused in - and is not compared further; the flags are read a few lines earlier for it (same value). Two new tests; the tests' ZIP writer (rawZip) gains a zip64Local option. ADR-0200 corrected. Why: a fresh-eyes review of #442 (2026-11-06, findings 1 and 3). Finding 1: Node added zlib.crc32 in 22.2.0 (and 20.15.0), and the kit's runtime checks admit any Node 22 - checkNode compares only the major, and the doctor's nodeLine fails only 22.13.0 to 22.22.0 (the windows-1252 regression, ADR-0159) - so on 22.0 and 22.1 every ZIP read threw "zlib.crc32 is not a function": `artifact.mjs validate` gave BLOCKED on a good package, collect-remote could not unwrap GitHub's wrapper and reported MANIFEST-MISSING after the paid run, and fi-validate refused every workbook (WB-PACKAGE-UNTRUSTED). Finding 3: a local header that gives its sizes as ZIP64 - Python's zipfile with force_zip64 writes 0xFFFFFFFF there, the sizes in a ZIP64 extra field, and 32-bit sizes in the directory - was refused as "its local header disagrees with the central directory on the compressed size (4294967295 there, ...)", which is untrue: the two headers agree. The refusal stands (the module refuses ZIP64 rather than half-read it); its reason is now true. Set aside (ADR-0200): zlib.crc32 alone, with checkNode and the doctor raised to 22.2 - which Node the kit refuses is a support decision of its own, and the documented floor (22.22.1+ or 24+) is enforced by neither check, a follow-up; importing the writer's crc32 (the module header keeps the writer and the reader apart); reading the local ZIP64 extra field and comparing its sizes with the directory's (it half-reads ZIP64 while the directory's ZIP64 is refused outright). What it touched: fetchproof/lib/artifact-zip.mjs (inspectLocalHeaders and its doc comment; a new CRC_TABLE and crc32Of before readEntry; readEntry's CRC-32 line); fetchproof/test/artifact-fixtures.mjs (rawZip's zip64Local option: version 45, 0xFFFFFFFF local sizes, a 0x0001 extra field of 16 bytes, offsets counting it); fetchproof/test/artifact-validator.test.mjs (+2 tests: with zlib.crc32 taken away and put back in one synchronous step, a stored and a deflated body read and a wrong CRC-32 is still refused, naming the sum zlib gives; a ZIP64 local header is refused with the ZIP64 reason and nothing else, and with bit 3 set it still reads); docs/adr/0200 (decision 1 gains the ZIP64 sentence, decision 3 is corrected, two rejected alternatives are added and one reworded, a consequence is added and the cost line corrected) and its index row; docs/ARCHITECTURE.md (row artifact-zip.mjs); CHANGELOG.md (the F05 entry under Unreleased, which this completes: Five tests -> Seven); fetchproof/README.md (2288 -> 2290 tests). No new error code, flag, check or file format. What you verified: verified - red first: with the two tests and rawZip's option, and the library at HEAD, `selftest artifact-validator` failed exactly those two (70 passed, 2 failed): "zlib.crc32 is not a function", and the ZIP64 header reported as disagreeing on both sizes (4294967295 there, 47 and 60 in the directory); verified - green: `selftest artifact fi- dispatch hardening audit evidence-context` (every group that reads a ZIP through artifact-zip.mjs) 352 passed, 0 failed; verified - a scratch differential: with zlib.crc32 taken away, the table sum accepted 3,000 of 3,000 random bodies (0 to 70,000 bytes, stored and deflated) whose directory CRC-32 the writer's crc32 computed; verified - the review's file written by Python's zipfile with force_zip64 (py-zip64-local.zip) now gets the ZIP64 reason; verified - the floor facts, read in runtime.mjs: checkNode tests major >= 22 (lines 27-33), nodeLine fails a 22 only where nodeDecodesWindows1252 is false, 22.13.0 to 22.22.0 (lines 49-55, 405-416); zlib.crc32 is "Added in: v22.2.0, v20.15.0" on nodejs.org's zlib page (fetched for F05, 2026-11-06); untested - a real Node 22.0 or 22.1 binary: simulated by deleting zlib.crc32, as the review did; expected - the full suite, run by this commit's hook (ADR-0120): the commit exists only if it is green. What you got wrong and fixed: 1. mistake: ADR-0200 and the F05 commit's report said zlib.crc32 lies "inside the kit's 22.22.1 floor"; where: ADR-0200 decision 3, and 723f4a4's report, which cited runtime.mjs's "install Node 22.22.1 or newer on the 22 line"; impact: on Node 22.0 and 22.1, which the kit's checks admit, every ZIP read threw (validate BLOCKED, collect-remote MANIFEST-MISSING, fi-validate WB-PACKAGE-UNTRUSTED); nothing delivered is affected, #442 is not merged; cause: I took the doctor's remedy text for a floor the code enforces and read neither check; fix: the table fallback, and decision 3 rewritten; verified: the first new test, red then green (above). 2. mistake: inspectLocalHeaders compared the ZIP64 sentinel as if it were a size; where: artifact-zip.mjs, inspectLocalHeaders (723f4a4); impact: such an entry was refused with a false reason (it was refused either way); not merged; cause: readCentralDirectory handles the sentinel, and I did not carry that over to the local fields; fix: the sentinel is named as ZIP64 before anything is compared; verified: the second new test, red then green (above). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y, and a wrapper whose package does not read says why (review of #442) What changed: fetchproof/lib/dispatch.mjs - waitForRun: when the deadline cuts off a poll whose status had come (a DispatchError with a status: a 403, whose text is read for a rate limit), the TIMEOUT now says "the status poll for run N answered HTTP 403, but its body was still being read when the watch ended after Ns", and the refusal's own remedy comes before the watch's (timedOut gains a `first` argument): the token's "Actions" permission, or the rate limit's wait. A poll with no answer keeps "had no answer". Still TIMEOUT, exit 4. unwrapArtifact: a wrapper whose one package does not read is still handed on as it came, now with the reason, `unreadable: { entry, code, message }` (the ZipError's code, else ZIP-READ). fetchCorpus: the wrapper is still written and validated; when it came with `unreadable`, the validation's MANIFEST-MISSING is replaced by a ZIP-READ problem, "the package inside GitHub's wrapper does not read: <the reader's reason>", path the entry, remedy "download it again with --run-id N; if it fails the same way, the stored artifact is damaged - re-collect". The status stays FAIL and buildAuthorized false. One new test, two extended. ADR-0200 and ADR-0201 corrected. Why: a fresh-eyes review of #442 (2026-11-06, findings 4, 5 and 6). Finding 4: with every read now checking the CRC-32, a wrapper whose package fails it - the review flipped one bit in both headers, so the wrapper opens and Python's zipfile names the entry - was reported "manifest.json is not in the package ... regenerate it with `artifact.mjs create`", after a paid run, where downloading again is the cheap first step; ADR-0200 said the validator "then names the fault", which it did not. The class was older (an inner package that does not inflate, 2026-10-01); this change had widened it. Finding 5: a 403 whose body stalled to the deadline was reported "had no answer", with "a longer --timeout" as the remedy, which sends a token without the permission back to the same refusal; the status line had arrived. Finding 6: the cut-off test gave the watch 50 ms; a stall of 60 ms in its first poll ended the watch before the poll under test, and the test then failed on a TypeError (calls[1] undefined), not an assertion. Set aside (ADRs 0200, 0201): the cut-off 403 reported as the refusal itself (HTTP or RATE_LIMITED) - which one can rest on the body that was not read, and the watch did end at its deadline; the wrapper's fault thrown out of fetchCorpus under a code of its own - a ZIP-READ thrown from there went past the validator once, and a new code is a word every caller must learn; only ADR-0200's sentence corrected (the review's lesser fix) - the operator would still be told to regenerate a package. What it touched: fetchproof/lib/dispatch.mjs (imports ZipError; timedOut(message, first); waitForRun's abort catch; unwrapArtifact's catch and its comment; fetchCorpus's return); fetchproof/test/dispatch.test.mjs (the cut-off test: each variant names the message it expects, the two 403 variants their remedy, a fourth variant is a rate-limited 403, the deadline is 500 ms inside a 5 s guard, and the call count is asserted before calls[1] is read; the inner-package test of 2026-10-01 now expects the reason, ZIP-READ alone; +1 test: a wrapper whose package fails its CRC-32, its two headers agreeing, is reported as ZIP-READ naming the CRC-32 with the --run-id remedy, and kept on disk as it came); docs/adr/0200 (the wrapper consequence rewritten; two rejected alternatives added) and docs/adr/0201 (decision 3 gains the cut-off 403; the clock-seam alternative notes 500 ms; one rejected alternative added) and their index rows; docs/ARCHITECTURE.md (row dispatch.mjs, whose ADR cell gains ADR-0201 and ADR-0200); CHANGELOG.md (the F06 and F05 entries under Unreleased, which this completes: F05 Seven tests -> Eight); fetchproof/README.md (2290 -> 2291 tests). No new error code, flag, exit or file format. What you verified: verified - red first: with the new tests and dispatch.mjs at HEAD (put back unstaged, then restored and compared), `selftest dispatch` failed exactly three (45 passed, 3 failed): the cut-off test at its first 403 variant, the inner-package test at "and with the reason they did not read", and the new CRC-32 test (no `unreadable`); verified - green: `selftest dispatch collect mcp artifact` 394 passed, 0 failed; verified - the review's own repros against this tree: its Python-made wrapped-badcrc.zip, fed to fetchCorpus by a stub fetch, gives FAIL, buildAuthorized false, one ZIP-READ "... does not match its CRC-32: the directory declares 0xce4fae9e, its bytes give 0xce4fae9f" with the --run-id 9 remedy; its f06.mjs case C (a 403 with a hung body) now reads "answered HTTP 403, but its body was still being read", cases B, D and E still "had no answer"; verified - collect-remote's --timeout is an integer of at least 1 (collect-remote.mjs:158), so the "after 0s" of a sub-second deadline is reachable from the library only; untested - a real GitHub 403 whose body stalls: stub fetch only, as in the review; expected - the full suite, run by this commit's hook (ADR-0120): the commit exists only if it is green. What you got wrong and fixed: 1. mistake: ADR-0200 said a wrapper whose package does not read is judged by the validator, which "then names the fault"; where: ADR-0200's consequences (723f4a4); impact: the record claimed a report the code did not give, and the F05 change added CRC-32 failures to the wrappers reported as a missing manifest, with a remedy to regenerate; not merged; cause: I wrote the consequence from the comment at unwrapArtifact, not from what fetchCorpus returns, and did not run a wrapper through it; fix: the reason is carried out and reported, and the consequence rewritten; verified: the new CRC-32 test, red then green (above). 2. mistake: a 403 cut off while its body was read was reported "had no answer", with only a longer --timeout as the remedy - and the test asserted it; where: dispatch.mjs waitForRun's abort catch, dispatch.test.mjs's cut-off test (c8f0e1b); impact: an operator whose token lacks "Actions" would retry with a longer timeout into the same refusal; not merged; cause: I classified by the abort alone and did not look at what the request had already answered; fix: the status and the refusal's remedy are kept; verified: the cut-off test's 403 variants, red then green (above). 3. mistake: the cut-off test's 50 ms deadline; where: dispatch.test.mjs (c8f0e1b); impact: on a loaded machine the commit gate could fail on a TypeError unrelated to the code; not merged; cause: I sized the window for an idle machine and read calls[1] before asserting the count; fix: 500 ms inside a 5 s guard, the count asserted first; verified: green above, and the review's margin repro (its f06-test-margin.mjs, copied with the deadline set to 500 ms) passes with the first poll held 0, 60, 300 and 450 ms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e set pass reads only what a placement can reach (review of #442) What changed: fetchproof/lib/quotes.mjs `anchorFound` - the leftmost chain is tried first: each fragment at its first occurrence at or after the end of the one before it, one `indexOf` per fragment; within the bound it is a placement (found), and since it goes on past a gap over the bound, a fragment that does not occur after it occurs after no placement (not found). The gap is tested as `!(at - end <= maxGap)`, as the set pass tests it, so a bound that compares as NaN places no quote of two fragments or more in either; a one-fragment quote has no gap to bound. Only a complete chain past the bound runs the set pass, which now reads each fragment's occurrences from the first end to the last end plus the bound (a binary search finds the first), lists them when it first needs them, once per distinct text, as `Int32Array` positions, and carries `ends` the same way. The doc names the residual - a crafted quote whose leftmost chain goes past the bound costs O(fragments x occurrences) - and the cap not taken. One test. ADR-0202. Why: the fresh-eyes review of #442 (2026-11-06), finding 2. F04's set pass (d8d78e7) listed every occurrence of every distinct fragment before any was needed and read all of them for every fragment, so it was slow where the recursion it replaced had been cheap: a found quote of 1000 fragments over 200 KB took 3637 ms (the recursion 40 ms), with a final fragment the page lacks 3140 ms, 2000 fragments over 4.77 MiB 56176 ms, and 101 distinct fragments 2599 ms and 201 MiB (36 ms, 57 MiB); the audit's bar was 3 s for twenty. `check`, the quote recheck and `measure.mjs` call it, and preflight and the commit gate run `check`. Set aside (ADR-0202): a cap on a quote's fragments (the review's alternative; a limit of its own, which the freeze admits only through an ADR, and the residual's trigger), the recursion memoized (its table grows to fragments x occurrences), the leftmost chain alone (misses ADR-0162's later occurrence), finding the occurrences again per fragment with no lists (this fix's first draft: the crafted case 6748 ms against d8d78e7's 4629 ms), the ends kept as runs (a second structure for a case the cap closes more simply). What it touched: fetchproof/lib/quotes.mjs (`anchorFound` and its doc); fetchproof/test/quotes.test.mjs (+1 test, in a child process: 1000 fragments over 200 KB, found and with a final fragment the page lacks, each under 2000 ms, and a chain that must slide - the final fragment 252 compared characters past the leftmost chain's, then 402 - found, then not); docs/adr/0202 and its index row; docs/ARCHITECTURE.md (row quotes.mjs: the `anchorFound` sentence, ADR-0202 in its ADR cell); CHANGELOG.md (the F04 entry under Unreleased, which this completes: the leftmost chain, the reach, the lists, the residual; "Two tests." -> "Three tests."); fetchproof/README.md (2291 -> 2292 tests). No new check, flag, error code, limit or format. What you verified: - verified: red first - with 9527d0e's quotes.mjs swapped in, `node fetchproof/bin/selftest.mjs quotes`: 42 passed, 1 failed, exactly the new test ("found in 3459 ms"); the file restored and compared identical (cmp). - verified: `node fetchproof/bin/selftest.mjs quotes quote-recheck checks measure` (every group whose code reaches `anchorFound`: checks.mjs, measure.mjs, the recheck through checks): 229 passed, 0 failed, cwd E:\Claude\fp-p0-fixes; `selftest.mjs repo-hygiene` (the ADR index): 12 passed. - verified: the review's differential fuzz (E:/Temp/review-pr442/scripts/f04-fuzz.mjs), the recursion against this, seeds 12345 and 777, 200,000 cases each: 0 verdicts differ; the bound's boundary and self-overlapping fragments agree; a bound that compares as NaN: the recursion true, this false, as d8d78e7, for two fragments or more (no caller passes one). - verified: the review's corpora differential (f04-corpus.mjs): 44 corpora, 227 rows, 557 anchors, all found by both, 0 differ (2233 ms against 2279 ms in total). - verified: timings on Node 24.20.0, quiet machine (table in ADR-0202): found 47 ms, missing 38 ms, 2000 fragments over 4.77 MiB 1523 ms, 101 distinct 38 ms and 57 MiB, crafted (1000 fragments `a`, then `omega` past a 400-character run, over 200 KB) 2079 ms against d8d78e7's 4250 ms; the recursion did not finish it in 30 s. - verified: the full suite in the commit hook (ADR-0120); this commit exists only because it passed. What you got wrong and fixed: 1. mistake: F04's set pass made the cheap cases slow - it listed every occurrence of every distinct fragment before any was needed, and read all of them for every fragment; where: quotes.mjs `anchorFound` (d8d78e7); impact: a found quote of 1000 fragments over 200 KB took 3.6 s in each of `check`, the recheck and measure, 2000 over 4.77 MiB took 56 s, and 101 distinct fragments held 201 MiB - on fix/audit-p1, not merged; cause: I measured the exponential case the audit named and not the found case the recursion had made cheap; fix: the leftmost chain first, the reach, the lists built when needed; verified: the new test red at 9527d0e's matcher and green here, and the timings above. 2. mistake: d8d78e7's doc said "Each fragment now costs one pass over its occurrences, which are found once per distinct text", which reads as a bound, while the pass is O(fragments x occurrences); where: quotes.mjs `anchorFound`'s doc (d8d78e7); impact: the record understated the worst case; cause: I described the step, not its total; fix: the doc names the residual, its order and the cap not taken; verified: the doc as committed. 3. mistake: this fix's first draft dropped the lists and found each fragment's occurrences again with `indexOf`, which made the crafted residual slower than d8d78e7's (6748 against 4629 ms); where: the draft of the set pass (never committed); impact: none delivered; cause: I weighed the lists for their memory alone; fix: the lists are built when the set pass first needs them, once per distinct text, as 32-bit positions; verified: the crafted case 2079 ms against 4250 ms in the same run. 4. mistake: ADR-0202 said "a `maxGap` that is not a number places nothing", which is untrue for a one-fragment quote (no gap: found) and for a numeric string or `null` (compared as its number); where: ADR-0202, the paragraph after the timings, and the comment in `anchorFound`; impact: none in code - no caller passes `maxGap` - but the record overstated the change; cause: I generalised from the fuzz's NaN case; fix: both say a bound that compares as NaN places no quote of two fragments or more; verified: a probe over ten bounds (undefined, 300, '300', '0', null, NaN, 'abc', true, Infinity, -1) - one fragment found under all ten; two fragments found under undefined, 300, '300' and Infinity only (the re-review of the follow-ups, 2026-11-06). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s, a watch's error names the way back to the run, and package problems print with their remedies, redacted, then escaped (re-review of #442) What changed: fetchproof/lib/dispatch.mjs - `readError` returns null for a body it could not read, not ''; `rateLimitError` becomes `forbiddenError(response, { lead, refusal, signal })`, the error for a 403 from the dispatch or a run read: RATE_LIMITED when a header or the body names a limit; the caller's refusal when the body was read and names none; and, with the headers silent and the body unread, the refusal's code with "<lead>: HTTP 403, a rate limit or a refusal - its body, which says which, could not be read" and the remedy "wait at least one minute before retrying; if it is refused again, <the refusal's remedy>". `dispatchCollection` and `getRun` call it for a 403, and the watch puts that remedy first in a cut-off poll's TIMEOUT, as before. In `waitForRun`, an error that ends the watch with a remedy of its own has "the run may be fine - pick it up again with --run-id <n> rather than dispatching another" after it (`wayBack`), unless the remedy names `--run-id` already or the error is a 404; one with no remedy is left to collect-remote's own way back. fetchproof/lib/render.mjs - `printable`, moved from bin/artifact.mjs, shows the C0 and C1 controls, DEL, U+0085, U+2028, U+2029 and the invisible characters that hide or reorder text as \xNN (\uNNNN above U+00FF); `renderProblems(errors, { clean })` prints the count, each code and message, and each remedy as a `fix:` line under it, each field passed through `clean` before it is escaped. bin/artifact.mjs prints through both, in `validate` and in `create`; bin/collect-remote.mjs prints a package's problems through `renderProblems` with `redact` as `clean`, and the saved file's name redacted, then escaped, so a remedy - "download it again with --run-id" for a wrapper whose package does not read - reaches its default output. fetchproof/test/artifact-validator.test.mjs - the test for a Node without `zlib.crc32` takes its expected sum from the kit's writer (`crc32`, lib/archive.mjs, pinned to the published vectors), not from `zlib.crc32`. Seven tests. ADR-0201 amended. Why: two re-reviews of #442's follow-ups (2026-11-06). The first (E:/Temp/review-followups/p1-findings.md) made three low findings. 1: a 403 cut off at the watch's deadline was blamed on the token's "Actions" permission - `readError` swallowed the abort and returned '', which `rateLimitError` read as a body naming no limit - though GitHub names a secondary rate limit only in the body, and the same token had read the run one poll before. The re-review's other fix, a poll that read the run taken as proof of a limit, is rejected in ADR-0201: it states one cause as fact the other way round, and leaves the dispatch's 403, and a first poll's, naming the permission alone. 2: collect-remote printed a package's problems without their remedies, so the "download it again" remedy 9527d0e added, which the CHANGELOG and ADR-0200 promise, reached only --json and the MCP server. Narrowing those sentences (the re-review's lesser fix) would leave the default output without the cheap first step; printing through one renderer also escapes collect-remote's messages, which it printed as they came. 3: the test for a Node without `zlib.crc32` computed its expected sum with `zlib.crc32`, so on 22.0 and 22.1, the Nodes it is for, it threw. The second, of this commit's draft (E:/Temp/review-followups/p1b-findings.md), made five, none delivered. F1 (medium): collect-remote redacted the rendered text, and an escape puts a word character before a token - `\x09ghp_...` - so a credential right after a control character in a package's problem printed in clear. F2: a 403 whose body was lost ended the watch with "wait ... before retrying", and collect-remote drops its own way back to the run when an error names a remedy, so the operator was sent to a command that dispatches a second paid run; the same held, from before #442, for the other errors that end the watch with a remedy of their own - a 401's new token, a proxy's HTML page. F3: `printable` escaped C0 and DEL alone, and a reader splitting lines on U+0085, U+2028 or U+2029 - Python's `splitlines`, a JavaScript `^` under the m flag - read a line the package wrote, one saying PASS among them; `artifact validate` had printed them raw since before #442. F4: ADR-0201 and the draft docs said every 403 is judged by `forbiddenError`, though the artifact list's and download's are not. F5: `artifact create` printed its own problem lines, with no remedy and unescaped, so a package refused for a credential did not say to rotate it, and the workflow's `validate` step, which would have, does not run after the failed one. Outside this task, and left for the next: collect-remote's --json and the MCP server's `structuredContent.errors` carry package messages unredacted, as before #442; and after the watch, collect-remote drops its way back to the run the same way when a download error names a remedy of its own that does not - a proxy's HTML page answering the artifact list (BAD_BODY, "retry, then check api.github.com"), an output folder lost after the dispatch (OUT_DIR) - `e.remedy ?? pickUpAgain()` after `fetchCorpus`, as before #442. What it touched: fetchproof/lib/dispatch.mjs, fetchproof/lib/render.mjs, fetchproof/bin/artifact.mjs, fetchproof/bin/collect-remote.mjs; tests fetchproof/test/dispatch.test.mjs (two new, one variant changed), fetchproof/test/render.test.mjs (new, four), fetchproof/test/artifact-cli.test.mjs (one new), fetchproof/test/artifact-validator.test.mjs (one changed); docs/ARCHITECTURE.md rows `dispatch.mjs`, `render.mjs` and `bin/collect-remote.mjs`; docs/adr/0201 (its Area, decisions 3 and 6, a new decision 7, three rejected alternatives, Consequences), of this unmerged PR, amended in place, title unchanged; CHANGELOG.md (the F05 and F06 entries); fetchproof/README.md (2292 -> 2299 tests). No new flag, error code, exit or format (ADR-0117): FORBIDDEN, HTTP and RATE_LIMITED as before, and collect-remote's --json keeps its fields. What you verified: - verified: red first - with the draft's lib/render.mjs, lib/dispatch.mjs, bin/artifact.mjs and bin/collect-remote.mjs swapped in (the modules the second re-review read), `node fetchproof/bin/selftest.mjs render dispatch artifact-cli`, cwd E:\Claude\fp-p0-fixes: 78 passed, 5 failed - render's F1, F3 and source tests, the dispatch way-back test, and the artifact-cli create test, which failed there on its fixture, not its printer (mistake 9); with the fixture corrected, `selftest.mjs artifact-cli` with the draft's bin/artifact.mjs: 28 passed, 1 failed, at the printed lines; the modules restored each time and compared identical (cmp). Before that, the first re-review's tests copied into a worktree of 9527d0e (E:/Temp/fp-red-p1, removed after), `selftest.mjs dispatch render artifact-validator`: 119 passed, 3 failed - the new 403 test, the changed cut-off variant, and render's test file, which does not load there. - verified: the crc32-less test with `zlib.crc32` deleted for the whole run (an `--import` preload, as on 22.0 and 22.1): 9527d0e's test, 71 passed, 1 failed, "zlib.crc32 is not a function"; this commit's, 72 passed, 0 failed; without the preload 9527d0e's passed, as on the Node 24 that ran it. - verified: green - the groups that load dispatch.mjs, render.mjs, bin/artifact.mjs or bin/collect-remote.mjs, `selftest.mjs dispatch render artifact mcp cli`: 373 passed, 1 failed, the create test on its fixture (mistake 9); after its fix, `selftest.mjs artifact-cli`: 29 passed, 0 failed. - verified: bin/collect-remote.mjs end to end, GitHub stubbed by an `--import` preload, with a synthetic token: (a) run 7 whose wrapper's package is off by one in its CRC-32 prints "fix: download it again with --run-id 7; if it fails the same way, the stored artifact is damaged - re-collect" under the ZIP-READ problem, exit 1; (b) a package entry named with a TAB right before a token-shaped string prints `page\x09gh?_<redacted>.md` and the token nowhere, where the draft's modules printed it in clear, exit 1 in both; (c) a first poll answering 403 with its body lost ends with `... it needs "Actions" set to read and write; the run may be fine - pick it up again with --run-id 7 rather than dispatching another`, exit 2, where the draft's named no run number. - verified: bin/artifact.mjs over the review's 51 sample packages, against 9527d0e's, in its default, --quiet and --json modes: 153 runs, 0 differ (the kit's own path, which one warning names, masked); 45 print problems, each with its fix: lines, 4 with an escaped control character. - verified: the first re-review's probe cutoff403.mjs: the cut-off 403's TIMEOUT remedy begins "wait at least one minute before retrying; if it is refused again, the token may not read this repository's runs" and ends with the run's way back; the same 403 read in time is RATE_LIMITED, now followed by "the run may be fine - pick it up again with --run-id 7 rather than dispatching another". - verified: `node --check` on the four modules and four test files; no byte-order mark and no carriage return in the twelve staged files. - verified by this commit's hook: the full suite (ADR-0120). What you got wrong and fixed: 1. mistake: a 403 cut off at the deadline named the token's permission first, as fact; where: dispatch.mjs `readError`, `rateLimitError` and the watch's cut-off branch (9527d0e); impact: a watch whose poll met a secondary rate limit at the deadline told the operator the token lacked "Actions" - on fix/audit-p1, not merged; cause: I took the body the abort emptied for a body that named no limit, though ADR-0201's own rejected alternative said the cause can rest on that body; fix: a body not read is null, and the remedy names both causes, the wait first; verified: the changed variant and the new test red at 9527d0e, green here. 2. mistake: 9527d0e's CHANGELOG entry and ADR-0200 promised the "download again" remedy without my checking that collect-remote's default output prints remedies; where: CHANGELOG.md (the F05 entry), ADR-0200's Consequences; impact: the promise held for --json and the MCP server only; cause: I read `fetchCorpus`'s result, not the command's printer; fix: collect-remote prints through `renderProblems`; verified: the render tests red at 9527d0e, green here. 3. mistake: the test for a Node without `zlib.crc32` computed its expected sum with `zlib.crc32`; where: artifact-validator.test.mjs, the crc32-less test (56a86d2); impact: it threw on 22.0 and 22.1, the Nodes it was written for (CI runs neither); cause: I took `zlib.crc32` away around the reader only, and wrote the oracle with what was at hand; fix: the writer's `crc32`; verified: with `zlib.crc32` removed for the whole run, red at 9527d0e (TypeError), green here. 4. mistake: this commit's draft escaped a package's problems before collect-remote redacted them (F1); where: `renderProblems` and collect-remote.mjs's problem line, before staging; impact: none delivered - in the draft, a credential right after a control character in a package's problem printed in clear on collect-remote's default output; cause: I put the escaping in the renderer and left the redaction on the whole printed line, without asking what an escape does to the word boundary a credential's pattern starts at; fix: `renderProblems` asks `clean` of each field before it escapes it, collect-remote passes `redact`, and the saved file's name is redacted, then escaped; verified: the render test, red with the draft's modules, green here. 5. mistake: the draft's remedy for a 403 whose body was lost said to retry, and the watch gave no way back to the run (F2); where: dispatch.mjs `forbiddenError` and `waitForRun`, before staging; impact: none delivered - the draft sent an operator whose poll met that 403 to a command that dispatches a second paid run; cause: I wrote the remedy for the request, and checked the watch's TIMEOUT path, which names the run, not its error path, which collect-remote prints without its own way back once an error names a remedy; fix: `wayBack` follows the remedy of every error that ends the watch, unless it names `--run-id` or the error is a 404, which covers a 401 and a proxy's HTML page too; verified: the dispatch test, red with the draft's dispatch.mjs, green here. 6. mistake: the draft said package text "cannot begin a line" while `printable` escaped C0 and DEL alone (F3); where: render.mjs `printable`, and bin/artifact.mjs's `display` before it; impact: none delivered by the draft; since before #442, `artifact validate` printed U+0085, U+2028 and U+2029 from a package raw, and a reader splitting lines on them read a line the package wrote; cause: I took control characters for the C0 set and did not ask which characters a reader takes for a line end; fix: `UNPRINTABLE` adds C1, the Unicode line ends and the characters that hide or reorder text, and text in any script prints as it is; verified: the render test, red with the draft's render.mjs, green here. 7. mistake: ADR-0201's amendments and the draft docs said every 403 is judged by `forbiddenError` (F4); where: ADR-0201 decisions 3 and 6 and its Consequences, the draft CHANGELOG and ARCHITECTURE text; impact: words only, none delivered - the artifact list's and download's 403s name their status alone; cause: I generalised from the two callers I changed and did not list the module's 403 paths; fix: the docs name the dispatch and the run read, and say the other two are as before; verified: by reading the four 403 paths in dispatch.mjs. 8. mistake: `artifact create` kept a printer of its own, with no remedy and no escaping (F5); where: bin/artifact.mjs `create`, before staging; impact: none delivered - in the draft, a package refused for a credential did not say to rotate it; cause: I moved `validate`'s printer into the renderer without looking for the file's other printer, and the source test asked only that the count line was gone; fix: create prints through `renderProblems`, and the source test refuses a problem line of the script's own; verified: the artifact-cli test, red with the draft's artifact.mjs, green here. 9. mistake: the create test wrote its credential to a file at the project's root, which does not travel; where: artifact-cli.test.mjs, the create test, before staging; impact: none delivered - the package validated, create exited 0, and the test failed on its exit code whatever the printer, so the first red run did not show the draft's printer failing it, and the first green run was red; cause: I took every file of the project for one that is packaged, and did not read the producer's list (the scaffolding at the root, then research/ and docs/); fix: the key is written to docs/notes.md; verified: `selftest.mjs artifact-cli`, 29 passed with this commit's bin/artifact.mjs, and 28 passed, 1 failed, at the printed lines, with the draft's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aped by Unicode property wherever it is printed, and a download's error, or a 404 after the run was read, names the way back (re-review of #442, second pass) What changed: fetchproof/lib/render.mjs - `printable` escapes by Unicode property, not from a list of BMP ranges: every control (`Cc`), the line and paragraph separators (`Zl`, `Zp`), every format character (`Cf`), a lone surrogate (`Cs`) and every `Default_Ignorable_Code_Point`, by code point, as \xNN, \uNNNN or, past U+FFFF, \u{NNNNN}; the new `printableFile(file, { clean })` prints a file the kit wrote with its folder as the operator gave it and its name through `clean`, then `printable`. fetchproof/lib/mcp.mjs - the text `fetch_corpus` gives the agent (a package's diagnostics, an error, the saved file's link name) is redacted, then escaped, field by field, in the shape it had; `structuredContent` is unchanged. fetchproof/bin/collect-remote.mjs - `die` prints a refusal's error and remedy redacted, then escaped; the saved line prints through `printableFile(file, { clean: redact })`; the download stage's remedy is `downloadRemedy(e, pickUpAgain())`. fetchproof/lib/dispatch.mjs - the new `downloadRemedy(err, wayBack)`: the error's own remedy, then the way back to the run, unless the error is the artifact's own (NO_ARTIFACT, ARTIFACT_NAME, TOO_LARGE, EXPIRED: another download meets it again) or the remedy names `--run-id` already, and the way back alone for an error with none; in `waitForRun`, `wayBack` leaves a 404 alone only before any poll read the run - after one, the remedy says the token may have lost access to the repository or the run was deleted, with the run's page, then names the way back. Six tests, and one changed. ADR-0200 and ADR-0201 amended. Why: the re-review of c747126 (E:/Temp/review-followups/c4-findings.md, 2026-11-06) made five findings, and one I had left for the next commit is the same class as its F3. F1 (medium): the MCP server's `fetch_corpus` put a package's diagnostics, and an error's message, out redacted only, so a manifest key holding a line feed wrote the server's own PASS sentence on a line of its own in the text an agent reads - since before #442. F2 (medium): `printable` left 4,178 code points that draw as nothing raw, and every lone surrogate - the tag characters U+E0000-E007F, the variation selectors and the Hangul fillers among them - so a package's text could hide in a line a person reads to decide; c747126's list of BMP ranges was the fault. F3 (low): a 404 after a poll had read the run ended the watch with "check the number the dispatch printed" and no way back, about a number the watch had just read, and the one command at hand dispatches another run. F4 (low): collect-remote's refusals printed redacted only, and ARTIFACT_NAME quotes the name with `JSON.stringify`, which leaves U+2028 raw: an artifact named with U+2028 and a fake verdict put a line beginning "PASS build AUTHORIZED" on stderr (exit 2) - since before #442. F5 (low): c747126's saved line escaped the operator's own folder, so one spelled with U+200C (Persian) or U+200D (an emoji sequence) printed as a path that does not exist, and on Windows the escape's backslash named another folder. Mine: after the watch, collect-remote's `e.remedy ?? pickUpAgain()` dropped the way back when a download error named a remedy of its own - a proxy's HTML page for the artifact list ("retry, then check api.github.com"), a dropped download ("check connectivity, then retry"), a folder to fix - so "retry" read as running collect-remote again, which dispatches and pays for another run; as before #442, and c747126's message left it for the next. Escaping where the text is printed, not where each message is made, covers every source, at a cost ADR-0200 records: a refusal that quotes the operator's own path escapes a joiner in it. Outside this task, and left for the brief's ledger: an artifact name that passes ARTIFACT_NAME but is too long to write fails as UNKNOWN, and its --run-id remedy fails the same way every time (the review's stuck item, from before #442); collect-remote's --json and the MCP server's `structuredContent` carry package text neither escaped nor redacted, as before #442. What it touched: fetchproof/lib/render.mjs, fetchproof/lib/mcp.mjs, fetchproof/lib/dispatch.mjs, fetchproof/bin/collect-remote.mjs; tests fetchproof/test/render.test.mjs (two new), fetchproof/test/mcp.test.mjs (two new), fetchproof/test/cli.test.mjs (one new), fetchproof/test/dispatch.test.mjs (one new, and the watch test's 404 variant inverted); docs/ARCHITECTURE.md rows `dispatch.mjs`, `mcp.mjs`, `render.mjs` and `bin/collect-remote.mjs`; docs/adr/0200 (its Area, new decisions 6 and 7, six rejected alternatives, Consequences) and docs/adr/0201 (its Area, decision 7, a new decision 8, two rejected alternatives, Consequences), of this unmerged PR, amended in place, titles unchanged; CHANGELOG.md (the F05 and F06 entries); fetchproof/README.md (2299 -> 2305 tests). No new flag, error code, exit or format (ADR-0117): the MCP text keeps its shape, and collect-remote's --json its fields. What you verified: - verified: red first, in two steps, cwd E:\Temp\fp-c5 (a worktree of c747126). The first draft's two tests over c747126's render.mjs, collect-remote.mjs and dispatch.mjs, the old download-stage logic extracted as `downloadRemedy` (`err?.remedy ?? wayBack`), `node fetchproof/bin/selftest.mjs render dispatch`: 54 passed, 2 failed - the download's way back and `printable` past U+FFFF and on a lone surrogate. The second pass's tests over the first draft's modules, with mcp.mjs as at c747126, `selftest.mjs render mcp dispatch cli`: 259 passed, 5 failed - the cli refusal test, the watch test at its 404 after a read, the two mcp tests, and render's test file, which does not load without `printableFile`. - verified: green, `selftest.mjs dispatch render artifact mcp cli hardening proxy`: 465 passed, 0 failed; after the docs, `selftest.mjs architecture-map repo-hygiene`: 33 passed, 0 failed. The render test enumerates U+0000 to U+10FFFF: no character of the classes prints raw; 6,226 of the 6,321 it escapes were raw at c747126. - verified: bin/collect-remote.mjs end to end, api.github.com stubbed for run 7 by an `--import` preload, with a proxy at a closed local port behind it (nothing reached the network) and a synthetic token, this commit's kit against c747126's (E:\Claude\fp-p0-fixes before this commit): (a) an HTML page for the artifact list, and (b) a dropped download, named `--run-id 7` 0 times at c747126 and once here; (c) an artifact named with a TAB, U+2028 and "PASS build AUTHORIZED state=APPROVED_BRIEF" printed a line beginning PASS and a raw U+2028 at c747126, neither here; (d) a run queued at the first poll and 404 at the second named `--run-id 7` 0 times at c747126, once here; (e) a package whose manifest key holds a tag character, a variation selector and a Hangul filler printed them raw at c747126 and as `\u{e0041}\ufe0f\u3164` here; (f) that package saved under an --out folder whose name holds U+200C: the printed path exists here, not at c747126. - verified: `node --check` on the four modules and four test files; no byte-order mark and no carriage return in the thirteen staged files. - verified by this commit's hook: the full suite (ADR-0120). - untested: whether GitHub answers 404 for a run its dispatch has just returned (a first poll, left with "check the number"); whether GitHub accepts an artifact name holding a TAB; how a terminal draws tag characters. What you got wrong and fixed: 1. mistake: `printable` escaped a hand-written list of BMP ranges; where: render.mjs `UNPRINTABLE` (c747126); impact: on fix/audit-p1, not merged - 4,178 code points that draw as nothing, and the lone surrogates, printed raw from a package in both CLIs (artifact.mjs printed them raw before #442 too); cause: I listed the characters I knew hide text instead of naming the class Unicode defines; fix: the properties, escaped by code point; verified: the render test red at c747126, green here, and e2e (e). 2. mistake: I gave the watch's errors the way back to the run in c747126 and left the download's as outside the task; where: collect-remote.mjs's download catch; impact: as before #442 - an error that ended the download with a remedy of its own said to retry, and collect-remote run again dispatches a second paid run; cause: I fixed the function the finding quoted (`waitForRun`), not every place an error from the same run is printed; fix: `downloadRemedy`; verified: the dispatch test red with the old logic, green here, and e2e (a) and (b). 3. mistake: `fetch_corpus`'s text went out redacted only (F1); where: mcp.mjs, its diagnostics, its catch and the link's name; impact: since before #442, a package could write the server's PASS sentence, or an artifact's name a line, into what the agent reads; cause: c747126 escaped the two printers the finding named and I did not list every printer of package text; fix: `shown`, redact then `printable`, per field; verified: the two mcp tests red at the first draft, green here. 4. mistake: collect-remote's refusals printed redacted only (F4); where: collect-remote.mjs `die`; impact: since before #442, an artifact's name could put a fake verdict line on stderr; cause: as 3 - I escaped the problem printer and not the refusal printer beside it; fix: `printable(redact(...))` in `die`; verified: the cli test red at the first draft, green here, and e2e (c). 5. mistake: c747126's saved line escaped the operator's folder (F5); where: collect-remote.mjs, `saved ${printable(redact(file))}`; impact: on fix/audit-p1, not merged - a folder spelled with a joiner printed as one that does not exist; cause: I escaped the whole path for the name's sake without asking whose text the folder is; fix: `printableFile`; verified: the render test, which makes the folders and checks the printed path exists, did not load at the first draft and passes here, and e2e (f). 6. mistake: `wayBack` left every 404 without the way back (F3); where: dispatch.mjs `wayBack` (c747126); impact: on fix/audit-p1, not merged - a 404 after a poll had read the run sent the operator to check a number that was right, with only the command that dispatches another at hand; cause: I took a 404 for a doubt about the number without asking whether the watch had read the run; fix: the 404 is left alone only while `seen` is null, and otherwise says what it is and names the way back; verified: the watch test red at the first draft, green here, and e2e (d). 7. mistake: c747126's watch test pinned 6's behaviour; where: dispatch.test.mjs, the 404 variant; impact: the suite held the defect in place; cause: I wrote the variant from my rule, not from what the operator can do next; fix: the variant inverted, and a 404 before any read kept as it was; verified: as 6. 8. mistake: the end-to-end check's package with a hidden manifest key first proved nothing; where: its stub, before staging; impact: none delivered - the package stopped at MANIFEST-HASH-MISSING, then at the format check, before any key was quoted, and both kits printed the same; cause: I built the package without reading the validator's order; fix: a manifest that declares the format, with its digest; verified: the re-run quotes the key, raw at c747126 and escaped here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What changed - Brings PR #441's commits since the merge base 1078ead into the #442 branch: the corpus reader and writer fixes for F01 and its reviews (`hiddenLines`, `readTable`, `TABLE_HIDDEN`, decompose's named refusals). The merge itself changes no code: the two branches touch no module in common. Why - #442 is stacked on #441. #441 merges first; #442 is then retargeted to main and must hold #441's final head, so it merges with nothing left to reconcile and its CI runs the combined code. What it touched - Resolved by hand: docs/adr/README.md, where both branches appended rows after 0199 (kept #442's 0200-0202 and #441's 0203-0204, in number order), and fetchproof/README.md, the test count (2277 at the merge base; #442 adds 28 for 2305, #441 adds 36 for 2313; together 2341). - Merged by git: CHANGELOG.md (Unreleased holds #442's entries, then #441's), docs/ARCHITECTURE.md (each branch edited other rows), fetchproof/test/cli.test.mjs (tests added at different places). - No ADR owed: no decision was made. The map rows and ADRs come with each branch. What you verified - verified: `git merge-tree --write-tree HEAD dde656a` before merging named only those two conflicts. - verified: no conflict marker is left in any file the merge changed (grep). - verified: the groups repo-hygiene, architecture-map, cli and decompose on the merged tree: 230 passed, 0 failed (274 s). - verified: the commit gate ran the full suite on this commit (ADR-0120); it lands only when green, and it refuses a stale "N tests, offline" line, so 2341 is checked too. What you got wrong and fixed - Nothing to report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What changed - Brings PR #441's three commits since the merge base dde656a into the #442 branch: 50b18d1, 50c0afd and 5f44730, the corpus reader's fixes from the re-reviews of dde656a, 50b18d1 and 50c0afd (footnote references read by cmark-gfm's inline parse, each owner's HTML read by its own tokenizer, a table no check can judge refused; inline raw HTML and a `<select>` left open leave the tables after them in doubt; raw HTML over a line end read with its line end, and the texts in doubt marked in linear time). The merge itself changes no code: since dde656a the two branches touch no module and no test file in common. Why - #442 is stacked on #441. #441 merges first; #442 is then retargeted to main and must hold #441's final head, so it merges with nothing left to reconcile and its CI runs the combined code. What it touched - Resolved by hand: fetchproof/README.md, the test count (2313 at the merge base; #442 adds 28 for 2341, #441 adds 30 for 2343; together 2371). - Merged by git: CHANGELOG.md (#441 edited its F01 entry, #442 added its own), docs/ARCHITECTURE.md (each branch edited other rows). - No ADR owed: no decision was made. The map rows and ADRs come with each branch. What you verified - verified: `git merge-tree --write-tree HEAD 5f44730` before merging named only the README count line as a conflict. - verified: no conflict marker is left in any file the merge changed (grep). - verified: the count, by the test declarations in fetchproof/test: dde656a 2248, #442's head 2276 (+28), #441's head 2278 (+30), the merged tree 2306 (+58); 2313 + 58 = 2371. - verified: the groups repo-hygiene, architecture-map, cli and decompose on the merged tree: 230 passed, 0 failed in 240.5 s (h/merge-p1-groups.log), cwd E:/Claude/fp-p0-fixes. - verified: the commit gate ran the full suite on this commit (ADR-0120); it lands only when green, and it refuses a stale "N tests, offline" line, so 2371 is checked too. What you got wrong and fixed - Nothing to report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- What changed: nothing in the tree; this records main's merge commit of PR #441 (e7e10a4) in the #442 branch, which already held #441's head 5f44730. - Why: #442, retargeted to main after #441 merged, was behind main by that merge commit alone. - What you verified: verified - `git merge-tree --write-tree HEAD origin/main` gave HEAD's own tree (7ff7d4d), and e7e10a4's tree is 5f44730's (81b18d8), so no file changes; the full suite ran on this tree in the hook for 705e3a6. - What you got wrong: nothing to report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #441 (the P0 fixes). Its base is
fix/audit-p0, so the diff hereis the three P1 commits only. Merge #441 first, with a merge commit as the repository does, then retarget this
PR to
main. The offline suite workflow runs only on PRs intomain, so CI starts here after that retarget.What this changes
The Codex audit's three P1 findings against
532c696: F04, F05 (assertions F05a and F05b) and F06. Each fixhas a test seen to fail on the old code first, and its own commit with the five-part report.
d8d78e7).anchorFoundtried every chain of fragment occurrences by recursion, so aquote of repeated fragments ending in one the page lacks took exponential time, and the citations check
that preflight and the commit gate run waited on it. It now carries the set of placements fragment by
fragment, in bounded time, with the same verdict for every input (ADR-0162's matching rule kept). One
remedy keeps every verdict, so no ADR (ADR-0075).
723f4a4, ADR-0200). An approved package whose local header names another file, orwhose CRC-32 fields are zeroed, validated PASS, while Python's
zipfile.testzip()names the entry as bad.Every entry's local header must now agree with the central directory on the stored name bytes, the
compression method, the encryption and UTF-8 bits and, unless bit 3 defers them to a data descriptor, the
CRC-32 and both sizes. A body the directory's size puts past the end of the file is named as that first.
readEntrychecks each body's CRC-32. A ZIP read error inmanifest.jsonormanifest.sha256is now FAIL,as it is for a payload file, not BLOCKED.
c8f0e1b, ADR-0201).collect-remote's watch checked its deadline only after a pollanswered, so a hung request held it past
--timeout. One AbortController and timer per watch (re-armedpast the 2^31-1 ms a timer holds) now cut off the poll in flight, a cut-off poll ends in the existing
TIMEOUT (exit 4), a wait ends at the deadline at the latest, and no poll starts after it.
How it was checked
selftest quotes41 pass, 1 fail on the old code (the new test timed out at 10 s), 42/0 with thefix; a scratch differential run against the old recursion agreed on all 200,000 random cases; the groups
that reach
anchorFound362/0.selftest artifact-validator65/5 on the old code (exactly the five new tests), andselftest artifact137/0 with the fix. A first neighbour run was red: a directory size pointing past the end of thefile was reported as the two headers disagreeing. It was fixed before the commit, which reports it, and the
neighbours re-ran 346/0 and 207/0.
selftest dispatch44/3 on the old code (exactly the three expected), 47/0 with the fix, and theneighbour groups 518/0. The Node behaviour ADR-0201 rests on was checked on Node 24.20.0.
audit-regressions.mjs) on this head: 9 pass, 1 fail. Controls 1-3, F01, F03,F04, F05a, F05b and F06 pass; F02 fails at the refusal Independent audit P0 fixes: A-1, F01, F03, F02 and FP-011 (ADRs 0197-0199) #441 adds, red by design (see Independent audit P0 fixes: A-1, F01, F03, F02 and FP-011 (ADRs 0197-0199) #441). The Claude
audit's A-1 regression test: 2 of 2 pass.
For the owner
zipfileon the method, the two flag bits, the CRC-32 andthe sizes, on purpose: a streaming extractor reads them from the local header (ADR-0200).
Follow-ups (not in this PR)
deadline; a deadline under a second is still reported rounded ("after 0s").
🤖 Generated with Claude Code