release: prepare Core Reset 0.40.0-beta.1 - #617
Conversation
* docs: establish Core Reset governance * test: strengthen Core Reset governance * docs: accept Core Reset RFC
* test: freeze Core Reset baseline contract * test: harden Core Reset measurement integrity * test: record Core Reset baseline evidence * docs: clarify baseline measurement provenance * docs: address baseline review findings
…583) * refactor: replace graph core with deterministic directed multigraph * test: normalize core reset metrics on Windows * test: make core reset LOC checks platform-neutral * fix: address directed multigraph review findings * fix: validate freshness against exact artifact bytes
Record the passed graph phase, its final evidence, and the canonical TypeScript index as the only Ready successor.
* feat: replace SPI with canonical TypeScript index * fix: address canonical index review findings
Record the completed canonical index evidence, the owner-approved CodeRabbit rate-limit exception, and the combined legacy/non-code deletion as the only Ready work item.
* docs: activate legacy extraction deletion * test: enforce deletion activation inventory * style: align governance test types * chore: update vulnerable fast-uri transitive
Deletes the retired extraction architecture and makes the canonical TypeScript/JavaScript index the sole generation path. Preserves fail-closed predecessor gates, freshness/worktree behavior, and package/test coverage. Governance closure remains tracked in #588.
Records the completed legacy/non-code deletion receipts, leaves the final package gate open, and makes generation-and-incremental Ready but not active. Closes the phase governance without changing production source.
Activates the owner-approved #592 governance contract without runtime or package changes.
* test: record failed incremental index experiment * refactor: simplify index refresh to full reconciliation * refactor: replace incremental refresh with full reconcile * docs: record full reconcile acceptance evidence * fix: canonicalize Windows index paths * docs: reseal incremental acceptance evidence * test: isolate incremental policy reconciles
Closes #592. Records the final generation/reconciliation evidence and leaves evidence-path query Ready but not active.
Freeze the owner-approved deletion, replacement, held-out, correctness, performance, package, and review contracts for the evidence-path query phase. This governance-only activation changes no production or released behavior.
…ence paths (#600) * test(core-reset): replace held-out evidence governance * refactor(core-reset): replace evidence query stack * feat(core-reset): implement evidence-path query * fix(eval): canonicalize held-out path arguments * test(core-reset): record held-out evidence * fix(query): preserve explicit relation paths * test(core-reset): refresh held-out evidence * perf(query): avoid redundant structural scans * test(core-reset): record performance evidence * test(core-reset): refresh optimized held-out evidence * test(core-reset): record phase inventory * fix(query): preserve exact declaration targets * test(core-reset): refresh final query evidence * fix(query): close evidence path acceptance gates * docs(core-reset): bind final evidence receipts * test(package): require successful retrieval parity * fix(query): make evidence boundaries platform-stable * chore(deps): update transitive postcss security fix * fix(query): bound retrieve question work * docs(core-reset): bind final portable evidence receipts
Activate the exact owner-approved #602 Thin Delivery governance contract on core-reset. Governance-only activation: zero production/package delta. All six exact-head CI jobs passed, independent review found no blocker, zero review threads remained, and CodeRabbit explicitly skipped the non-default base.
* feat(core-reset): implement Thin Delivery contract * fix(ci): escape benchmark MCP paths
Governance-only activation of the exact owner-approved #606 contract against core-reset. No production code, package, dependency, publication, release, Registry, main-branch, or Capability Validation change.
Exact approved governance-only Capability Validation v2 activation. Offline implementation remains limited to the accepted contract; campaign/provider execution remains unauthorized.
📝 WalkthroughWalkthroughThis PR implements the accepted Core Reset (RFC ChangesCore Reset Implementation
Estimated code review effort: 5 (Critical) | ~180+ minutes Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant Server as McpServer
participant Runtime as McpRuntime/WatchIndex
participant App as retrieveContext (application)
participant Graph as KnowledgeGraph (graph.json)
Client->>Server: initialize / tools-list
Server->>Runtime: start reconciler (first ready)
Runtime->>Graph: generateIndex / updateIndex
Graph-->>Runtime: accepted build_id
Client->>Server: tools/call retrieve(question, budget)
Server->>Runtime: loadQueryIndex()
Runtime-->>Server: ReadyQueryIndex or unavailable
Server->>App: retrieveContext(index, request)
App->>Graph: authenticate nodes/edges via source hashes
App-->>Server: RetrieveContextResult (evidence/boundaries)
Server-->>Client: JSON-RPC result (madar.retrieve envelope)
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
b80261d to
07ac630
Compare
07ac630 to
691b83a
Compare
mohanagy
left a comment
mohanagy
left a comment
There was a problem hiding this comment.
Independent exact-head review — no blocker found.
Reviewed exact head 691b83abce9a1fc47c13f8ff9bc06741b5470c2a, tree c587bdbe811034e9873d2d81951b64aa1b9e5d04, against protected Core Reset anchor 5c9d1e2436932f7420169ea4ffa617c6bea4fbd0 and PR base next.
The review verified that the original three blockers are resolved: publication uses the existing tag-triggered release.yml; the lock-only SBOM has 105/105 lock-backed components with a regression guard; and Capability Validation is explicitly historical/stopped with all unused authority revoked. The release workflow binds checkout, remote next, and the peeled remote release tag to GITHUB_SHA during initial validation, at publication-step entry, immediately before irreversible npm publication, and again before GitHub prerelease creation. It publishes npm next with tokenless OIDC provenance before creating the prerelease, verifies exact shasum/integrity, dist-tags, provenance, signatures, authoritative remote tag, prerelease state, and unchanged latest, and has no token/no-provenance fallback. No src/** or dependency change exists. No blocker was found. CI remains a separate exact-head merge gate.
|
Pre-merge CodeRabbit disposition for exact head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/eval/lib/infrastructure/benchmark/suite.ts (1)
853-868: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRecursive copy publishes every compare artifact; only
report.jsonis scrubbed.
cpSynccopies the whole compare output root into the publishedraw/tree, then just overwritesreport.jsonwith the share-safe body. Any other non-share-safe artifact produced byexecuteNativeAgentCompare(transcripts, raw provider output) is published as-is. Consider copying an explicit allowlist of artifact names instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/eval/lib/infrastructure/benchmark/suite.ts` around lines 853 - 868, The copyReportArtifacts function currently publishes every compare artifact and only sanitizes report.json. Replace the recursive directory copy with an explicit allowlist of share-safe artifact names, copying only those entries into copiedRoot; continue validating and writing report.json from report.share-safe.json, and do not publish transcripts or raw provider output.
🟠 Major comments (25)
.github/scripts/verify-packed-retrieval-parity.mjs-60-85 (1)
60-85: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftByte-exact tarball identity is pinned in two places and assumes a reproducible
npm packacross three different npm installs. The recorded shasum/integrity/SHA-256 values are produced by gzip plus a specific npm version;.github/workflows/release.ymlpinsnpm@12.0.1(line 36) while.github/workflows/ci.ymlrunsnpm run verify:pack-paritywith whatever npm the runner image ships, and the receipt indocs/core-reset/removal-manifest.ymlwas measured on a maintainer machine. Any of those drifting turns a correct change into a hard release/CI failure.
.github/scripts/verify-packed-retrieval-parity.mjs#L60-L85: keep file-count and the budget maxima as the CI-side gate, and move the exactnpm_shasum/npm_integrity/npm_artifact_sha256equality behind a flag that only the pinned release workflow sets..github/workflows/release.yml#L166-L167: deriveEXPECTED_SHASUM/EXPECTED_INTEGRITYfrom the receipt indocs/core-reset/removal-manifest.yml(single source of truth) instead of duplicating literals here, so a re-measured artifact only has to be updated once.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/verify-packed-retrieval-parity.mjs around lines 60 - 85, Keep the file-count and budget checks unconditional in verify-packed-retrieval-parity.mjs, but gate the npm_shasum, npm_integrity, and npm_artifact_sha256 equality checks behind an explicit flag enabled only by the pinned release workflow. In .github/workflows/release.yml, derive EXPECTED_SHASUM and EXPECTED_INTEGRITY from docs/core-reset/removal-manifest.yml instead of duplicating literals, while preserving the existing release verification behavior..github/scripts/verify-packed-retrieval-parity.mjs-469-473 (1)
469-473: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftTiming-dependent assertion is a flake source in a required gate.
The check requires the
ping(id 702) response to arrive and the queuedretrieve(id 701) to still be absent within a 5s window. On a fast runner (or if reconcile of the 300-file workspace completes quickly), 701 can land before the poll loop observes 702, failing CI for a correct runtime. Conversely a slow runner never sees 702 in 5s.Assert responsiveness by ordering rather than absence — e.g., record the arrival index of 702 vs 701 in the accumulated stream and require 702 to precede 701 — and derive the deadline from
requestWaitMsinstead of a fixed 5s.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/verify-packed-retrieval-parity.mjs around lines 469 - 473, Update the responsiveness assertion in the packed MCP stdio verification flow to compare the arrival positions of responses 702 and 701 in the accumulated stream, requiring 702 to precede 701 rather than requiring 701 to remain absent during polling. Use a deadline derived from requestWaitMs instead of the fixed 5-second timeout, while preserving failure handling when 702 is not received or arrives after 701..github/scripts/ci-eval-regression.mjs-30-37 (1)
30-37: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNon-finite metrics silently pass the gate.
If
reportdrifts (renamed/missingavg_recall,mrr,avg_snippet_coverage), the comparisons becomeNaN < 90→falseand CI reports success with no enforcement. Validate the metrics are finite before comparing.🛡️ Proposed guard
const recall = report.avg_recall * 100 const snippetCoverage = report.avg_snippet_coverage * 100 +for (const [label, value] of Object.entries({ recall, mrr: report.mrr, snippetCoverage })) { + if (!Number.isFinite(value)) { + throw new Error(`eval report metric ${label} is not a finite number: ${value}`) + } +} if (recall < 90 || report.mrr < 0.95 || snippetCoverage < 95) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/ci-eval-regression.mjs around lines 30 - 37, Update the threshold validation around recall, report.mrr, and snippetCoverage to reject any non-finite metric values before performing comparisons. Ensure missing, renamed, NaN, or infinite values throw an evaluation-threshold error rather than allowing CI to pass, while preserving the existing threshold checks and error reporting for valid metrics.tools/eval/core-reset/schemas/capability-validation-receipt.schema.json-158-158 (1)
158-158: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUpdate the hardcoded source identities in
campaignLockandcampaignLockPass
Theimplementation_source_hashesentries still point totools/eval/core-reset/capability-validation.mjs,tools/eval/lib/capability-validation/contract.ts,tools/eval/lib/capability-validation/report.ts, andtests/unit/capability-validation-contract.test.ts, but the repo only containstools/eval/core-reset/contract-validation.mjs. If these paths stay as-is, receipts built from this schema will fail validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/eval/core-reset/schemas/capability-validation-receipt.schema.json` at line 158, Update the implementation_source_hashes path constants in both campaignLock and campaignLockPass to reference the repository’s actual tools/eval/core-reset/contract-validation.mjs source identity, removing the stale capability-validation entry paths while preserving the existing identity ordering and hash validation structure.tools/eval/core-reset/evidence-path-held-out.mjs-1337-1344 (1)
1337-1344: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
--permissionhere.--experimental-permissionis rejected by Node v24.15.0, so held-out children abort before executing andrunContainedNodefails immediately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/eval/core-reset/evidence-path-held-out.mjs` around lines 1337 - 1344, Update the nodeArguments construction in runContainedNode to pass Node’s supported --permission flag instead of --experimental-permission, while preserving the existing read/write permission arguments and entry-point ordering.tools/eval/core-reset/schemas/capability-validation-receipt-v2.schema.json-404-429 (1)
404-429: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
builder_pathpoints at a file that isn't present in this tree, so v2 receipts won't satisfy this schema until that builder lands.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/eval/core-reset/schemas/capability-validation-receipt-v2.schema.json` around lines 404 - 429, The builder_path constant in the receipt schema references a file absent from the tree; update it to the actual existing v2 receipt-builder location, while preserving the required builder-path validation for valid receipts.tools/eval/lib/shared/shell.ts-25-42 (1)
25-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLogin shell is the wrong default for measured runs.
options.login === false ? '-c' : '-lc'makes-lcthe default, and the only caller (compare.tsline 235) passes no options. A login shell sources the operator's profile/rc files, so PATH, proxies, version managers, and agent configuration leak into every measured arm — directly at odds with the scrubbed-environment requirements the surrounding contracts impose, and it makes results host-dependent. Invert the polarity so login is opt-in.🛠️ Proposed fix
return { file: '/bin/sh', - args: [options.login === false ? '-c' : '-lc', command], + args: [options.login === true ? '-lc' : '-c', command], useProcessGroup: true, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/eval/lib/shared/shell.ts` around lines 25 - 42, Update resolveShellCommand to make the non-login '-c' mode the default for non-Windows platforms, enabling '-lc' only when options.login is explicitly true. Preserve the existing Windows command handling and process-group behavior.tools/eval/lib/infrastructure/compare.ts-231-294 (1)
231-294: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTimeout leaves the real agent process running.
resolveShellCommandreturnsuseProcessGroup: true, butspawnhere is notdetached, so the abort on timeout terminates only/bin/sh. The agent process it launched survives as an orphan — it keeps burning provider tokens and CPU and may still writeanswerFileafter the arm is recorded astimed_out. HonouruseProcessGroupand kill the group.🛠️ Proposed fix
const command = resolveShellCommand(input.command) const child = spawn(command.file, command.args, { cwd: input.cwd, shell: false, + detached: command.useProcessGroup, signal: input.signal, stdio: ['ignore', 'pipe', 'pipe'], }) + if (command.useProcessGroup && child.pid !== undefined) { + input.signal?.addEventListener('abort', () => { + try { + process.kill(-child.pid!, 'SIGKILL') + } catch { + /* group already gone */ + } + }, { once: true }) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/eval/lib/infrastructure/compare.ts` around lines 231 - 294, Update defaultNativeAgentRunner to honor the useProcessGroup value returned by resolveShellCommand: spawn the command detached when requested, and ensure abort/timeout handling in runNativeArm terminates the entire process group rather than only the shell. Preserve normal process cleanup and existing result reporting.examples/quick-benchmark.sh-15-18 (1)
15-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInstall the authorized beta channel explicitly.
Without
@next, npm resolves its default channel, so this script can install a different stable or older CLI contract than the0.40.0-beta.1workflow being documented.Proposed fix
- echo "Install Madar first: npm install -g `@lubab/madar`" + echo "Install Madar first: npm install -g `@lubab/madar`@next"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/quick-benchmark.sh` around lines 15 - 18, Update the missing-madar installation guidance in the quick benchmark script to explicitly install the authorized beta channel by appending `@next` to the `@lubab/madar` package specifier. Keep the existing command-not-found check and exit behavior unchanged.docs/mcp-registry/server.json-19-28 (1)
19-28: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake the
npxlaunch non-interactive. AddruntimeArguments: [{ "type": "named", "name": "--yes" }]; otherwise uncached installs can prompt and stall MCP hosts without stdin.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/mcp-registry/server.json` around lines 19 - 28, Add a runtimeArguments entry to the server configuration alongside the existing runtimeHint and transport settings, using a named --yes argument so npx launches non-interactively. Preserve the existing packageArguments configuration.docs/benchmarks/suite/holdouts/tasks.json-7-8 (1)
7-8: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not silently remove a frozen holdout prompt.
This changes the evaluator workload and can invalidate existing receipts and comparability. Restore the prompt, or amend the evaluation contract and regenerate every dependent receipt before merging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/benchmarks/suite/holdouts/tasks.json` around lines 7 - 8, Restore the frozen “holdout-order-service” prompt in the prompts configuration, preserving its existing text and key. Do not remove or alter the evaluator workload unless the evaluation contract is explicitly amended and all dependent receipts are regenerated.docs/roadmap.md-91-96 (1)
91-96: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDisambiguate the
<80 MiBRSS gate.The gate reports metrics for both commands, while the recorded initialize/tools-list maximum RSS is 208,977,920 bytes, well above 80 MiB. If the cap applies to both commands, the evidence fails; if it applies only to
madar --version, scope the requirement explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap.md` around lines 91 - 96, Clarify the RSS requirement in the roadmap entry so the <80 MiB cap explicitly applies only to madar --version, or revise the recorded initialize/tools-list result and acceptance criteria if both commands must satisfy it. Keep the metrics and claims consistent with the chosen scope.docs/roadmap.md-134-136 (1)
134-136: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRecord the required publication ordering and verification gates.
The release sequence must explicitly require OIDC provenance, immutable package verification, exact
next-tip confirmation, npm-before-GitHub ordering, and Registry publication only after exact package confirmation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap.md` around lines 134 - 136, Update the release sequence in the roadmap to explicitly require OIDC provenance, immutable verification of the published package, confirmation that the reviewed commit is the exact tip of next, npm publication before creating the GitHub prerelease, and MCP Registry publication only after confirming the exact package. Preserve the existing beta scope and all listed validation gates.docs/migrations/0.40.0-beta.1.md-78-80 (1)
78-80: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign the installer byte-mutation contract across all documentation.
The documents promise zero repository-byte changes for uninstall while also documenting cleanup of repository-local, Madar-owned legacy artifacts. Make the exception explicit everywhere:
docs/migrations/0.40.0-beta.1.md#L78-L80: distinguish normal byte-neutral operations from legacy cleanup.docs/reference/cli-and-mcp.md#L110-L124: qualify the uninstall guarantee.docs/roadmap.md#L90-L90: clarify the thin-delivery contract.docs/security/mcp-threat-model.md#L31-L35: state that exact owned-artifact cleanup is the limited exception.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/migrations/0.40.0-beta.1.md` around lines 78 - 80, Align the installer byte-mutation contract across all documented sites: in docs/migrations/0.40.0-beta.1.md lines 78-80, distinguish normal byte-neutral install, reinstall, and uninstall operations from exact Madar-owned legacy cleanup; in docs/reference/cli-and-mcp.md lines 110-124, qualify the uninstall guarantee with that exception; in docs/roadmap.md line 90, clarify the thin-delivery contract similarly; and in docs/security/mcp-threat-model.md lines 31-35, state that exact owned-artifact cleanup is the sole limited exception.docs/share-safe-dashboard-decision.md-8-8 (1)
8-8: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not promise that
report.share-safe.jsonhides workstation paths.Share-safe artifacts are documented elsewhere as best-effort redactions that must be reviewed before publication. Qualify this sentence to avoid a false privacy guarantee.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/share-safe-dashboard-decision.md` at line 8, Update the description of report.share-safe.json in the documentation to state that it provides best-effort redaction only, rather than guaranteeing workstation paths are hidden. Direct readers to review the artifact before sharing or publication, while preserving its purpose for benchmark and compare receipts.docs/benchmarks/suite/isolation/environment.json-9-9 (1)
9-9: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRecompute
project_claude_md_hashfromdocs/benchmarks/suite/isolation/.claude/CLAUDE.md. The stored value doesn’t match the file’s current SHA-256 (sha256:0cf98a0a8f9c27017a0bebc7b017fc17518d9103c62ca384da20b2776b0aade7vssha256:729ce56a7c59791da8720f3d30d9a580a12d0115072afbc9815e8296f6b8b386), so the isolation attestation is stale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/benchmarks/suite/isolation/environment.json` at line 9, Update the project_claude_md_hash value in environment.json to the current SHA-256 of the isolation .claude/CLAUDE.md file: sha256:0cf98a0a8f9c27017a0bebc7b017fc17518d9103c62ca384da20b2776b0aade7.docs/claims-and-evidence.md-17-21 (1)
17-21: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAnchor historical comparisons to exact receipts or remove them.
Lines 17-21 still publish comparative tool-call, input-token, latency, and cost results, but provide no dated receipt links or repository/task conditions. Either link the immutable historical artifacts and label this explicitly as archived, non-beta evidence, or remove the comparison to preserve the PR’s no-comparative-claims boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/claims-and-evidence.md` around lines 17 - 21, Update the historical comparison in the surrounding claims-and-evidence section to either link immutable, dated receipts that document the repository and task conditions and label the results as archived non-beta evidence, or remove the comparative tool-call, input-token, latency, and cost claims entirely. Preserve the no-comparative-claims boundary if verifiable artifacts are unavailable.src/domain/query/slice.ts-100-113 (1)
100-113: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSelf-referential token count can fail to stabilize and the fallback is very expensive.
serialized_tokensis embedded in the very payload being tokenized, sof(n) = countTokens(serialize(result(n)))can oscillate between two values across a digit boundary (e.g.999 ↔ 1000), exhausting the 16 passes. The fallback then performs up to 10,001 full canonical serializations + tokenizations on the request path, and — becauseresultWithTokenCountis also called on the pre-budgetcappedResult(line 183), whose count can exceed 10,000 tokens — it can miss the fixed point entirely and throwUnable to stabilize retrieve serialized token count, failing the whole retrieve call.Consider removing the self-reference instead of searching for it: either exclude
metrics.serialized_tokensfrom the tokenized payload, or serialize it as a fixed-width value so the count is digit-length invariant. If iteration is kept, detect the 2-cycle and pick the larger of the oscillating pair rather than scanning a bounded range.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/query/slice.ts` around lines 100 - 113, The resultWithTokenCount stabilization logic in src/domain/query/slice.ts must avoid the expensive bounded fixed-point scan and failure for oscillating or large token counts. Prefer removing the self-reference by excluding metrics.serialized_tokens from the tokenized payload or encoding it at fixed width; if iteration remains, detect a two-value cycle and return a result using the larger oscillating count, preserving correct behavior for pre-budget cappedResult.tests/unit/review-express-regressions.test.ts-44-55 (1)
44-55: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
.sizeinstead oftoHaveLengthon theseSetchecks.
Setdoesn’t have alengthproperty, so these uniqueness assertions should read.sizedirectly.🐛 Proposed fix
expect(middleware).toHaveLength(2) - expect(new Set(middleware.map(([id]) => id))).toHaveLength(2) + expect(new Set(middleware.map(([id]) => id)).size).toBe(2) @@ expect(routeEdges).toHaveLength(2) - expect(new Set(routeEdges.map((edge) => edge.toId))).toHaveLength(2) + expect(new Set(routeEdges.map((edge) => edge.toId)).size).toBe(2)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/review-express-regressions.test.ts` around lines 44 - 55, Update the uniqueness assertions in the test block to compare each Set’s `.size` with the expected count instead of using `toHaveLength`. Apply this to the Sets created from `middleware` IDs and `routeEdges` destination IDs, while leaving the array length assertions unchanged.src/shared/discovery-safety.ts-286-287 (1)
286-287: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDropping the artifact size guard makes this read unbounded.
The previous implementation capped the graph artifact bytes before parsing;
loadGraphArtifactnow reads and fully parses the artifact just to pullgraph.discovery_safety. On a very large (or untrusted)graph.jsonthis becomes an unbounded memory/CPU cost on every cache miss, and the cache miss path is triggered by any touch of the file. Consider keeping a byte ceiling, or reusing an artifact already loaded by the caller.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/discovery-safety.ts` around lines 286 - 287, Restore a bounded read in the discovery metadata cache-miss path instead of calling loadGraphArtifact, which fully loads and parses the artifact. Update the logic around parseDiscoverySafetyMetadata and discoveryMetadataCache.set to extract graph.discovery_safety from a size-limited artifact read, or reuse an already loaded artifact when available, while preserving cache behavior.src/domain/query/rank.ts-585-590 (1)
585-590: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
fields[0]is not reliably the label field.
buildCorpus(Lines 168-181) drops any field whose value produces no tokens, so when a node has nolabelthe first element becomesqualified_name(orframework/metadata/node_kind).labelMatches,exactLabelMatches, andlabelPrecisionthen silently score against the wrong field. Positional lookup by weight won't help either, sincelabelandqualifiedNameshare weight12.🐛 Proposed fix: carry the label tokens explicitly on the corpus node
interface RankCorpusNode { id: string; documentKey: string; attributes: GraphAttributes; fields: readonly RankField[] + labelTokens: ReadonlySet<string> tokens: ReadonlySet<string>; pathTokens: ReadonlySet<string>; sourceFile: string- const labelTokens = candidate.node.fields[0]?.tokens ?? new Set<string>() + const labelTokens = candidate.node.labelTokensand in
buildCorpus, setlabelTokens: new Set(lexicalTokens(stringAttribute(attributes, 'label'))).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/query/rank.ts` around lines 585 - 590, Stop deriving labelTokens from candidate.node.fields[0], since fields are omitted when empty and their order is not reliable. Extend the corpus node produced by buildCorpus with an explicit labelTokens field populated from the label attribute via stringAttribute and lexicalTokens, then update the ranking logic around labelMatches, exactLabelMatches, and labelPrecision to use that field while preserving the empty-set fallback behavior.tests/unit/canonical-index-framework-regressions.test.ts-121-121 (1)
121-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
.sizefor theSetcardinality check
toHaveLengthonly reads.length, so this assertion won’t check the 16 distinct storage operations. Use.sizeinstead:
expect(new Set(operations.map((node) => node.storage_operation)).size).toBe(16)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/canonical-index-framework-regressions.test.ts` at line 121, Update the distinct storage-operation assertion in the canonical index regression test to read the Set’s size and compare it with 16, rather than using toHaveLength on the Set.src/adapters/typescript/framework-express.ts-357-364 (1)
357-364: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
express.Router()via a default import is not detected.The property-access branch only consults
namespaceAlias, so the most common Express router idiom —import express from 'express'; const r = express.Router()— never yields anexpress_routerrole, and every route registered onris dropped.🐛 Proposed fix: also accept members on the default-import binding
if (ts.isPropertyAccessExpression(callee) && ts.isIdentifier(callee.expression) && ts.isIdentifier(callee.name)) { const namespaceName = callee.expression.text const memberName = callee.name.text - if (bindings.namespaceAlias.has(namespaceName)) { + if (bindings.namespaceAlias.has(namespaceName) || bindings.appFactory.has(namespaceName)) { if (memberName === 'Router') return 'express_router' if (memberName === 'default') return 'express_app' } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/typescript/framework-express.ts` around lines 357 - 364, Update the property-access handling around the namespaceAlias check to also recognize when callee.expression matches the default-import binding for Express. Return express_router for the Router member and preserve express_app handling for default, ensuring express.Router() through a default import is classified correctly.src/adapters/typescript/framework-express.ts-149-163 (1)
149-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMount prefix can be applied more than once to the same handler symbol.
The loop rewrites
handler.framework_metadata.route_pathper matching edge, but a single handler symbol can be the target of multipleroute_handleredges from the same router (e.g.router.get('/a', handler)androuter.post('/a', handler)) or from several mounted routers. Each pass re-prefixes the already-prefixed value, producing paths like/api/api/a.🐛 Proposed fix: prefix each handler at most once
+ const prefixed = new Set<string>() for (const edge of opts.edges) { if (edge.kind !== 'route_handler') continue const router = routerById.get(edge.from) if (!router) continue const mountPath = router.framework_metadata?.mount_path if (typeof mountPath !== 'string' || mountPath.length === 0) continue const handler = routeHandlerById.get(edge.to) if (!handler) continue + if (prefixed.has(handler.id)) continue const existingPath = handler.framework_metadata?.route_path if (typeof existingPath !== 'string') continue + prefixed.add(handler.id) handler.framework_metadata = { ...(handler.framework_metadata ?? {}), route_path: joinRoutePath(mountPath, existingPath), } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/typescript/framework-express.ts` around lines 149 - 163, Update the route-handler edge loop so each handler symbol’s route path is prefixed at most once, even when multiple edges target it or multiple routers mount it. Use the original unprefixed route path for each handler, track processed handlers or otherwise guard against repeated updates, and preserve the existing mount-path validation and joinRoutePath behavior.src/infrastructure/watch-index.ts-26-35 (1)
26-35: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winWatcher ignore filter doesn't match nested
node_modules/.git/outdirectories.The regex
/^(?:out|node_modules|\.git)(?:\/|$)/is anchored to the start of the path, so it only filters events at the watched root. With recursivefs.watch,filenameis reported relative to the watched root, so any change inside a nestednode_modules(e.g.packages/app/node_modules/..., a layout this project explicitly supports per the canonical-index monorepo fixtures) is not filtered and triggerschanged(), causing debounced full reconciliation storms during ordinary operations likenpm installin a sub-package.🐛 Proposed fix: match nested occurrences too
- const watcher = watchFilesystem(root, { recursive: true }, (_event, filename) => { - if (/^(?:out|node_modules|\.git)(?:\/|$)/.test(filename?.toString().replaceAll('\\', '/') ?? '')) return + const watcher = watchFilesystem(root, { recursive: true }, (_event, filename) => { + if (/(?:^|\/)(?:out|node_modules|\.git)(?:\/|$)/.test(filename?.toString().replaceAll('\\', '/') ?? '')) return changed() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/infrastructure/watch-index.ts` around lines 26 - 35, Update the ignore filter in defaultEventSource so out, node_modules, and .git directories are ignored wherever they occur in the recursive filename path, including nested package directories. Normalize path separators as currently done and preserve changed() for non-ignored events.
1 participant
Outcome
Prepare the first Core Reset beta,
@lubab/madar@0.40.0-beta.1, for the npmnextchannel without touchingmain.Closes the release-preparation portion of #577. RFC #577 and the Core Reset milestone remain open until npm, the GitHub prerelease, MCP Registry metadata, and clean-install verification all complete.
Exact base and scope
core-resetanchor:5c9d1e2436932f7420169ea4ffa617c6bea4fbd035b68cd733e07b9306d52e2076a287269f8919eb691b83abce9a1fc47c13f8ff9bc06741b5470c2ac587bdbe811034e9873d2d81951b64aa1b9e5d04nextsrc/**changes: noneThis candidate updates only governance truth, version/lock metadata, changelog and migration documentation, SBOM and MCP Registry metadata, release workflows, package-receipt verification, and their tests.
Capability Validation #610, #612, #614, #615, and #616 is cancelled and closed not planned. No campaign ran, no provider request or paid spend occurred, and no Native/Graphify/Madar result exists. This beta makes no comparative correctness, token, latency, cost, activation, retention, or external-user claim.
Release safety
0.40.0-beta.1nextonlyv0.40.0-beta.1next-tip recheck, OIDC provenance, immutable registry verification, and npm-before-GitHub ordering; no token or no-provenance fallback0.40.0, npmlatest, andmain: not authorizedThe current
next-only history commit has the same tree as the shared merge base, so this PR has no content conflict with the Core Reset line.Local verification
npm ci: passed; 0 vulnerabilitiesgit diff --check: passedFresh package receipt:
5811e58e02799d103ad12b7220c31e6a7244043c390ba73ae215caea37a3403d7963f709997ac86d462e8e3dedfed8a710c7fa40Merge gate
Do not merge until all six exact-head CI jobs pass, independent exact-head review finds no blocker, and zero review threads remain. CodeRabbit runs on a non-default base here; its actual result or skip must be recorded honestly.
Summary by CodeRabbit
New Features
retrievetool for evidence, relationships, and clear result boundaries.Documentation
Bug Fixes