Skip to content

feat: conductor foundations — session resolution (P0–P1) + durable owner-delegated identity (P2) - #51

Merged
saucam merged 13 commits into
mainfrom
feat/conductor
Jul 6, 2026
Merged

feat: conductor foundations — session resolution (P0–P1) + durable owner-delegated identity (P2)#51
saucam merged 13 commits into
mainfrom
feat/conductor

Conversation

@saucam

@saucam saucam commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

What

First three phases of the conductor build plan (docs/conductor-build-plan.md) — spec docs plus P0–P2 implementation. P3 (conductor session + read-only fleet tools) builds directly on P1 + P2, so this is the natural merge point.

P0 — Session-card store + eval harness

  • Session cards in the memory engine (memory/cards.ts) with FTS + vector indexing.
  • Offline eval runner (eval/baseline.ts + fixtures) with measured baselines in eval/BASELINE.md; eval/metrics.ts (P@1 / MRR / recall@k / percentiles) is unit-tested.

P1 — Cross-workspace session resolution

  • Global fusion across workspaces (RRF) + cross-encoder rerank (memory/reranker.ts, transformers.js backend) to clear the resolution gate — see docs/session-resolution.md for how a query like "the authz fix" lands on the right session.

P2 — Conductor identity foundation (design R1/R2)

  • New protocol scopes session:read / session:dispatch; CONDUCTOR_SCOPES deliberately omits tools:write/tools:execute — ZeroID grants the intersection of the subject's scopes on every delegation hop, so the conductor's whole subtree is read-only on targets by construction.
  • registerConductor(ownerSub) registers a ZeroID orchestrator identity and persists {identityId, wimseUri, apiKey} to the Store; resumeSessions() reloads it on restart — one stable WIMSE URI across process lifetimes. The actor keypair is regenerated per boot and re-registered (never at rest); the api_key is the single credential on disk.
  • mintConductorToken(ownerToken) — RFC 8693 owner→conductor exchange; deactivateConductor() cascade-revokes the delegation subtree server-side.

Testing

  • 1006 unit tests green, including new fetch-stubbed ZeroID tests for the full conductor lifecycle (register / resume / stale-row drop / key rotation / mint / deactivate) and mocked-transformers reranker tests (no model downloads in CI).
  • bun run test:integration (new script, excluded from CI globs) run against a live ZeroID: owner → conductor → child → sub-agent mints at delegation_depth 3 with a verified act chain per hop; tools:write cannot be minted below the conductor; deactivating the conductor kills conductor/child/sub-agent tokens while the owner's token stays active. All test identities use codeoid-conductor-test-* external_ids and are deactivated afterward.
  • bun run typecheck + bun run lint clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added a durable conductor identity with session read/dispatch capabilities and support for resuming across restarts.
    • Upgraded natural-language session resolution with cross-workspace retrieval plus optional cross-encoder reranking.
    • Introduced session “cards” with full-text search and bi-temporal fact tracking.
    • Added an evaluation baseline runner for session-resolution quality.
  • Bug Fixes
    • Improved startup behavior by restoring conductor state before rebuilding session data.
  • Documentation
    • Added conductor build plan, design, and prior-art comparison docs.
  • Tests
    • Added integration and unit tests covering conductor persistence, token/scope behavior, retrieval, reranking, and evaluation metrics.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e80555a-e2a9-4c35-a38e-ec65531d55b1

📥 Commits

Reviewing files that changed from the base of the PR and between 692e6f2 and f719e74.

📒 Files selected for processing (7)
  • docs/conductor-design.md
  • src/daemon/agent-identity.ts
  • src/daemon/memory/cards.test.ts
  • src/daemon/memory/cards.ts
  • src/daemon/memory/engine.ts
  • src/daemon/memory/reranker-transformersjs.ts
  • src/tests/agent-identity-conductor.test.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/conductor-design.md
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/daemon/memory/reranker-transformersjs.ts
  • src/tests/agent-identity-conductor.test.ts
  • src/daemon/memory/engine.ts
  • src/daemon/agent-identity.ts
  • src/daemon/memory/cards.ts

📝 Walkthrough

Walkthrough

