feat: source pipeline integrity and operator tooling (v1.34.0)#143
Conversation
An early-closed stdout pipe (`o2b <listing> | head -1`) surfaces as an EPIPE write failure. That is the normal end of a shell pipeline, not a program error, so the CLI must exit 0 with no diagnostic. A non-EPIPE stdout write failure (ENOSPC, EIO, ...) is a real I/O fault and was being swallowed silently; it now fails loudly with its message and a nonzero exit. The new guard installs a `process.stdout` error listener (Bun surfaces a closed-pipe write as an asynchronous error event) and the entry point's promise catch covers the synchronous-throw case. `vault-log` shares the same `main.ts` entry, so both CLIs are covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hygiene repo scan only skipped a static list of directory names, so paths ignored by the repo's own .gitignore files were still read and scanned. A new path-scope engine (src/core/fs/ignore.ts) implements the documented gitignore subset: basename vs anchored matching, directory-only patterns, `*`/`?`/doubled-star wildcards, nested composition where a deeper file scopes its subtree, a nearer `!` re-include winning over an outer ignore, and `.git/info/exclude` layered at the lowest precedence. scan-repo composes these per directory during its walk. With no ignore files present the walk is byte-identical to the static skip-dir baseline. Malformed patterns are reported on stderr and produce no rule, so a bad pattern never silently drops a path. The engine sits below both hygiene and ingest so P2 can reuse it without a duplicate matcher. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Folder-ingest planning could only walk a whole source directory. It now accepts two optional scoping controls, plumbed through the batch-plan CLI verb and the brain_ingest_batch_plan MCP tool: - --src-subpath restricts discovery to a subtree of the source dir; a value escaping the source root rejects with a typed error. - --exclude takes gitignore-style patterns matched by the shared src/core/fs/ignore.ts engine (no duplicate matcher); an excluded directory is pruned so its subtree is never walked. A malformed pattern is a typed error, never a silent skip. With neither flag set the plan (paths, planId, sourceDir) is byte-identical to before. No new MCP tools are added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The schema pack stored an `extractable` allowlist of schema tokens (set via the set_extractable mutation) but nothing consulted it. Folder-ingest page discovery now gates on it: when the allowlist is non-empty, a discovered page whose `schema_type` is not in it is skipped before extraction, reported on the plan as skipped-with-reason, and logged to stderr rather than silently ingested. A page with no declared type stays ungated, and an empty allowlist gates nothing so the plan (paths, planId, and serialized output) is byte-identical to before. The gate is a pure, read-only helper that reads frontmatter and returns a partition; it touches no schema mutation surface. The batch-plan CLI verb and brain_ingest_batch_plan MCP tool surface the skipped set only when non-empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a no-LLM, stdlib-only pre-ingest pass (P4, t_ef786747) that turns a code source into JSON entity/edge seeds: classes and functions as entity seeds, imports and inheritance as edge seeds. Structural parsing per language family (TypeScript/JavaScript and Python) via a line grammar, deduped and sorted so the same input always yields byte-identical output. Unsupported extensions are reported as unextracted with a reason, never a fake empty success. Wire the pass into ingestSource behind an opt-in preExtract option that surfaces the seeds on the result for the agent step; with the pass off the ingest and its summary page are byte-identical to before. Expose the pass via a new `o2b brain pre-extract` CLI verb and an optional pre_extract flag on brain_ingest_source (no new MCP tool; tool count stays 102). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add reconcilePlan (P5, t_d067a153): after a batch plan drains, diff the plan's dispatched set (batch files plus manifest-unchanged skips) against the checkpoint's completed entries and report the gap - each source that was dispatched but never recorded as ingested. It is a warning surface, not a retry: read-only over checkpoint state, idempotent, and it re-dispatches nothing. A plan that dispatched nothing, or whose sources all completed, reports an empty gap explicitly. Surface the report via a `reconcile` flag on brain_ingest_batch_plan and `--reconcile` on `o2b brain batch-plan`; the report rides on the output only when requested, so the default output stays byte-identical. The MCP reconcile runs before any resume-drain checkpoint clear so it reads a live checkpoint. No new MCP tool; tool count stays 102. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vents Add a structural parser for inline `[Source: <name>, YYYY-MM-DD]` prose markers and a scan surface that promotes each well-formed citation into the temporal timeline as a dated `source-citation` provenance event, stamped at the citation date. - New `src/core/brain/temporal/citations.ts`: pure `parseCitations` (fixed marker grammar, ISO-shaped date validated as a real calendar date, no natural-language date parsing) plus `scanCitations` which walks the configured note folders and appends one dated event per unique citation. - Dedup on (normalized name, date) against already-logged source-citation events, so a re-scan of an unchanged vault promotes nothing. Malformed markers are reported explicitly and skipped. - New `source-citation` log event kind; a new `o2b brain scan-citations` CLI verb (--dry-run, --strict, --path, --exclude, --json). Kept CLI-only for the agent surface: scanning is an operator maintenance action like scan-inline (also CLI-only), and adding an MCP tool would break the frozen 102-tool parity count with no existing tool to extend. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Assemble the `chunk_fts` FTS5 tokenizer clause from validated config
keys instead of a hardcoded literal, so operators can tune diacritic
folding and layer Porter stemming without patching the schema.
- New `buildFtsTokenize` in schema.ts composes the clause from an
allow-list (`search_fts_diacritics` 0|1|2, `search_fts_stemmer`
none|porter); an out-of-range value throws a typed
SearchError("INVALID_INPUT") listing the allowed values before it can
reach the DDL.
- Migrations that create `chunk_fts` (v1, v2, v5) take the clause via a
MigrationContext threaded through applyMigrations; unset config yields
the historical `unicode61 remove_diacritics 2` byte-identically. The
CJK `chunk_trigram` shadow index keeps its `trigram` tokenizer.
- resolveSearchConfig resolves and validates the keys once
(ResolvedSearchConfig.ftsTokenize); Store.open passes the clause to
applyMigrations. No implicit reindex: the clause only takes effect on a
fresh build, documented in the CLI reference.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add count predicates over a note's backlinks/outlinks to the search
filter DSL, so operators and agents can select orphans (`backlinks=0`),
hubs (`outlinks>=N`), and any `= != > >= < <=` comparison.
- graph-index.ts gains directed in/out degree maps on the memoized
snapshot plus a `degreeForPath` accessor; a path with no resolved
edges (or an unknown path) reads as an orphan.
- property-filter.ts gains `parseDegreePredicate` (typed
SearchError("INVALID_INPUT") on bad syntax, listing allowed fields and
operators) and a pure `applyDegreeFilters` (dependency-injected degree
lookup; empty list is a byte-identical pass-through).
- search.ts applies the predicates as a post-rank phase backed by the
link-graph snapshot; result-filters.ts wires the store. CLI `--degree`
and the MCP brain_search `degree` param feed the same predicate list.
- Queries without degree predicates are byte-identical; the MCP tool
count stays 102 (a new param on the existing tool, not a new tool).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Introduce the wave's diagnostics-signal model (issue class + detector + optional fixer + next-command hint) co-located with the doctor, and a guarded `o2b brain doctor --repair` mode built on it. - `--repair` previews planned fixes without writing (dry-run default); `--repair --apply` performs them and appends one typed `doctor-repair` event per fix. Re-running after apply is an idempotent no-op. - Fixers exist only for issue classes the doctor already detects: `wal-gap` closes a dangling dream workrun (an append-only write-ahead checkpoint log with no terminal phase) by appending the missing `interrupted` marker; `orphaned-reference` prunes a dead `evidenced_by` Brain link from a preference/retired record and its `## Origin` bullet. Broken structural links (supersedes, retired_by, superseded_by) are reported needs-review, never auto-removed. - Detected classes with no fixer are aggregated as unfixable, each with its next command from the signal registry (hints travel with the issue definition, never hardcoded in a formatter). - Plain doctor and `--strict` stay read-only and byte-identical; `--strict` cannot be combined with an applying repair. - MCP: optional `repair`/`apply` params on the existing brain_doctor tool (tool count stays 102). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `o2b brain status` (and the `brain_status` MCP counterpart): one read-only verb composing the operator signal sources that previously required polling six commands - doctor, semantic health, hygiene, stale scan, review candidates, active profile, and state-file health - into a single readable snapshot. - Every problem line carries the exact next command to run, looked up from the O2 diagnostics-signal registry (resolveSignal), so hints travel with the issue definition and are never hardcoded in the formatter. - A healthy vault prints a compact all-clear; each source is wrapped fail-soft so one broken surface degrades its line, not the snapshot. - Reads only. - MCP tool count goes 102 -> 103; frozen parity/count tests updated deliberately (mcp.test, removed-tools, brain-tools-parity), and brain_status is added to the preview-budget exempt list (bounded snapshot). Also point the review-queue signal hint at `o2b brain dream --dry-run` (the real surface for review candidates). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughChangesThe v1.34.0 release adds scoped discovery, ignore rules, extractability gating, deterministic code extraction, reconciliation, citation promotion, configurable search, graph-degree predicates, repair/status tooling, MCP updates, and clean stdout-pipe handling. Documentation, tests, and version metadata are updated. Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/hygiene/scan-repo.ts (1)
205-220: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the ignore scope to root scan files too.
The recursive targets use
baseScope, butSCAN_ROOT_FILESare appended unconditionally. Consequently, an ignored root file such asREADME.mdis still scanned, violating the new discovery contract.Proposed fix
for (const name of SCAN_ROOT_FILES) { const p = join(root, name); try { - if (statSync(p).isFile()) abs.push(p); + if (statSync(p).isFile() && !baseScope.isIgnored(toPosixRel(root, p), false)) { + abs.push(p); + } } catch {Add a regression test covering a root file ignored by
.gitignore.🤖 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/core/hygiene/scan-repo.ts` around lines 205 - 220, Update listScanTargets so each SCAN_ROOT_FILES entry is checked against baseScope before being appended, matching the filtering applied by collectFiles; preserve absent-file handling and reportIgnoreWarnings behavior. Add a regression test demonstrating that a root file ignored by .gitignore is excluded from the returned scan targets.
🧹 Nitpick comments (3)
src/cli/command-manifest.ts (2)
198-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd missing flags to the command manifest.
The
scan-citationsverb accepts flags (e.g.--dry-run,--strict,--path,--exclude) in its handler (cmdBrainScanCitations), but they are missing from the command manifest. Declaring them here ensures shell completions and manifest-driven tooling can surface these options.🛠️ Proposed fix
- command("scan-citations", "Promote inline [Source: ...] citations to dated events"), + command( + "scan-citations", + "Promote inline [Source: ...] citations to dated events", + [ + flag("vault", "string"), + flag("dry-run", "boolean"), + flag("strict", "boolean"), + flag("path", "string-array"), + flag("exclude", "string-array"), + flag("agent", "string"), + flag("json", "boolean"), + ] + ),🤖 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/cli/command-manifest.ts` at line 198, Add the handler-supported flags --dry-run, --strict, --path, and --exclude to the scan-citations command manifest entry, reusing the existing manifest flag declaration conventions so completions and manifest-driven tooling expose them. Keep the command description and cmdBrainScanCitations behavior unchanged.
134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd missing flags to the command manifest.
The
statusverb accepts flags (e.g.--vault,--json) in its handler (cmdBrainStatus), but they are missing from the command manifest. Declaring them here ensures shell completions and manifest-driven tooling can surface these options.🛠️ Proposed fix
- command("status", "Unified operator status snapshot with next-command hints"), + command( + "status", + "Unified operator status snapshot with next-command hints", + [ + flag("vault", "string"), + flag("json", "boolean"), + ] + ),🤖 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/cli/command-manifest.ts` at line 134, Update the status command entry in the command manifest to declare the flags accepted by cmdBrainStatus, including --vault and --json, using the manifest’s existing flag-definition format so completions and manifest-driven tooling expose them.src/core/brain/ingest/batch-plan.ts (1)
315-316: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse case-insensitive extension matching.
DEFAULT_INGESTIBLE_EXTENSIONSnormalizes extensions to lowercase, butextname(entry.name)preserves the file's actual case. This will silently ignore valid files with uppercase extensions likeREADME.MD. Consider lowercasing the extension before the set lookup.♻️ Proposed fix
- } else if (entry.isFile() && extensions.has(extname(entry.name))) { + } else if (entry.isFile() && extensions.has(extname(entry.name).toLowerCase())) { if (exclude.isIgnored(rel, false)) continue; out.push(abs);🤖 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/core/brain/ingest/batch-plan.ts` around lines 315 - 316, Update the extension lookup in the directory-entry handling branch around entry.isFile() to lowercase extname(entry.name) before checking extensions.has(...), preserving the existing ignore filtering and ingestion flow.
🤖 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 @.codex-plugin/plugin.json:
- Line 3: The version in .codex-plugin/plugin.json was edited directly; revert
that change, update the single source-of-truth version in package.json, then run
scripts/sync-version.ts via Bun to regenerate all mirrored manifest versions.
In `@src/cli/brain/verbs/doctor.ts`:
- Around line 19-20: Require repair mode whenever apply is supplied: update
src/cli/brain/verbs/doctor.ts lines 19-20 and the doctor command validation at
lines 30-34 to reject apply without repair via usageError before normal
execution, using plain stderr and exit code 2 rather than a JSON envelope;
update src/mcp/brain/health-tools.ts lines 25-37 to reject apply: true unless
repair: true, and align the schema contract at lines 160-169 with this
requirement.
In `@src/cli/brain/verbs/scan-citations.ts`:
- Around line 35-59: Replace every use of the nonexistent result.malformed
property in the scanCitations output and strict-exit logic with
result.malformedMarkers.length. Preserve the existing JSON field, text output,
and --strict behavior while deriving the malformed count from the
malformedMarkers collection.
In `@src/core/brain/diagnostics.ts`:
- Around line 322-337: The apply method must acquire the existing workrun file
lock before its idempotency recheck, then hold that lock through
scanDanglingWorkruns and appendFileSync. Update the apply flow around path and
the second scan to use the established locking mechanism, releasing it reliably
afterward while preserving the current null-return behavior and append logic.
- Around line 447-454: Update removeOriginBullet so it only filters matching
wikilink bullets while inside the ## Origin section. Track section boundaries
during line processing, preserve bullets in all other sections, and retain the
existing normaliseWikilinkTarget comparison within Origin.
- Around line 561-571: Update the repair flow around FIXER_BY_CODE and
appendLogEvent so disk mutations and their doctorRepair audit events are
failure-atomic: ensure a failed appendLogEvent rolls back the fixer’s persisted
changes, or otherwise defer/commit the repair only after logging succeeds.
Preserve idempotent no-op handling and only add the repair to applied after both
mutation and audit logging complete.
- Around line 470-489: The COVERED_DOCTOR_CODES logic in planRepair must not
discard every broken-wikilink finding based solely on its code. Restrict
coverage to findings actually enumerated by the relevant fixer, or retain
unmatched broken-wikilink instances in the unfixable results, while preserving
aggregation of other uncovered doctor findings.
In `@src/core/brain/ingest/batch-plan.ts`:
- Around line 195-197: Remove the process.stderr.write loop over
skippedNonExtractable from the batch-plan core logic, leaving
skippedNonExtractable available on the returned plan for callers such as
cmdBrainBatchPlan to present. Do not add replacement logging or other side
effects in this function.
In `@src/core/brain/ingest/extractable-gate.ts`:
- Around line 65-87: The partitionExtractable flow currently performs
synchronous pageType/frontmatter I/O for every path when the allowlist is
active. Refactor the planning flow around classifyPaths in batch-plan.ts to
apply the extractability gate only to new and modified files, preserving
unchanged files as unchanged; avoid calling partitionExtractable for the full
discovered-path set.
In `@src/core/brain/ingest/ingest.ts`:
- Around line 197-210: Update runPreExtract to treat any source that cannot be
read as unextracted, not only paths rejected by existsSync. Guard readFileSync
for directories, permission errors, and deletion races, returning the existing
extracted: false result with the canonicalSource reason instead of propagating
the read failure; preserve successful preExtractCodeStructure processing for
readable files.
In `@src/core/brain/ingest/pre-extract.ts`:
- Around line 140-219: Update parseTypeScript and parsePython to tokenize or
otherwise track multiline comment and string-literal state before applying
declaration/import regexes, so matches inside block comments, triple-quoted
Python strings, and TS/JS string literals are ignored. Preserve extraction of
real declarations and imports outside comments and literals, including correct
state across lines.
In `@src/core/brain/ingest/reconcile.ts`:
- Around line 48-57: Update reconcilePlan to build the complete source set from
batch files, plan.skipped, and checkpoint completions matching the plan, then
classify manifest skips and checkpointed paths as ingested rather than missing.
Preserve missing classification only for dispatched batch paths without either
confirmation, and add regressions covering an all-unchanged plan and a fully
resumed plan.
In `@src/core/brain/operator-snapshot.ts`:
- Around line 121-123: Update each catch block in the snapshot collection flow
to record an explicit degraded problem for the failed source instead of leaving
its clean default unchanged. Apply this to the hygiene, stale, review, and
doctor probes, ensuring the added problem causes healthy to become false while
preserving snapshot resilience and continued collection.
- Around line 95-109: Update the runDoctor invocation in the health-check flow
to pass the already resolved search database path from resolveSearchConfig,
using the same vault and configPath inputs as the preceding existsSync check.
Preserve the existing doctor options, including now, so DB-backed findings such
as tier-drift are included.
In `@src/core/brain/temporal/citations.ts`:
- Around line 303-332: Make the citation check-and-persist sequence in the
marker promotion flow atomic by acquiring the shared log lock before checking
DedupIndex and keeping it through appendLogEvent and dedup updates. On
appendLogEvent failure, reconcile DedupIndex from the authoritative JSONL state
before deciding whether to count the citation as promoted or allow a retry, so
partial Markdown failures cannot leave committed events untracked or create
duplicates.
In `@src/core/fs/ignore.ts`:
- Around line 118-132: Update stripTrailingWhitespace so an odd preceding
backslash still preserves the trailing whitespace but removes the backslash that
escapes it from the returned rule. Keep existing trimming behavior for unescaped
whitespace and add a regression test confirming foo\ compiles to match the
filename foo .
- Around line 65-115: Update translateGlob to avoid emitting backtracking-prone
regexes for repeated **/ and wildcard chains, using a linear-time matching
strategy or enforcing a strict wildcard complexity budget while preserving glob
semantics. Also unescape escaped trailing spaces before translation so patterns
such as foo\ match foo rather than a backslash followed by a space.
---
Outside diff comments:
In `@src/core/hygiene/scan-repo.ts`:
- Around line 205-220: Update listScanTargets so each SCAN_ROOT_FILES entry is
checked against baseScope before being appended, matching the filtering applied
by collectFiles; preserve absent-file handling and reportIgnoreWarnings
behavior. Add a regression test demonstrating that a root file ignored by
.gitignore is excluded from the returned scan targets.
---
Nitpick comments:
In `@src/cli/command-manifest.ts`:
- Line 198: Add the handler-supported flags --dry-run, --strict, --path, and
--exclude to the scan-citations command manifest entry, reusing the existing
manifest flag declaration conventions so completions and manifest-driven tooling
expose them. Keep the command description and cmdBrainScanCitations behavior
unchanged.
- Line 134: Update the status command entry in the command manifest to declare
the flags accepted by cmdBrainStatus, including --vault and --json, using the
manifest’s existing flag-definition format so completions and manifest-driven
tooling expose them.
In `@src/core/brain/ingest/batch-plan.ts`:
- Around line 315-316: Update the extension lookup in the directory-entry
handling branch around entry.isFile() to lowercase extname(entry.name) before
checking extensions.has(...), preserving the existing ignore filtering and
ingestion flow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 779d4c38-1f5a-41fc-aabb-3db8b50d4e54
📒 Files selected for processing (83)
.claude-plugin/plugin.json.codex-plugin/plugin.jsonCHANGELOG.mdREADME.mddocs/brainstorm/source-pipeline-integrity/cli-output/claude.mddocs/brainstorm/source-pipeline-integrity/cli-output/prompt.mddocs/brainstorm/source-pipeline-integrity/design.mddocs/brainstorm/source-pipeline-integrity/plan.mddocs/brainstorm/source-pipeline-integrity/variants.mddocs/cli-reference.mddocs/mcp.mdopenclaw.plugin.jsonpackage.jsonplugin.yamlplugins/codex/.codex-plugin/plugin.jsonplugins/hermes/plugin.yamlpyproject.tomlsrc/cli/brain.tssrc/cli/brain/help-text.tssrc/cli/brain/verbs/batch-plan.tssrc/cli/brain/verbs/doctor.tssrc/cli/brain/verbs/index.tssrc/cli/brain/verbs/pre-extract.tssrc/cli/brain/verbs/scan-citations.tssrc/cli/brain/verbs/status.tssrc/cli/command-manifest.tssrc/cli/main.tssrc/cli/search.tssrc/cli/stdout-guard.tssrc/core/brain/diagnostics.tssrc/core/brain/doctor.tssrc/core/brain/ingest/batch-plan.tssrc/core/brain/ingest/extractable-gate.tssrc/core/brain/ingest/ingest.tssrc/core/brain/ingest/pre-extract.tssrc/core/brain/ingest/reconcile.tssrc/core/brain/link-graph/graph-index.tssrc/core/brain/operator-snapshot.tssrc/core/brain/temporal/citations.tssrc/core/brain/types.tssrc/core/fs/ignore.tssrc/core/hygiene/scan-repo.tssrc/core/search/index.tssrc/core/search/property-filter.tssrc/core/search/result-filters.tssrc/core/search/schema.tssrc/core/search/search.tssrc/core/search/store.tssrc/core/search/types.tssrc/mcp/brain/health-tools.tssrc/mcp/brain/ingest-tools.tssrc/mcp/registry-guard.tssrc/mcp/search-tools.tstests/cli/brain-batch-plan-reconcile.test.tstests/cli/brain-batch-plan-scoping.test.tstests/cli/brain-pre-extract.test.tstests/cli/brain.test.tstests/cli/stdout-epipe-guard.test.tstests/core/brain.diagnostics.test.tstests/core/brain.operator-snapshot.test.tstests/core/brain.types.test.tstests/core/brain/graph-index.test.tstests/core/brain/ingest/batch-plan-extractable.test.tstests/core/brain/ingest/batch-plan-scoping.test.tstests/core/brain/ingest/extractable-gate.test.tstests/core/brain/ingest/ingest.test.tstests/core/brain/ingest/pre-extract.test.tstests/core/brain/ingest/reconcile.test.tstests/core/brain/temporal/citations.test.tstests/core/fs/ignore.test.tstests/core/hygiene/scan-repo-ignore.test.tstests/core/search/degree-filter-search.test.tstests/core/search/degree-filter.test.tstests/core/search/fts-tokenizer.test.tstests/core/search/store.test.tstests/core/search/store.vec.test.tstests/helpers/epipe-stream-harness.tstests/helpers/search-fixtures.tstests/mcp/brain-tools-parity.test.tstests/mcp/brain.test.tstests/mcp/ingest-tool.test.tstests/mcp/mcp.test.tstests/mcp/removed-tools.test.ts
| { | ||
| "name": "open-second-brain", | ||
| "version": "1.33.0", | ||
| "version": "1.34.0", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Revert this hand-edited version bump and use the sync script.
As per coding guidelines, the version in package.json is the single source of truth and mirrored files like .codex-plugin/plugin.json must never be hand-edited. Please revert this change, bump the version in package.json, and run bun run scripts/sync-version.ts to propagate the change to all mirrored manifest files.
🤖 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 @.codex-plugin/plugin.json at line 3, The version in
.codex-plugin/plugin.json was edited directly; revert that change, update the
single source-of-truth version in package.json, then run scripts/sync-version.ts
via Bun to regenerate all mirrored manifest versions.
Source: Coding guidelines
| malformed: result.malformed, | ||
| malformed_markers: result.malformedMarkers.map((m) => ({ | ||
| path: m.path, | ||
| line: m.line, | ||
| raw: m.raw, | ||
| reason: m.reason, | ||
| })), | ||
| errors: result.errors.map((e) => ({ path: e.path, message: e.message })), | ||
| files_with_citations: result.filesWithCitations.map((f) => ({ | ||
| path: f.path, | ||
| citations: f.citations, | ||
| })), | ||
| }); | ||
| } else { | ||
| ok(`scanned: ${result.scanned}`); | ||
| ok(`found: ${result.found}`); | ||
| ok(`promoted: ${result.promoted}`); | ||
| ok(`deduped: ${result.deduped}`); | ||
| if (result.malformed > 0) ok(`malformed: ${result.malformed}`); | ||
| for (const m of result.malformedMarkers) { | ||
| info(` malformed: ${m.path}:${m.line}: ${m.reason} (${m.raw})`); | ||
| } | ||
| for (const e of result.errors) info(` error: ${e.path}: ${e.message}`); | ||
| } | ||
| if (flags["strict"] && result.malformed > 0) return 2; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Use malformedMarkers.length instead of malformed.
The CitationScanResult returned by scanCitations does not have a malformed property; it contains malformedMarkers. As a result, result.malformed evaluates to undefined, which silently breaks the --strict exit-code logic (undefined > 0 is false) and omits the count from both the JSON and text outputs.
🐛 Proposed fix
- malformed: result.malformed,
+ malformed: result.malformedMarkers.length,
malformed_markers: result.malformedMarkers.map((m) => ({
path: m.path,
line: m.line,
raw: m.raw,
reason: m.reason,
})),
errors: result.errors.map((e) => ({ path: e.path, message: e.message })),
files_with_citations: result.filesWithCitations.map((f) => ({
path: f.path,
citations: f.citations,
})),
});
} else {
ok(`scanned: ${result.scanned}`);
ok(`found: ${result.found}`);
ok(`promoted: ${result.promoted}`);
ok(`deduped: ${result.deduped}`);
- if (result.malformed > 0) ok(`malformed: ${result.malformed}`);
+ if (result.malformedMarkers.length > 0) ok(`malformed: ${result.malformedMarkers.length}`);
for (const m of result.malformedMarkers) {
info(` malformed: ${m.path}:${m.line}: ${m.reason} (${m.raw})`);
}
for (const e of result.errors) info(` error: ${e.path}: ${e.message}`);
}
- if (flags["strict"] && result.malformed > 0) return 2;
+ if (flags["strict"] && result.malformedMarkers.length > 0) return 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| malformed: result.malformed, | |
| malformed_markers: result.malformedMarkers.map((m) => ({ | |
| path: m.path, | |
| line: m.line, | |
| raw: m.raw, | |
| reason: m.reason, | |
| })), | |
| errors: result.errors.map((e) => ({ path: e.path, message: e.message })), | |
| files_with_citations: result.filesWithCitations.map((f) => ({ | |
| path: f.path, | |
| citations: f.citations, | |
| })), | |
| }); | |
| } else { | |
| ok(`scanned: ${result.scanned}`); | |
| ok(`found: ${result.found}`); | |
| ok(`promoted: ${result.promoted}`); | |
| ok(`deduped: ${result.deduped}`); | |
| if (result.malformed > 0) ok(`malformed: ${result.malformed}`); | |
| for (const m of result.malformedMarkers) { | |
| info(` malformed: ${m.path}:${m.line}: ${m.reason} (${m.raw})`); | |
| } | |
| for (const e of result.errors) info(` error: ${e.path}: ${e.message}`); | |
| } | |
| if (flags["strict"] && result.malformed > 0) return 2; | |
| malformed: result.malformedMarkers.length, | |
| malformed_markers: result.malformedMarkers.map((m) => ({ | |
| path: m.path, | |
| line: m.line, | |
| raw: m.raw, | |
| reason: m.reason, | |
| })), | |
| errors: result.errors.map((e) => ({ path: e.path, message: e.message })), | |
| files_with_citations: result.filesWithCitations.map((f) => ({ | |
| path: f.path, | |
| citations: f.citations, | |
| })), | |
| }); | |
| } else { | |
| ok(`scanned: ${result.scanned}`); | |
| ok(`found: ${result.found}`); | |
| ok(`promoted: ${result.promoted}`); | |
| ok(`deduped: ${result.deduped}`); | |
| if (result.malformedMarkers.length > 0) ok(`malformed: ${result.malformedMarkers.length}`); | |
| for (const m of result.malformedMarkers) { | |
| info(` malformed: ${m.path}:${m.line}: ${m.reason} (${m.raw})`); | |
| } | |
| for (const e of result.errors) info(` error: ${e.path}: ${e.message}`); | |
| } | |
| if (flags["strict"] && result.malformedMarkers.length > 0) return 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 `@src/cli/brain/verbs/scan-citations.ts` around lines 35 - 59, Replace every
use of the nonexistent result.malformed property in the scanCitations output and
strict-exit logic with result.malformedMarkers.length. Preserve the existing
JSON field, text output, and --strict behavior while deriving the malformed
count from the malformedMarkers collection.
| if (dedup.has(marker.name, marker.date)) { | ||
| deduped++; | ||
| continue; | ||
| } | ||
| if (opts.dryRun) { | ||
| // Count the promotion the run WOULD make, and record it in the | ||
| // in-memory index so repeated markers within one dry run still | ||
| // dedup against the first. | ||
| promoted++; | ||
| dedup.add(marker.name, marker.date); | ||
| continue; | ||
| } | ||
| try { | ||
| appendLogEvent(vault, { | ||
| timestamp: `${marker.date}T00:00:00Z`, | ||
| eventType: BRAIN_LOG_EVENT_KIND.sourceCitation, | ||
| body: { | ||
| agent: opts.agent, | ||
| name: marker.name, | ||
| date: marker.date, | ||
| source: `[[${vaultRelSource}]]`, | ||
| }, | ||
| }); | ||
| dedup.add(marker.name, marker.date); | ||
| promoted++; | ||
| } catch (err) { | ||
| errors.push({ | ||
| path: file.absPath, | ||
| message: `promote failed: ${(err as Error).message ?? String(err)}`, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make citation deduplication atomic with event persistence.
The check and append are separate operations, so concurrent scans can both emit the same citation. Additionally, appendLogEvent can persist JSONL before its Markdown write fails; the catch then leaves DedupIndex stale, allowing another identical marker in this scan to append a duplicate and reporting the committed event as unpromoted.
Perform check-and-append under one shared log lock, and reconcile the authoritative JSONL state after partial failures.
🤖 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/core/brain/temporal/citations.ts` around lines 303 - 332, Make the
citation check-and-persist sequence in the marker promotion flow atomic by
acquiring the shared log lock before checking DedupIndex and keeping it through
appendLogEvent and dedup updates. On appendLogEvent failure, reconcile
DedupIndex from the authoritative JSONL state before deciding whether to count
the citation as promoted or allow a retry, so partial Markdown failures cannot
leave committed events untracked or create duplicates.
| function translateGlob(body: string): string { | ||
| let re = ""; | ||
| const n = body.length; | ||
| let i = 0; | ||
| while (i < n) { | ||
| const ch = body[i]!; | ||
| if (ch === "*") { | ||
| const isDouble = body[i + 1] === "*"; | ||
| if (isDouble) { | ||
| const prevIsBoundary = i === 0 || body[i - 1] === "/"; | ||
| let j = i; | ||
| while (body[j] === "*") j++; | ||
| const nextIsBoundary = j >= n || body[j] === "/"; | ||
| if (prevIsBoundary && nextIsBoundary) { | ||
| if (body[j] === "/") { | ||
| // `**/` - zero or more leading path segments. | ||
| re += "(?:[^/]+/)*"; | ||
| i = j + 1; | ||
| } else { | ||
| // trailing `**` - everything below (and the node itself). | ||
| re += ".*"; | ||
| i = j; | ||
| } | ||
| } else { | ||
| // `**` glued to a segment behaves like a single `*`. | ||
| re += "[^/]*"; | ||
| i = j; | ||
| } | ||
| } else { | ||
| re += "[^/]*"; | ||
| i++; | ||
| } | ||
| } else if (ch === "?") { | ||
| re += "[^/]"; | ||
| i++; | ||
| } else if (ch === "[") { | ||
| let j = i + 1; | ||
| if (body[j] === "!" || body[j] === "^") j++; | ||
| if (body[j] === "]") j++; // a leading `]` is a literal member | ||
| while (j < n && body[j] !== "]") j++; | ||
| if (j >= n) throw new MalformedPatternError("unterminated character class"); | ||
| let cls = body.slice(i, j + 1); | ||
| if (cls.startsWith("[!")) cls = `[^${cls.slice(2)}`; | ||
| re += cls; | ||
| i = j + 1; | ||
| } else { | ||
| re += escapeLiteral(ch); | ||
| i++; | ||
| } | ||
| } | ||
| return re; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the dynamic-regex construction and all parser call sites.
rg -n -C4 'new RegExp|translateGlob|parseIgnore(?:Layer|Patterns)\s*\(' \
src testsRepository: itechmeat/open-second-brain
Length of output: 24117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the ignore parser file and inspect the whitespace / matching logic around the claimed issue.
wc -l src/core/fs/ignore.ts tests/core/fs/ignore.test.ts
sed -n '1,240p' src/core/fs/ignore.ts
printf '\n--- tests/core/fs/ignore.test.ts ---\n'
sed -n '1,220p' tests/core/fs/ignore.test.tsRepository: itechmeat/open-second-brain
Length of output: 14380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Build a small read-only probe that exercises the generated regexes on crafted glob bodies.
python3 - <<'PY'
import re, time
def escape_literal(ch: str) -> str:
return re.sub(r'([\\.^$|?*+()\[\]{}])', r'\\\1', ch)
def translate_glob(body: str) -> str:
re_src = ""
n = len(body)
i = 0
while i < n:
ch = body[i]
if ch == "*":
is_double = i + 1 < n and body[i + 1] == "*"
if is_double:
prev_is_boundary = i == 0 or body[i - 1] == "/"
j = i
while j < n and body[j] == "*":
j += 1
next_is_boundary = j >= n or body[j] == "/"
if prev_is_boundary and next_is_boundary:
if j < n and body[j] == "/":
re_src += "(?:[^/]+/)*"
i = j + 1
else:
re_src += ".*"
i = j
else:
re_src += "[^/]*"
i = j
else:
re_src += "[^/]*"
i += 1
elif ch == "?":
re_src += "[^/]"
i += 1
elif ch == "[":
j = i + 1
if j < n and body[j] in "!^":
j += 1
if j < n and body[j] == "]":
j += 1
while j < n and body[j] != "]":
j += 1
if j >= n:
raise ValueError("unterminated character class")
cls = body[i:j+1]
if cls.startswith("[!"):
cls = "[^" + cls[2:]
re_src += cls
i = j + 1
else:
re_src += escape_literal(ch)
i += 1
return re_src
def compile_pat(body: str, anchored=False):
src = translate_glob(body)
if anchored:
return re.compile("^" + src + "$")
return re.compile("^(?:.*/)?"+ src + "$")
cases = [
"*a"*20,
("**/"*10) + "a",
("a*"*20) + "b",
("*/"*20) + "a",
]
for body in cases:
r = compile_pat(body)
path = "/".join(["x"*20]*8) + "/z"
t0 = time.perf_counter()
for _ in range(1000):
r.fullmatch(path)
dt = time.perf_counter() - t0
print(body[:40], len(body), dt)
PYRepository: itechmeat/open-second-brain
Length of output: 416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the parser implementation and probe the two claims:
# 1) whether escaped trailing spaces are preserved correctly,
# 2) whether generated regexes show pathological growth on crafted inputs.
node - <<'JS'
const fs = require('fs');
const text = fs.readFileSync('src/core/fs/ignore.ts', 'utf8');
const lines = text.split('\n');
const slice = lines.slice(0, 220).map((l, i) => String(i + 1).padStart(4) + ': ' + l).join('\n');
console.log(slice.split('\n').slice(58, 156).join('\n'));
function escapeLiteral(ch) {
return ch.replace(/[\\.^$|?*+()\[\]{}]/g, '\\$&');
}
function translateGlob(body) {
let re = '';
const n = body.length;
let i = 0;
while (i < n) {
const ch = body[i];
if (ch === '*') {
const isDouble = body[i + 1] === '*';
if (isDouble) {
const prevIsBoundary = i === 0 || body[i - 1] === '/';
let j = i;
while (body[j] === '*') j++;
const nextIsBoundary = j >= n || body[j] === '/';
if (prevIsBoundary && nextIsBoundary) {
if (body[j] === '/') {
re += '(?:[^/]+/)*';
i = j + 1;
} else {
re += '.*';
i = j;
}
} else {
re += '[^/]*';
i = j;
}
} else {
re += '[^/]*';
i++;
}
} else if (ch === '?') {
re += '[^/]';
i++;
} else if (ch === '[') {
let j = i + 1;
if (body[j] === '!' || body[j] === '^') j++;
if (body[j] === ']') j++;
while (j < n && body[j] !== ']') j++;
if (j >= n) throw new Error('unterminated');
let cls = body.slice(i, j + 1);
if (cls.startsWith('[!')) cls = `[^${cls.slice(2)}`;
re += cls;
i = j + 1;
} else {
re += escapeLiteral(ch);
i++;
}
}
return re;
}
// Trailing escaped-space probe using the parser's trimming rules.
function normalizePattern(pattern) {
let line = pattern;
let escapedTrailingSpace = false;
while (line.endsWith(' ') && !line.endsWith('\\ ')) {
line = line.slice(0, -1);
}
if (line.endsWith('\\ ')) escapedTrailingSpace = true;
return { line, escapedTrailingSpace };
}
for (const p of ['foo\\ ', 'foo ', 'foo\\\\ ']) {
const norm = normalizePattern(p);
const regexSrc = translateGlob(norm.line);
console.log({p, norm, regexSrc});
}
// Backtracking probe.
const cases = ['*a'.repeat(8), 'a*'.repeat(8) + 'b', '**/*a'.repeat(5)];
for (const body of cases) {
const source = '^(?:.*/)?' + translateGlob(body) + '$';
const re = new RegExp(source);
const input = 'x'.repeat(1000);
const t0 = process.hrtime.bigint();
for (let i = 0; i < 20000; i++) re.test(input);
const dt = Number(process.hrtime.bigint() - t0) / 1e6;
console.log({body, source, ms: dt.toFixed(2)});
}
JSRepository: itechmeat/open-second-brain
Length of output: 4587
Avoid backtracking regexes for ignore globs. Repeated **/ and similar wildcard chains can make path scans CPU-heavy; a linear matcher or strict wildcard budget would avoid a config-driven DoS. Escaped trailing spaces also need to be unescaped before translation, since foo\ currently matches a literal backslash plus space instead of foo .
🤖 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/core/fs/ignore.ts` around lines 65 - 115, Update translateGlob to avoid
emitting backtracking-prone regexes for repeated **/ and wildcard chains, using
a linear-time matching strategy or enforcing a strict wildcard complexity budget
while preserving glob semantics. Also unescape escaped trailing spaces before
translation so patterns such as foo\ match foo rather than a backslash
followed by a space.
Source: Linters/SAST tools
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodeRabbit review dispositionThanks for the review. Each finding was verified against the current code before acting. Summary below.
Validation: |
Summary
Vault intake becomes trustworthy end to end: ingest is scoped (
--src-subpath/--exclude) and gated (extractableallowlist), batches are reconciled so silently-lost sources are named, a deterministic no-LLM pre-pass seeds code structure before agent ingest, prose citations become dated provenance events, search gains a configurable FTS tokenizer and graph-degree predicates, and the operator getsdoctor --repairplus a unifiedo2b brain statussnapshot with an exact next command on every problem line. 11 kanban tasks, one release (v1.34.0).Why this wave pays off
flowchart LR SRC["Source tree"] --> SCOPE["Scope<br/>src-subpath + exclude<br/>nested gitignore"] SCOPE --> GATE["Gate<br/>extractable allowlist"] GATE --> PRE["Pre-extract<br/>deterministic code seeds"] PRE --> AGENT["Agent ingest"] AGENT --> REC["Reconcile<br/>dispatched vs ingested"] REC -->|"gap named, never hidden"| OP["Operator"] OP --> STATUS["brain status<br/>next-command hints"] STATUS --> REPAIR["doctor --repair<br/>guarded one-command fixes"] REPAIR --> SRCBefore this wave a monorepo ingested whole or not at all, a dead pack flag gated nothing, an agent could drop dispatched files without a trace, and health signals were scattered across six read-only surfaces. Now every stage of intake is explicit and every gap is reported, and the operator loop closes: status names the problem, the problem names its command, the command fixes it.
Concrete benefits
--reconcilediffs a plan's dispatched set against checkpoint completions and names every missing source; over-reports rather than hides.src/core/fs/ignore.ts) drives both the hygiene scan and ingest excludes; one home, no drift.[Source: name, YYYY-MM-DD]citations land on the temporal timeline, deduplicated and dated at the citation.--degreepredicate away.brain statusanddoctor --repaircan never disagree.What ships
src/core/fs/ignore.tsengine +scan-repo.tso2b brain batch-plan --src-subpath/--exclude, MCP paramso2b brain pre-extract,pre_extractonbrain_ingest_sourceo2b brain batch-plan --reconcile, MCPreconcileo2b brain scan-citationssearch_fts_diacritics,search_fts_stemmero2b search --degree,degreeonbrain_searcho2b brain doctor --repair [--apply], MCP paramso2b brain status, new MCP toolbrain_status(103 total)Test plan
bun test- 6347 pass / 0 fail (793 files)bun run typecheck- cleanbun run lint- exactly 134 warnings / 0 errors (baseline)bun run fmt:check- cleanbun run sync-version:check- 1.34.0 across all 7 manifeststools/listreturns 103 with the new params presentgit check-ignoreground truthSummary by CodeRabbit