Skip to content

feat(local): document and expose BYO brain routing - #56

Draft
jeremylongshore wants to merge 2 commits into
mainfrom
feat/iel-25a53-byo-brain-config
Draft

feat(local): document and expose BYO brain routing#56
jeremylongshore wants to merge 2 commits into
mainfrom
feat/iel-25a53-byo-brain-config

Conversation

@jeremylongshore

Copy link
Copy Markdown
Owner

Summary

  • document the fork/local BYO-brain configuration contract and derived qmd tenant path
  • expose mode, tenant, brain root, export tree, and qmd index in brain_status
  • align installer/preflight output with the routing contract
  • add config unit coverage and built-runtime smoke assertions
  • rebuild the shipped runtime bundle against the matching Registrar provenance surface

Tracking

Validation

  • pnpm test — 83 passed
  • pnpm typecheck:ci
  • pnpm lint
  • node smoke/smoke.mjs
  • node --test smoke/mode-dispatch.test.mjs
  • node --test scripts/verify-anchors.test.mjs
  • node --check bin/init.mjs
  • git diff --check

Local mode remains in-process and zero-egress; the status receipt and smoke test verify tenant-scoped routing.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

MiniMax Review

Review

Findings

1. Bundle regeneration not in the diff (rebundle contract)

src/config.ts and src/local-server.ts are modified in this PR — both feed the vendored runtime. plugin-runtime/governed-brain.cjs is not in the diff. Per the rebundle contract, a src/ change must be accompanied by a regenerated bundle. Please either include the rebuilt plugin-runtime/governed-brain.cjs or flag in the description that npm run build (and the verify step) ran clean in CI.

2. qmdIndexPath derivation duplicated across two layers (low)

bin/init.mjs adds qmdIndexPath(args) = join(base, 'qmd-index', tenant), while src/config.ts adds resolveQmdIndexPath(basePath, tenantId) with the same formula. Two sources of truth for the contract path makes it easy for one to drift (e.g., if someone later moves qmd under a different directory name). Consider importing resolveQmdIndexPath from config.ts so the installer and the runtime can't disagree. Not a security issue; just a contract-drift risk the docs were careful to call out.

3. Docs assume a resolveMode behavior worth confirming

The new README + onboarding text instructs users to "Leave TEAMKB_API_URL unset (or empty)". The smoke now asserts st.mode === 'local' regardless, which gives some confidence — but worth a moment to confirm resolveMode(TEAMKB_API_URL) treats "" (empty string) the same as unset. If it currently tests !== undefined rather than truthiness, TEAMKB_API_URL="" would silently flip the fork onto the team dispatch — exactly the local-mode-DEFAULT violation the policy is meant to catch. Smoke appears to cover this; just confirm it sets the variable to "" (not unset) at least once.

What looks fine

  • Secrets/credentials: no hardcoded URLs, tokens, hosts, IPs, or paths into a tailnet. All new env knobs (TEAMKB_BASE_PATH, TEAMKB_TENANT_ID, TEAMKB_EXPORT_DIR) are per-user paths under $HOME, no network involved. The README/AGENTS explicitly say "no API key or network is needed for local grounding," which the local dispatch supports.
  • Mode-boundary integrity: no new write tools, no new admin surfaces, no changes to resolveMode or remote-server.ts. The new localConfigReceipt() is only consumed in local-server.ts, including the NATIVE_DEP_HINT error branch, which is an improvement for diagnosing misconfigured forks.
  • Govern invariant: no new write tools; brain_status remains read-only and doesn't mutate state outside capture→govern.
  • Honesty invariant: no tamper-proof / immutable / non-repudiation / blockchain / unqualified append-only wording introduced. Tool description rewording ("counts by lifecycle/category plus tenant and qmd paths. Read-only.") is accurate. The audit-claim honesty framing from the prior block is preserved.
  • Tests: src/config.test.ts covers derivation, full-env resolution, and the whitespace-blank → local/kb-export defaults. Mocking @qmd-team-intent-kb/common is correctly scoped to the unit suite per the comment.
  • Smoke: assertions on mode/tenantId/basePath/exportDir/qmdIndexPath are exactly the contract the docs advertise, and reinforce the local-mode default rather than relaxing it.