This PR adds conductor planning docs, expands shared scopes for conductor sessions, implements durable conductor identity persistence and delegation, extends memory retrieval across workspaces with reranking, adds session-card/bi-temporal storage, and introduces evaluation tooling for session resolution.

Changes

Conductor documentation

Layer / File(s) Summary
Design and prior-art docs
docs/conductor-design.md, docs/conductor-build-plan.md, docs/conductor-prior-art-firstmate.md, docs/conductor-prior-art-hermes.md, docs/conductor-session-resolution.md, docs/session-resolution.md
Documents the conductor architecture, phased implementation plan, prior-art comparisons, and session-resolution design/evaluation results.
Implementation and evaluation docs
src/daemon/eval/BASELINE.md
Records the baseline evaluation methodology, results, and go/no-go notes.

Conductor identity and retrieval implementation

Layer / File(s) Summary
Scopes and conductor identity
packages/protocol/src/scopes.ts, src/tests/scopes.test.ts, src/daemon/agent-identity.ts, src/daemon/store.ts, src/daemon/session-manager.ts, src/tests/agent-identity-conductor.test.ts, src/integration/conductor-zeroid.test.ts
Adds conductor-only scopes, durable conductor identity lifecycle methods, persisted conductor state, startup resume wiring, and unit/integration coverage for registration, delegation, and revocation.
Global recall and reranking
src/daemon/memory/reranker.ts, src/daemon/memory/reranker-transformersjs.ts, src/daemon/memory/engine.ts, src/daemon/memory/store.ts, src/daemon/memory/engine.test.ts, src/tests/reranker.test.ts
Adds pluggable cross-encoder reranking, global cross-workspace retrieval primitives, and tests covering the new retrieval flow.
Session cards and bi-temporal facts
src/daemon/memory/cards.ts, src/daemon/memory/cards.test.ts
Adds persistent session cards, FTS-backed lookup, and bi-temporal fact queries.
Evaluation harness
src/daemon/eval/metrics.ts, src/daemon/eval/baseline.ts, src/daemon/eval/metrics.test.ts, src/daemon/eval/fixtures/session-resolution.json, package.json
Adds evaluation metrics, a baseline runner, fixtures, and an integration test script.

Estimated code review effort: 4 (Complex) | ~75 minutes

Possibly related issues

Possibly related PRs

  • saucam/codeoid#101: Modifies the shared scopes source of truth that this PR extends with session:read and session:dispatch.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: session resolution work for P0–P1 and durable owner-delegated identity for P2.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/conductor

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.69231% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.47%. Comparing base (57327a0) to head (f719e74).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/daemon/eval/metrics.ts 91.07% 5 Missing ⚠️
src/daemon/memory/engine.ts 96.11% 4 Missing ⚠️
src/daemon/memory/cards.ts 98.57% 3 Missing ⚠️
src/daemon/memory/reranker-transformersjs.ts 94.23% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
- Coverage   77.76%   77.47%   -0.29%     
==========================================
  Files          79       85       +6     
  Lines       12438    13396     +958     
