fix: repair the trust guarantees and research claims from the 2026-09-26 deep review - #164
Merged
Merged
Conversation
Every reproduced finding (F01-F16) gets a fix and a regression test: - verify: the code-state fingerprint is a canonical, length-delimited manifest bound to HEAD (renames, byte repartitions, empty files, modes, symlinks; unreadable files fail closed) (F01); nested workspace suites are planned, run and reported as coverage (F08); a runner found only as a dependency is inventory, not an extra obligation (F09); a tree that changes while the tests run is INCOMPLETE, never a signed PASS (F10); each run records a MAC'd verifier event (A01). - reuse: exact keys are lossless except whitespace (F04); near candidates must pass a semantic guard; artifacts are revalidated at serve time against their file digest and dependency contracts, and a missing atlas is "unknown", not "ok" (F05). - ledger: evidence is counted per event, so aliases of one git object vote once (F06); a rewritten lesson inherits no trust unless equivalent (F07); archive reasons are recorded and an idle/duplicate archive is not a refutation (F15); similar-but-opposite rules are never merged (F16). - context: the rendered block never exceeds the budget while claiming success, pointers are pending reads, spans/truncation are explicit (F02, F03, R13). - dash: every route checks Host; writes need a session token and the exact origin (F13); missing spend is unknown, not $0 (A10). - router: sparse cost fits use the mean log cost with an explicit variance prior (F11); infeasible budgets/targets are explicit (F12); outcomes are schema-validated, idempotent and labeled self-reported (A04). - bench: every labeled reuse tier is validated before timing; all sketch caches are cleared for cold rows (F14, A06). - imagine: an isolated checkout, not a sandbox; foreign runners are an explicit unsupported result (A05). - CI runs the Python prototype suites and the research recomputation (A02). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
…c/cli/ A pure move (review A03): the ledger/reuse/context, verify/imagine/uicheck and route/models/cost command handlers now live in src/cli/memory.js, src/cli/verification.js and src/cli/routing.js, with the shared presentation helpers (heading, paint, table, bar) defined once in src/cli/shared.js. cli.js keeps dispatch, help and the remaining handlers (3,188 -> 1,670 lines). Handler bodies are unchanged apart from their relative dynamic-import paths; the package stays one installable unit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
Follow-up to the 2026-09-26 deep review fixes: - route universal (A07): each cascade step names the providers that can serve it and where its cost comes from; a recommendation containing a model no configured provider serves is labeled advice only (applicable: false, unmapped). The shipped default recommendation was two such models. - substrate (A06): a graph truncated by the atlas file cap says so (capped, skippedFiles) in the result and the rendered advisory. - verify: forge's own outputs under .forge/ (provenance.json, verify-events.jsonl) are no longer counted as changed files, matching the code-state fingerprint; nestedPackages treats an empty git listing as authoritative instead of falling back to a directory scan. - router cost fit: the sparse mean-log path no longer returns a value from a forEach callback (Biome useIterableCallbackReturn). - dash: template literals for the estimate labels (Biome useTemplate). - A02: seeded property tests for manifest transformations (any composition moves the fingerprint, undoing restores it) and for stale artifacts (never served after an edit, move or deletion). - bench: src/cli/memory.js is a labeled claimText dependent after the CLI handler move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
…inputs Review R10/R11/R12: the shipped prior's refit had no in-repo path, and the reviewer's attempt stopped at a 180 s limit. - reproduce.sh runs the whole path on a cold machine: a venv with the pinned pyarrow, build_input.py (downloads the sources pinned in sources.json and checks each sha256), fit_prior.mjs, then compare_priors.mjs against data/router_prior.json. Exit 0 only when every value matches except provenance.fittedAt. - Result on this container: all 176 fitted values identical (Node v22.22.2, Python 3.11, 4-vCPU Xeon @ 2.80GHz, fit ~413-424 s). The dataset revision 78f471b lives in SWE-bench/SWE-bench_Verified. - holdout_eval.mjs is a NEW seeded 150/350 held-out experiment, labeled as such (not a reproduction of the external 76.3% / $0.093 headline): the router does not beat a dev-chosen fixed cascade on solve rate (80.0% vs 80.6%, paired bootstrap) and is slightly cheaper; expected cost is under-predicted by 5-22% because failed attempts cost 1.2-2.0x as much as successful ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
Research and documentation findings R01-R15 plus the documentation side of A05, A07-A10 and A12: - Claim/status registry (docs/status/claims.json, 47 claims assessed at d2abfa6) with a generated table; scripts/claims-status.mjs --check validates it, fails on a stale table or a drifted docs/ copy of a research/ file, reads CRLF checkouts correctly, and now runs in the CI quality gate. - Cost model: scenarios built on the refuted 0.62 routing factor are withdrawn; stage savings are hypotheses; a cost headline must meet a stated acceptance rule. Cost numbers state currency, basis and missing-data status (missing logs are unknown, not $0). - Theorem D joint attainability (with counterexample), the silent-miss bound's equality condition, "a caught miss is not a completed task", the frozen-model thesis restated with in-context learning, prior art (CoALA, Reflexion), threshold-separated impact-oracle results; recompute_corrections.py --theorem-checks asserts them without data. - Universal router: the run-4 headline is repository-reported; the prior's exact in-repo refit, the new held-out replay and the modeling limits (cost under-prediction, optimistic targets, budgets bound only expected cost, infeasible results, outcome provenance) are documented. - Evidence grades split bibliographic verification from claim support, design, replication and transfer; METR scoped to its study. - Qur'anic lens: source text, translation, tafsir and the author's design analogy are labeled separately; no Arabic text or translation changed (verified run by run). - Research PDFs are recorded as historical, pre-correction editions (research/HISTORICAL_EDITIONS.md) and were not re-rendered. - docs/INTEGRATIONS.md: per tool, config emission, MCP registration, automatic hooks and enforcement, each tested/declared/unsupported. - README leads with the three jobs; P4/P8 partial; imagine --run is an isolated checkout, not a sandbox; UI checks are advisory; legacy files carry archive banners. - GUIDE: verify coverage/binding/events, stack inventory, ledger archive reasons and conflicts, reuse semantics and revalidation, dashboard Host/token, route advice-only models. - Benchmarks re-measured after the CLI move (reports/benchmarks.md, README, landing: 851 ms gate, 1.68 ms blast radius). - CHANGELOG [Unreleased] covers the whole review repair. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+149
to
+151
| String(s ?? "") | ||
| .replace(/\r?\n/g, " ") | ||
| .replace(/\|/g, "\\|") |
CodeWithJuber
marked this pull request as ready for review
September 26, 2026 21:05
CodeWithJuber
pushed a commit
that referenced
this pull request
Sep 26, 2026
CodeQL flagged one new high-severity alert in #164 and two older ones. A local run of the same CodeQL version (2.27.0, javascript-code-scanning suite) goes from 3 results to 0 with this change. - scripts/claims-status.mjs (js/incomplete-sanitization, new in #164): table cells escaped `|` but not `\`, so a trailing backslash could undo the escape. Backslashes are escaped first. - src/model_catalog.js (js/polynomial-redos, pre-existing): the tokenizer's `/(?:\.0)+$/` and trimUrl's `/\/+$/` backtracked quadratically on library input. Trailing ".0" parts are popped instead, and URLs use a new linear stripTrailingSlashes (src/util.js). - The same patterns in code added by #164 are hardened too: the workspace-glob trim (src/stack.js) and the semantic guard's edge punctuation trim, now a code-point scan (trimEdges). Each replacement is checked against the regex it replaced (tokenize on known ids; trimEdges on 2,000 seeded random tokens) and gets a linear-time test on a hostile input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
CodeWithJuber
pushed a commit
that referenced
this pull request
Sep 26, 2026
After the 2026-09-26 review fixes merged (#164), several surfaces still described the old behavior: - Mintlify reference: forge verify (per-package coverage, pre/post binding, verifier events, .forge/forge.config.json), forge stack (available runners), forge context (what COMPLETE means, --budget, --block), forge reuse and forge ledger (lossless keys, serve-time revalidation, one vote per event, archive reasons, conflicts, --fix --dry-run), forge dash (Host check on every route, session token), and the universal router, which the site did not cover at all (objectives, INFEASIBLE, advice-only models, outcome provenance, and the limits of its evidence). - Landing page: all ten native targets (OpenClaw was missing, "Nine native targets"). The grid is now two rows of five on wide screens and two columns below 1180px, checked in headless Chromium at eight widths with no overflow or horizontal scroll. - Claim registry: the claims the review found refuted at d2abfa6 are re-assessed on 7eef611 (the merge), after re-running the review's probes as regression tests (46 pass; 1 skip needs a non-root user): verify binding, context completeness and exact reuse are implemented (with their scope in the notes); evidence independence and outcome provenance are partial; the imagine sandbox and router target claims stay refuted. The impact fixture's F1 is 0.28 after the CLI move. - biome.json migrated to the installed Biome 2.5.13 (schema URL; `recommended` → `preset`); lint results unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
Merged
12 tasks
CodeWithJuber
pushed a commit
that referenced
this pull request
Sep 26, 2026
The v1.5.0 release commit (fd71bf8) moved the [Unreleased] notes of #164 under [1.5.0] and bumped the version strings; this branch had added its own notes to the same [Unreleased] section, so the PR could not merge and CI never ran. Resolution: - CHANGELOG.md: this PR's four new entries (the CodeQL fixes, the generated changelog page, the Mintlify reference sync and the two historical corrections) move back to [Unreleased]; [1.5.0] keeps exactly what shipped, apart from this PR's bold headline leads. Fixes a doubled "separately" in the Qur'anic-lens entry. - landing/index.html: this branch's page with the release's version strings (softwareVersion and both "forgekit v1.5.0" labels), as scripts/bump.mjs writes them. - mintlify/changelog/overview.mdx: re-rendered with `forge docs render`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
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
3 participants
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.
What & why
An external deep review of
d2abfa6(2026-09-26) found 43 items: 16 reproduced defects (F01–F16), 15 research and documentation findings (R01–R15), and 12 architecture recommendations (A01–A12). Its common theme was a label (PASS,complete,exact, trusted,$0) that claimed more than the evidence behind it. This PR fixes every reproduced defect, each with a regression test. It also corrects the research and docs, and implements the recommendations that can be done in code.Reproduced defects (all fixed, one regression test each):
manifest-v2); an unreadable file makes the state unbindableforge contextreturnedok: trueover its hard budgetoverflow: true,ok: false- read <file>pointer counted as coveragependingread>=/<=, case and literalsunknownINCOMPLETEtestInventory, "available")INCOMPLETEwithmutated: truefeasible: false,reason,minimumExpectedCost, labeledfallbackattic/<id>.log; only retracted or dormant claims drop lessonsResearch and docs (R01–R15). The refuted routing factor is out of the live cost model. A claim/status registry (
docs/status/claims.json) is now checked in CI byscripts/claims-status.mjs --check. Theorem D and the silent-miss bound are restated. The universal-router headline is labeled repository-reported. P4 is marked partial. The Qur'anic lens is kept separate from empirical claims, with the Arabic text and translations unchanged. Citation existence and claim support are graded separately. The historical editions are indexed inresearch/HISTORICAL_EDITIONS.md.Router reproduction (R10/R11/R12).
bench/universal-router/reproduce.shrebuilds the shipped prior from pinned, sha256-checked public inputs. The refit matchesdata/router_prior.jsonexactly: all 176 fitted values, with onlyfittedAtdifferent. It ran on Node v22.22.2 and took about 413–424 s on a 4-vCPU Xeon, so the reviewer's 180 s limit was too short. The revision78f471blives inSWE-bench/SWE-bench_Verified. A new held-out experiment,holdout_eval.mjs, is labeled as new and not as a reproduction. In it the router does not beat a fixed cascade on solve rate (80.0% vs 80.6%) and is slightly cheaper. It also under-predicts cost by 5–22%, because failed attempts cost 1.2–2.0× as much as successful ones; R12 is now documented as a measured limit. The 76.3% / $0.093 headline still cannot be reproduced from this repo, because its harness is external.Recommendations: A01 immutable, MAC'd verifier events (
.forge/verify-events.jsonl), whichroute outcome --verify-runcan cite · A02 seeded property tests (test/trust_properties.test.js) and a CIresearchjob (49 + 23 pytest, theorem checks, recomputation) · A03src/cli.js3,117 → 1,670 lines by a pure move intosrc/cli/· A04src/schema.jsvalidation for.forge/models.json, router outcomes (plus attempt ids) and evidence records · A05imagine --rundescribed as an isolated checkout, not a sandbox, with unsupported runners reported · A06 benchmarks record fs type, commit and uncommitted state, andforge substratereports a capped graph · A07docs/INTEGRATIONS.md, androute universallabels models no provider serves asadvice only· A08 UI checks documented as advisory · A09 onboarding leads with three jobs · A10 cost numbers labeled as estimates, with missing logs shown as unknown and not$0· A11 archive reasons, lineage,--fix --dry-run, and a concurrent-append test · A12 legacy and research material marked historical.Not done here, on purpose:
research/HISTORICAL_EDITIONS.md(git blob and sha256 for each). A fresh render set the Qur'anic Arabic in mixed fallback fonts that nobody could check, so it was not published. The refutation paper also needs a TeX toolchain, which isn't installed here.The zero-runtime-dependency rule (ADR-0001) meant the schema and semantic-guard helpers were written in-house instead of pulling in a library.
Checklist
npm testpasses: 1,678 tests, 1,674 pass, 0 fail, 4 platform-gated skips (Node 22 locally; CI runs the matrix)npm run checkpasses (Biome lint + format)feat:/fix:/docs:…)CHANGELOG.mdupdated under## [Unreleased]forge substrate,forge impact, router/gate, or MCP substrate toolsRisk & rollback
Risk level: medium. Several results are deliberately stricter, and scripts that read
--jsonmay need to adapt:verifycan now returnINCOMPLETEwhere it used to return PASS, and stamps from older forge versions no longer verify.contextreports overflow.route universalreturnsok: falsewhen the request is infeasible.These warrant a minor version bump at release.
Rollback plan: revert the merge commit. The on-disk formats are additive: new fields,
attic/<id>.log, andverify-events.jsonl. After a rollback, re-runforge verifyso the stamp uses the older fingerprint again.Extra checks (tick if applicable)
npm run typecheckpasses🤖 Generated with Claude Code
https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
Generated by Claude Code