LGTM once the bundle is regenerated and the empty-string resolveMode behavior is confirmed.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

MiniMax Adversarial Review

Adversarial audit — feat(local): document and expose BYO brain routing

Below are claims that fail the evidence ladder. Documentation/comment edits that line up with code are not separately called out; no version drift was visible in the diff (no package.json / CHANGELOG / plugin manifest included, so version-skew cannot be ruled out from this PR alone).


1. "rebuild the shipped runtime bundle against the matching Registrar provenance surface"

  • Claim (quoted): Summary bullet — "rebuild the shipped runtime bundle against the matching Registrar provenance surface."
  • What the diff evidences: Source-level edits to src/config.ts, src/local-server.ts, bin/init.mjs, smoke/smoke.mjs, src/config.test.ts (added), and Markdown. No bundle artifact, no checksum, no commit hash, no CI artifact link to a re-built bundle.js (or equivalent) is present in the diff.
  • Gap: Exactly the pattern the audit policy calls out: "a rebuilt bundle claimed but not shown is not a verified bundle." "Matching Registrar provenance surface" is also undefined — there is no documented surface (e.g., expected XDG root, expected socket path, expected protocol version) that the rebuild is checked against.
  • Smallest honest rewording / missing evidence:
    • Either drop the bullet, or replace it with a concrete artifact: e.g. "Rebuilt bundle at dist/<file> (sha256: …) built from this commit; the boot receipt and CI run linked below show qmd path matches resolveQmdIndexPath(base, tenant) = …."
  • Confidence: High — claim is unsubstantiated as written.

2. "pnpm test — 83 passed"

  • Claim (quoted): Validation section.
  • What the diff evidences: A new src/config.test.ts is added with three it(...) cases. No test logs, no Jest/Vitest reporter output. The prior count is not visible.
  • Gap: 83 is a precise number, but the diff alone doesn't establish that number; the new file alone cannot account for the count. Per policy, complete / verified style assertions require recorded evidence, not assertion.
  • Smallest honest rewording / missing evidence: Attach the test runner output (e.g. paste the Tests 83 passed (…ms) summary line) and note the prior baseline (e.g. "83 passed (80 pre-existing + 3 new in src/config.test.ts)").
  • Confidence: Medium — the count may be accurate, but it's unrecorded.