==========================================
+ Hits         9672    10379     +707     
- Misses       2766     3017     +251     
Flag Coverage Δ
daemon 77.47% <97.69%> (-0.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/protocol/src/scopes.ts 100.00% <100.00%> (ø)
src/daemon/agent-identity.ts 45.62% <100.00%> (ø)
src/daemon/memory/reranker.ts 100.00% <100.00%> (ø)
src/daemon/memory/store.ts 91.98% <100.00%> (+0.43%) ⬆️
src/daemon/session-manager.ts 48.46% <100.00%> (+0.26%) ⬆️
src/daemon/store.ts 86.72% <100.00%> (+1.50%) ⬆️
src/daemon/memory/cards.ts 98.57% <98.57%> (ø)
src/daemon/memory/reranker-transformersjs.ts 94.23% <94.23%> (ø)
src/daemon/memory/engine.ts 98.37% <96.11%> (-1.16%) ⬇️
src/daemon/eval/metrics.ts 91.07% <91.07%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

saucam and others added 11 commits July 6, 2026 10:54
…d plan

Spec set for the codeoid conductor: a single global, identity-native supervisor session that resolves fuzzy natural-language references to the right coding session across all workspaces, coordinates the existing session fleet, and never goes out of context.

- conductor-design.md: architecture + locked decisions (owner-delegated privileged identity, durable conductor / disposable children, confirm before send-class acts, approval-gated egress, metrics-only cost guard)
- conductor-session-resolution.md: SOTA retrieval architecture (BGE-M3 hybrid + bge-reranker-v2-m3 + LFM2-350M-Extract cards + bi-temporal state), backed by a 9-agent research fan-out
- conductor-build-plan.md: phased plan P0-P8

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First P0 slice of the conductor (see docs/conductor-build-plan.md).

- SessionCardStore (src/daemon/memory/cards.ts): per-session digest cards with a standalone FTS5 mirror for keyword/identifier recall, plus a bi-temporal fact log (Zep/Graphiti pattern) — state changes are invalidated-not-deleted (valid_at/invalid_at event time + created_at/expired_at system time), giving time-travel + a lossless audit trail.
- Eval harness metrics (src/daemon/eval/metrics.ts): precision@1 / MRR / recall@k + latency percentiles for known-item session resolution — the go/no-go gate for the whole feature.

13/13 unit tests pass; tsc + biome clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bile app

- conductor-prior-art-firstmate.md: analysis of firstmate (a shipped bash+prompt conductor, codeoid's architectural inverse). Borrows ranked: read-only-by-construction, zero-token event-driven supervision, ship/scout task shapes, per-project autonomy modes, /afk + /stow, harness dispatch profiles, secondmates (nested-conductor scaling). Where codeoid is already better: cryptographic identity, semantic session resolution, determinism (code vs 122KB prompt), daemon-native events.
- build-plan: added 'Informed by firstmate' refinements and 'Reconciliation with the mobile app plan'. Key reconciliation: the conductor needs ZERO new client wire types (mobile doc §8), so the P3 protocol-level fleet.find is downgraded to optional; shared @codeoid/core extraction up front; sequencing = conductor backend (P1 session resolution) first as the risk-retiring gate, mobile app in parallel on today's protocol, converging at mobile-P5.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… add P4.5 routines

- conductor-prior-art-hermes.md: analysis of NousResearch/hermes-agent, the most complete personal-assistant prior art (multi-platform gateway, cron routines, autonomous skills, delegate + Kanban). Borrows: routines (cron+webhooks+script-injection [SILENT]), durable Kanban work-queue, leaf/orchestrator delegate role model, session-lifecycle hardening, multi-platform gateway shape, zero-context-cost tool-RPC scripts, Curator safe-autonomy invariants, ACP + serverless-persistence notes. Where codeoid stays ahead: cryptographic identity, rerank+bi-temporal retrieval, typed modular daemon (hermes is a Python monolith).
- build-plan: upgraded P4 dispatch (durable Kanban-style queue + leaf/orchestrator roles enforced via ZeroID scopes + session-lifecycle hardening: resume_pending/stuck-loop/clean-shutdown/burst-collapse queue) and added P4.5 Routines (scheduled + webhook-triggered autonomy, [SILENT] monitors, cron hardening). Updated phase table, dependency graph, and added the 'Informed by hermes' section.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed number

Wires a baseline resolver over the current workspace-scoped searchSessions and measures it against the real Hetzner corpus (16 sessions / 11 workspaces / 11,938 episodes) with 37 hand-labeled fuzzy references.

Result: within-workspace P@1 = 89.2% (primitives sound), cross-workspace P@1 = 21.6% with R@5 = 81% — a ranking/fusion problem, not recall: searchSessions normalizes BM25 batch-relative PER workspace, so a small workspace's inflated scores dominate the naive cross-workspace merge (~22 misses return the same wrong small-workspace session at #1). 21.6% is the number P1 must beat; global normalized fusion + cross-encoder rerank should convert the 81% R@5 into P@1. See src/daemon/eval/BASELINE.md. memory.db is NOT committed (real session content); only the derived fuzzy-reference fixture.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… 21.6% -> 35.1%)

feat/conductor was 22 commits behind main; rebased onto origin/main (conflict-free — P0 adds only new files). Re-ran the baseline on the current base: cross-workspace P@1 rose 21.6% -> 35.1% (MRR 0.45 -> 0.54) because main's #94 (append-to-vector-cache) improves vector coverage. Within-workspace unchanged at 89.2%; R@5 still 81%; same small-workspace-domination failure mode. 35.1% is the refreshed P1 target.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The conductor resolves a fuzzy reference across ALL workspaces, which the current workspace-scoped searchSessions can't do. Adds engine.recallGlobal(): unions FTS + vector candidates across every workspace and ranks in ONE batch, so the ranker's BM25 min-max normalization is GLOBAL (fixes the small-workspace-domination failure a naive per-workspace merge has). searchSessions() goes global when no workspaceId is passed. New store primitives: listWorkspaceIds, ftsSearchGlobal, episodesByIds.

Measured on the real Hetzner corpus (37 labeled refs), vs the naive-merge baseline:
- latency 4224 -> 24ms p95 (~200x: one search vs 11 per-workspace)
- R@5 81% -> 92%, R@3 76% -> 81%, MRR 0.54 -> 0.61
- P@1 35.1% -> 37.8% (modest; remaining misses are rank 2-3 as big verbose sessions fill #1 — slice 2 cross-encoder rerank targets exactly this)

4 new deterministic cross-workspace tests (17 total green); tsc + biome clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a Reranker interface + transformers.js cross-encoder impl (Xenova/ms-marco-MiniLM-L-6-v2, swappable for bge-reranker-v2-m3). MemoryEngine reranks the top-8 candidate sessions by (query, evidence) when a reranker is present; searchSessions({rerank}) gates it (defaults on when ready) and degrades to fusion-only if the model fails to load.

Measured on the real Hetzner corpus (37 refs): cross-workspace P@1 37.8% -> 86.5% (MRR 0.61 -> 0.91, R@3 81% -> 95%), latency +~30ms (88ms p95). Converts slice 1's 92% R@5 into precision@1 — essentially the within-workspace ceiling (97.3% with rerank on).

P1 go/no-go gate CLEARED: cross-workspace P@1 35.1% (baseline) -> 86.5%, p95 < 100ms vs the 2s budget. 18 tests green (added a deterministic rerank test); tsc + biome clean. Remaining P1 slices (BGE-M3, identifier-aware lexical, session cards) are now optional polish. See src/daemon/eval/BASELINE.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First-class explainer of the shipped cross-workspace session-resolution capability (two-stage: global fusion -> cross-encoder rerank). Distinct from the design/plan doc (conductor-session-resolution.md) and the eval writeup (BASELINE.md): covers the problem, the pipeline, why two stages, measured results (P@1 35.1% -> 86.5%), the local models, a code map, repro, and limitations.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
codeoid is public; the eval fixture named internal project specifics. Genericized all 37 reference strings to neutral software-work descriptions (gold labels are opaque UUIDs — unchanged) and scrubbed an internal name from BASELINE.md.

Re-ran: pure-conceptual references (no exact identifiers) are a harder, conservative eval — cross-workspace P@1 21.6% (naive) -> 35.1% (fusion) -> 73.0% (rerank), R@5 92%, <100ms. Two-stage story unchanged; identifier-bearing references resolve higher still. Refreshed numbers in BASELINE.md, docs/session-resolution.md, and the build plan.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tity

Conductor scope profile (R1): session:read / session:dispatch join the
protocol scopes; CONDUCTOR_SCOPES deliberately omits tools:write and
tools:execute, so ZeroID's per-hop scope intersection makes the conductor's
whole delegation subtree read-only-by-construction on targets.

Durable identity (R2): registerConductor(ownerSub) registers a ZeroID
orchestrator identity and persists {identityId, wimseUri, apiKey} to the
Store; resumeSessions reloads it on daemon restart — one stable WIMSE URI
across process lifetimes, with the actor keypair regenerated (never at
rest) and re-registered per boot. mintConductorToken exchanges the owner's
subject token for the conductor's working token (RFC 8693), and
deactivateConductor cascade-revokes the subtree via ZeroID's parent_jti
walk.

Integration test (bun run test:integration, live ZeroID required): mints
the owner → conductor → child → sub-agent chain at delegation_depth 3 with
a verified act chain per hop, proves tools:write can't be minted below the
conductor, and asserts deactivation kills conductor/child/sub-agent tokens
while the owner's token stays active.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fetch-stubbed ZeroID covers register/resume/mint/deactivate — including the
stale-row drop, key rotation on resume, and the actor-assertion wire
contract — plus Store persistence per tenant. The cross-encoder reranker is
covered with @xenova/transformers mocked (no model download): batching
shape, single-logit vs two-class score extraction, and re-init after close.

The live-ZeroID integration test remains the depth-3 / cascade-revocation
proof; these keep the CI patch-coverage gate honest without a server.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@saucam saucam changed the title Conductor: identity-native fleet supervisor (spec + P0 foundations) feat: conductor foundations — session resolution (P0–P1) + durable owner-delegated identity (P2) Jul 6, 2026
@saucam
saucam marked this pull request as ready for review July 6, 2026 03:33

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (5)
src/daemon/memory/engine.ts (2)

406-406: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify the rerank-gating expression.

(opts.rerank ?? this.#rerankerReady) && this.#rerankerReady double-references #rerankerReady; it's equivalent to the clearer this.#rerankerReady && opts.rerank !== false.

♻️ Simplify
-    const doRerank = (opts.rerank ?? this.#rerankerReady) && this.#rerankerReady;
+    const doRerank = this.#rerankerReady && opts.rerank !== false;
🤖 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/daemon/memory/engine.ts` at line 406, The rerank gating in the engine
logic is overly duplicated and should be simplified. Update the `doRerank`
assignment in `MemoryEngine` to use the clearer `this.#rerankerReady &&
opts.rerank !== false` form instead of rechecking `#rerankerReady` twice. Keep
the behavior the same while making the condition easier to read and maintain.

211-271: 🚀 Performance & Scalability | 🔵 Trivial

Cross-workspace recall scans every workspace's full vector matrix on every query.

recallGlobal iterates listWorkspaceIds() and brute-force cosines against every embedded episode across ALL workspaces per query — an intentional fix for the batch-relative BM25 normalization issue described in the docstring, but it means per-query cost now scales with total corpus size across the whole machine rather than one workspace. As workspace count/episode volume grows this could become a real latency bottleneck for the conductor's cross-workspace path, since there's no ANN index (e.g. HNSW) — just a linear scan + full sort per query.

Worth keeping an eye on as usage scales; an approximate nearest-neighbor index (or capping the number of workspaces/vectors scanned per query) would be the natural next step if BASELINE.md latency numbers regress at scale.

🤖 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/daemon/memory/engine.ts` around lines 211 - 271, recallGlobal currently
brute-force scans every workspace’s full vector matrix and sorts all scores on
each query, which will become a latency bottleneck as corpus size grows. Update
the cross-workspace path in engine.ts, specifically recallGlobal, to avoid full
linear scans by introducing an ANN-style lookup (or another indexed candidate
retrieval strategy) instead of iterating listWorkspaceIds() and scoring every
vector, while preserving the global ranking behavior. If indexing is not
feasible yet, add a bounded scan strategy so the number of vectors/workspaces
examined per query is capped.
src/daemon/memory/reranker-transformersjs.ts (1)

27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant existsSync check before mkdirSync.

mkdirSync(dir, { recursive: true }) doesn't throw if the directory already exists, so the existsSync guard is unnecessary.

♻️ Simplify
   constructor(modelName: string, cacheDir?: string) {
     this.modelName = modelName;
     this.#cacheDir = cacheDir ?? join(homedir(), ".codeoid", "models");
-    if (!existsSync(this.#cacheDir)) {
-      mkdirSync(this.#cacheDir, { recursive: true });
-    }
+    mkdirSync(this.#cacheDir, { recursive: 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 `@src/daemon/memory/reranker-transformersjs.ts` around lines 27 - 33, The
constructor in reranker-transformersjs does an unnecessary existsSync check
before mkdirSync. Simplify the directory setup in the constructor by removing
the existsSync guard and relying on mkdirSync(this.#cacheDir, { recursive: true
}) alone, keeping the modelName and `#cacheDir` initialization intact.
src/daemon/eval/baseline.ts (1)

40-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Fixture JSON parsed without runtime validation.

JSON.parse(readFileSync(FIXTURE, "utf8")) is cast directly to FixtureCase[] with no schema check. A malformed or partially-edited fixture (missing field, wrong type) will silently produce undefined references/expected ids that flow into evalCases and skew the reported precision/recall numbers rather than failing fast.

As per coding guidelines, src/**/*.ts should "Use Zod for validation of runtime data and configuration."

♻️ Validate with Zod
+import { z } from "zod";
+
+const FixtureCaseSchema = z.object({
+  reference: z.string(),
+  expectedSessionId: z.string(),
+  expectedWorkspaceId: z.string(),
+});
+
-const cases: FixtureCase[] = JSON.parse(readFileSync(FIXTURE, "utf8"));
+const cases: FixtureCase[] = z.array(FixtureCaseSchema).parse(JSON.parse(readFileSync(FIXTURE, "utf8")));
🤖 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/daemon/eval/baseline.ts` around lines 40 - 44, Runtime fixture data in
baseline evaluation is being trusted directly, so malformed JSON can produce bad
`evalCases` instead of failing fast. Update the fixture loading in `baseline.ts`
to validate the parsed result with Zod before mapping it into `EvalCase[]`,
ensuring each `FixtureCase` has the required `reference` and `expectedSessionId`
fields with the right types. Use the existing `cases`/`evalCases` flow in
`baseline.ts` so invalid fixture content throws a clear validation error before
metrics are computed.

Source: Coding guidelines

src/daemon/store.ts (1)

93-102: 🔒 Security & Privacy | 🔵 Trivial

Conductor api_key is persisted in cleartext SQLite.

This is called out as a deliberate tradeoff in agent-identity.ts ("api_key is the ONE credential at rest"), so not blocking, but worth hardening given this key can mint fleet-wide session:dispatch tokens.

Consider encrypting api_key at rest (e.g., via an OS keychain/secret manager or an application-level envelope key) or restricting file permissions on the SQLite DB file, since anyone with read access to the DB can impersonate the conductor.

🤖 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/daemon/store.ts` around lines 93 - 102, The conductor_identity table
currently stores api_key in cleartext in the SQLite-backed store, so harden the
persistence in store.ts by encrypting api_key before writing and decrypting it
when reading, or by switching to an OS keychain/secret manager or envelope key
approach. Update the store methods that handle conductor_identity records so the
api_key field is never persisted directly, and also tighten SQLite DB file
permissions where this storage is initialized.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@docs/conductor-design.md`:
- Around line 263-275: Clarify the security boundary by making the egress
contract explicit in the conductor design: keep v1 limited to owner approval
only and move Shield out of the current boundary, or revise the “Shield on
egress” guidance to match the v1 approval flow. Update the “Security boundary —
dogfood Highflame” section and any related references so the intended gate is
unambiguous and consistent with the existing approvalId-based permissions path.
- Around line 143-149: Move the conductor `session:*` permission out of
`AGENT_TOOL_SCOPES` and update the wording in the conductor design section to
treat it as a conductor identity grant instead of an agent-tool scope. Adjust
the relevant text around `AGENT_TOOL_SCOPES`, `tokens.delegate`, and the
conductor spawn flow so it clearly reflects that protocol scopes and tool scopes
are separate, while preserving the `created_by` and delegation-depth
explanation.
- Around line 110-113: The MCP allowlist in the Claude provider is too narrow
and still only permits codeoid_memory tools, so conductor-mounted codeoid_fleet
remains blocked. Update allowedTools in the Claude provider logic to include
codeoid_fleet alongside codeoid_memory, and make sure the allowlist is applied
where query() builds the tool set so the conductor role can reach the new MCP
server.

In `@src/daemon/agent-identity.ts`:
- Around line 439-468: The conductor identity flow sets `#conductor` before
saveConductorIdentity completes, so a Store failure leaves in-memory state out
of sync with durable storage. Update the registration path in the conductor
identity method to persist the identity first, and only assign this.#conductor
after saveConductorIdentity and the audit call succeed; if persistence fails,
keep the in-memory state unchanged so resumeConductor() cannot short-circuit on
a non-durable identity.
- Around line 591-627: The deactivateConductor flow is deleting the persisted
conductor identity even when agents.deactivate() fails, which drops the only
retryable record. Update deactivateConductor in agent-identity.ts so
deleteConductorIdentity() and clearing `#conductor` only happen after a successful
remote deactivation, and preserve the stored row on the catch path for later
retry. Keep the audit call tied to the success path and use the existing
conductor/#store.getConductorIdentity references to locate the logic.

In `@src/daemon/memory/cards.ts`:
- Around line 264-292: The assertFact flow in cards.ts needs a chronological
guard before superseding an existing open fact: when an open row is found,
compare the incoming validAt against open.valid_at and reject or explicitly
handle cases where validAt is earlier. Update the logic around assertFact and
the UPDATE facts closure so it never sets invalid_at before the prior row’s
valid_at, preserving the bi-temporal invariants and keeping factsAsOf queries
valid for historical backfills.
- Around line 168-212: The card persistence flow in the upsert path is not
atomic because the session_cards write and the session_cards_fts delete/insert
run as separate statements. Update the save logic in the card upsert method to
wrap the existing prepare/run calls and the FTS refresh in a single
this.#db.transaction(...) so the session_cards and session_cards_fts tables stay
in sync if a failure occurs mid-write. Use the existing session_cards,
session_cards_fts, and cardFtsText(card) operations inside that transaction and
keep the final getCard(card.sessionId) return after the transaction completes.
- Around line 275-322: The assertFact flow currently performs the supersede
UPDATE and replacement INSERT as separate operations, so a failure between them
can leave no open fact for the same subject/predicate. Wrap the body of
assertFact in a single transaction using this.#db.transaction(...) so the
open-fact lookup, optional close-out UPDATE, and INSERT all commit atomically,
and keep the existing rowToFact/open logic and returned FactRow shape unchanged.

In `@src/daemon/memory/engine.ts`:
- Around line 108-120: The reranker initialization in `MemoryEngine.init()` can
stall daemon startup, so wrap `this.#reranker.init()` in a bounded timeout and
treat timeout as a failure. Keep the existing `catch` fallback path in
`MemoryEngine` so `#rerankerReady` is set to false and the fusion-only log
remains the recovery behavior. Use the `this.#reranker` init block as the only
change point and ensure the timeout rejects/aborts cleanly without bypassing the
current error handling.

In `@src/daemon/memory/reranker-transformersjs.ts`:
- Around line 80-85: The close() method in reranker-transformersjs should
explicitly dispose the loaded model before clearing references. Update the
close() implementation on the reranker class to await the model’s dispose method
if present, then null out `#tokenizer`, `#model`, and `#initPromise`; keep the change
scoped to the model cleanup path since AutoTokenizer does not need disposal.

---

Nitpick comments:
In `@src/daemon/eval/baseline.ts`:
- Around line 40-44: Runtime fixture data in baseline evaluation is being
trusted directly, so malformed JSON can produce bad `evalCases` instead of
failing fast. Update the fixture loading in `baseline.ts` to validate the parsed
result with Zod before mapping it into `EvalCase[]`, ensuring each `FixtureCase`
has the required `reference` and `expectedSessionId` fields with the right
types. Use the existing `cases`/`evalCases` flow in `baseline.ts` so invalid
fixture content throws a clear validation error before metrics are computed.

In `@src/daemon/memory/engine.ts`:
- Line 406: The rerank gating in the engine logic is overly duplicated and
should be simplified. Update the `doRerank` assignment in `MemoryEngine` to use
the clearer `this.#rerankerReady && opts.rerank !== false` form instead of
rechecking `#rerankerReady` twice. Keep the behavior the same while making the
condition easier to read and maintain.
- Around line 211-271: recallGlobal currently brute-force scans every
workspace’s full vector matrix and sorts all scores on each query, which will
become a latency bottleneck as corpus size grows. Update the cross-workspace
path in engine.ts, specifically recallGlobal, to avoid full linear scans by
introducing an ANN-style lookup (or another indexed candidate retrieval
strategy) instead of iterating listWorkspaceIds() and scoring every vector,
while preserving the global ranking behavior. If indexing is not feasible yet,
add a bounded scan strategy so the number of vectors/workspaces examined per
query is capped.

In `@src/daemon/memory/reranker-transformersjs.ts`:
- Around line 27-33: The constructor in reranker-transformersjs does an
unnecessary existsSync check before mkdirSync. Simplify the directory setup in
the constructor by removing the existsSync guard and relying on
mkdirSync(this.#cacheDir, { recursive: true }) alone, keeping the modelName and
`#cacheDir` initialization intact.

In `@src/daemon/store.ts`:
- Around line 93-102: The conductor_identity table currently stores api_key in
cleartext in the SQLite-backed store, so harden the persistence in store.ts by
encrypting api_key before writing and decrypting it when reading, or by
switching to an OS keychain/secret manager or envelope key approach. Update the
store methods that handle conductor_identity records so the api_key field is
never persisted directly, and also tighten SQLite DB file permissions where this
storage is initialized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0d168bfa-b965-4acd-9c00-5d8d1636b6bb

📥 Commits

Reviewing files that changed from the base of the PR and between 84a6446 and 692e6f2.

📒 Files selected for processing (27)
  • docs/conductor-build-plan.md
  • docs/conductor-design.md
  • docs/conductor-prior-art-firstmate.md
  • docs/conductor-prior-art-hermes.md
  • docs/conductor-session-resolution.md
  • docs/session-resolution.md
  • package.json
  • packages/protocol/src/scopes.ts
  • src/daemon/agent-identity.ts
  • src/daemon/eval/BASELINE.md
  • src/daemon/eval/baseline.ts
  • src/daemon/eval/fixtures/session-resolution.json
  • src/daemon/eval/metrics.test.ts
  • src/daemon/eval/metrics.ts
  • src/daemon/memory/cards.test.ts
  • src/daemon/memory/cards.ts
  • src/daemon/memory/engine.test.ts
  • src/daemon/memory/engine.ts
  • src/daemon/memory/reranker-transformersjs.ts
  • src/daemon/memory/reranker.ts
  • src/daemon/memory/store.ts
  • src/daemon/session-manager.ts
  • src/daemon/store.ts
  • src/integration/conductor-zeroid.test.ts
  • src/tests/agent-identity-conductor.test.ts
  • src/tests/reranker.test.ts
  • src/tests/scopes.test.ts

Comment thread docs/conductor-design.md
Comment thread docs/conductor-design.md Outdated
Comment thread docs/conductor-design.md
Comment thread src/daemon/agent-identity.ts Outdated
Comment thread src/daemon/agent-identity.ts
Comment thread src/daemon/memory/cards.ts Outdated
Comment thread src/daemon/memory/cards.ts Outdated
Comment thread src/daemon/memory/cards.ts Outdated
Comment thread src/daemon/memory/engine.ts
Comment thread src/daemon/memory/reranker-transformersjs.ts
…d init

- registerConductor persists the identity BEFORE exposing it in memory, so a
  Store failure reads as registration failure, not a phantom durable identity.
- deactivateConductor keeps the persisted row when the remote deactivation
  fails — it's the only durable record of a still-live identity, and the next
  call retries against it (test added).
- Card upsert + FTS mirror refresh and assertFact's supersede + insert each
  commit in one transaction, so a crash can't leave the FTS drifted or a
  (subject, predicate) with no open fact.
- assertFact rejects out-of-order validAt instead of closing the open fact
  with invalid_at < valid_at, which made the row unsatisfiable for every
  factsAsOf() read (test added).
- reranker.init() is bounded by a 120s timeout — a stalled model download
  degrades to fusion-only instead of wedging daemon startup; close() disposes
  the ONNX model to actually free WASM memory.
- conductor-design.md: fleet MCP allowlist gotcha noted for P3, §4 updated to
  the CONDUCTOR_SCOPES profile as implemented, §9 Shield marked later-phase
  (v1 = owner approval only, per R4).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@saucam
saucam merged commit b2fb7e0 into main Jul 6, 2026
5 checks passed
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.

1 participant