3. "Local mode remains in-process and zero-egress; the status receipt and smoke test verify tenant-scoped routing."

  • Claim (quoted): Final paragraph of the PR body.
  • What the diff evidences: smoke/smoke.mjs gains five new ok(...) assertions on brain_status fields (mode, tenantId, basePath, exportDir, qmdIndexPath). local-server.ts adds localConfigReceipt() and threads it into both branches of brain_status (incl. the missing-native-dep branch). No smoke output is attached, and "verify" is one of the audit-policy red-flag words.
  • Gap: The new smoke assertions are an intent to verify, not evidence that they passed; zero-egress is a static property of the local path and was already true before this PR (this PR doesn't change transport).
  • Smallest honest rewording / missing evidence: Rewrite without "verify": "Local mode stays in-process and zero-egress; the new smoke assertions exercise the tenant-scoped fields — attached log shows them passing."
  • Confidence: Medium — assertions exist; outcome is not shown.

4. Validation command list (pnpm typecheck:ci, pnpm lint, node smoke/smoke.mjs, node --test smoke/mode-dispatch.test.mjs, node --test scripts/verify-anchors.test.mjs, node --check bin/init.mjs, git diff --check)

  • Claim (quoted): Validation section, all of the above.
  • What the diff evidences: Only git diff --check is genuinely self-evidencing (no whitespace damage in the shown hunks); the rest are invocations with no outputs. smoke/mode-dispatch.test.mjs and scripts/verify-anchors.test.mjs are referenced but neither file appears in the diff — they may be pre-existing, but they cannot be located in the changes either.
  • Gap: Commands listed but no pass/fail result is attached. For two of them the cited file is not in the diff at all.
  • Smallest honest rewording / missing evidence: Either paste the trailing summary line of each tool (✓ … problems (0 errors, 0 warnings); 1..1 ok / tests X; code: 'ERR_MODULE_NOT_FOUND' → fail) or collapse the list to a single line that points to a CI check run URL.
  • Confidence: Medium for the unspecified ones; High that the listing is insufficient as evidence.

5. src/config.test.ts — whitespace-only TEAMKB_TENANT_ID expects tenantId: 'local'

  • Claim (quoted): Test "keeps the safe local defaults when tenant and export overrides are blank" asserts tenantId: 'local' after vi.stubEnv('TEAMKB_TENANT_ID', ' ') (and the same trim-then-default for TEAMKB_EXPORT_DIR).
  • What the diff evidences: src/config.ts diff only adds qmdIndexPath: resolveQmdIndexPath(basePath, tenantId). The pre-existing body of resolveConfig (which decides what tenantId and exportDir resolve to for whitespace input) is not shown in the diff. The mocked getTeamKbBasePath() does .trim(), but that helper only governs basePath.
  • Gap: If the existing resolveConfig is envTenant && envTenant.length > 0 ? envTenant : 'local' (no trim), ' ' is non-empty and would land in the receipt as ' ', not 'local'. Either the test is wrong, or resolveConfig trims in a way that's not in this diff. Whichever is true, the audit policy says the diff must evidence its own claims — and this contract is currently unverifiable from the patch.
  • Smallest honest rewording / missing evidence: Show the full resolveConfig (or at minimum the tenantId/exportDir resolution lines) so a reviewer can confirm trim-or-treat-as-empty applies to whitespace. Otherwise drop the whitespace case from the test or document why it passes.
  • Confidence: Medium — could resolve either way; flagging because the PR purports to ship this contract test green and the contract's trim behavior is not visible.

Items checked and not flagged

  • Audit-honesty framing in AGENTS.md: "tamper-evidentnot tamper-proof; a local writer can edit an event and re-hash forward" — preserved and reasserted. No new "immutable / non-repudiation / blockchain" over-claims introduced.
  • Privacy / secret over-share in a public PR: Diff exposes no token, tailnet host/IP, internal hostname, real API URL, or operator-specific path; sample paths are /tmp/fork-brain / $HOME/.teamkb-my-fork (illustrative). TEAMKB_API_URL="" is a recommended unset, not a value.
  • Routing-contract internal consistency: bin/init.mjs::qmdIndexPath(args) = join(base, 'qmd-index', tenant) matches src/config.ts::resolveQmdIndexPath(basePath, tenantId), and the new AGENTS.md / README / onboarding docs state the same <base>/qmd-index/<tenant> derivation. brain_status now exposes exactly the five fields the docs claim (mode, tenantId, basePath, exportDir, qmdIndexPath).
  • Diff does what the body says (modulo the bundle claim): documentation, config.ts derivation, local-server.ts brain_status enrichment, bin/init.mjs preflight+receipt, config unit test, smoke assertions — all present and aligned.
  • Anti-ratchet: This is the first review of this PR; no prior claims to re-rate.

TL;DR

The substantive code/docs changes look coherent. Five claims need evidence attached or rewording: the bundle rebuild (no artifact), the pnpm test — 83 passed count (no log), "verify tenant-scoped routing" (use a softer verb; attach smoke output), the tail of validation commands (paste results; locate mode-dispatch/verify-anchors test files), and the whitespace-default test in src/config.test.ts (show the matching trim logic in resolveConfig or shrink the test).

Copy link
Copy Markdown
Owner Author

Local receipt for iel-25a.5.3: commit 5e3adf8 on feat/iel-25a53-byo-brain-config. pnpm test passed (83 tests), pnpm typecheck:ci, pnpm lint, node smoke/smoke.mjs (including mode/tenant/base/export/qmd routing assertions), node --test smoke/mode-dispatch.test.mjs, node --test scripts/verify-anchors.test.mjs, node --check bin/init.mjs, and git diff --check all pass. The branch is based on origin/main 8684fbf; bead remains in progress pending PR merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document and harden the local BYO-brain configuration surface

1 participant