Skip to content

Work/review 20260912 - #135

Closed
bpstr wants to merge 49 commits into
bartolli:mainfrom
bpstr:work/review-20260912
Closed

bpstr wants to merge 49 commits into
bartolli:mainfrom
bpstr:work/review-20260912

Conversation

@bpstr

@bpstr bpstr commented Sep 12, 2026

Copy link
Copy Markdown

Full Rust review

38 of 55 findings survived the ≥80 confidence threshold: 11 P0, 21 P1, 6 P2. The repository remains unchanged.

P0 — Merge blockers

  1. [NEW] Serve-lock reclamation can delete another live server’s lock — 96
    src/cli/commands/serve.rs:49-96
    Concurrent reclaimers can remove a newly acquired lock, potentially allowing multiple servers to corrupt one Tantivy index.

  2. [NEW] Vector replacement writers share one staging filename — 95
    src/vector/storage.rs:205-248
    Independent writers can truncate or mutate the same temporary inode after publication, corrupting the vector generation.

  3. [NEW] Chunked embedding publishes only the final chunk — 96
    src/vector/engine.rs:71-124, src/indexing/pipeline/stages/embed.rs:15-88
    Indexing 257+ symbols discards preceding 256-symbol chunks while counters still report success.

  4. [PRE-EXISTING] Network MCP authentication is forgeable or absent — 98
    src/mcp/http_server.rs:243-455, src/mcp/https_server.rs:204-285
    HTTP exposes a fixed-token acquisition path, while HTTPS incorrectly treats TLS as client authorization.

  5. [PRE-EXISTING] Project-registry load failures can erase all registrations — 95
    src/init.rs:330-364
    Malformed, unreadable, or incompatible registries are converted to empty registries and overwritten.

  6. [PRE-EXISTING] OAuth parameters permit script injection and open redirects — 98
    src/mcp/http_server.rs:322-401, src/mcp/https_server.rs:397-498
    redirect_uri and state reach executable HTML/JavaScript without contextual escaping or registered-URI validation.

  7. [PRE-EXISTING] Force-reindex can expose arbitrary filesystem content — 97
    src/mcp/server.rs:272-355, src/indexing/facade.rs:1111-1237
    MCP callers can index paths outside configured workspace roots.

  8. [PRE-EXISTING] Request diagnostics disclose credentials and OAuth payloads — 98
    src/mcp/http_server.rs:261-338, src/mcp/https_server.rs:247-412
    Headers, cookies, token forms, OAuth state, and request bodies are logged verbatim.

  9. [PRE-EXISTING] Plugin extraction follows symlinks outside an untrusted clone — 98
    src/plugins/resolver.rs:38-79, src/plugins/mod.rs:809-830
    Repository-controlled symlinks can copy readable host files into installed or indexed plugin trees.

  10. [PRE-EXISTING] Visualization renderers execute repository-controlled markup — 99
    agents/plugins/claude/codanna-toolset/skills/{graph,x-ray}
    Repository strings can escape inline scripts and execute when generated pages open.

  11. [PRE-EXISTING] Executable-path overrides are interpolated into shell commands — 99
    agents/plugins/claude/codanna-toolset/skills/{graph,x-ray}
    Shell metacharacters in --binary PATH can execute arbitrary commands.

P1 — Should fix

  1. [NEW] Async paths block while holding shared locks — 94
    src/mcp/tools/search.rs, src/watcher/unified.rs, src/semantic/simple.rs

  2. [NEW] Single-file hot reload rebuilds the repository-wide symbol cache — 98
    src/indexing/pipeline/incremental.rs, types.rs, watcher/unified.rs

  3. [NEW] Single-file semantic persistence rewrites the full vector corpus — 97
    src/indexing/pipeline/incremental.rs, src/semantic/simple.rs

  4. [NEW] Graph operations issue nested searches and per-node hydration — 94
    src/indexing/facade.rs, src/storage/tantivy/query.rs, MCP search/symbol tools

  5. [NEW] HTTPS sessions overwrite one shared notification peer — 99
    src/mcp/https_server.rs, src/mcp/server.rs, src/mcp/notifications.rs

  6. [NEW] HTTP notification listeners survive disconnected sessions — 99
    src/mcp/http_server.rs, src/mcp/notifications.rs

  7. [NEW] Behavior lookup can panic or substitute Rust semantics — 88
    src/parsing/factory.rs:345-358

  8. [NEW] The crate exposes two incompatible storage error types — 98
    src/error.rs, src/storage/error.rs, src/lib.rs

  9. [NEW] Legacy parser construction rejects implemented JavaScript/TypeScript — 94
    src/parsing/factory.rs, language.rs, registry.rs

  10. [NEW] Compact symbols truncate 32-bit file IDs to 16 bits — 96
    src/symbol/mod.rs:81-338

  11. [NEW] Invalid chunking settings reach production indexing — 99
    src/documents/config.rs, src/cli/commands/documents.rs, src/documents/chunker.rs

  12. [NEW] TypeScript parser tests race through process-wide CWD — 98
    tests/parsers/typescript/test_alias_resolution.rs, test_pipeline_resolution.rs

  13. [NEW] CLI shell regressions neither run in CI nor fail reliably — 98
    tests/cli/*.sh, tests/cli_tests.rs, .github/workflows/full-test.yml

  14. [PRE-EXISTING] Search limits can panic Tantivy or request enormous allocation — 99
    src/mcp/requests.rs, src/mcp/tools/search.rs, src/storage/tantivy/query.rs

  15. [PRE-EXISTING] Deleting one collection drops every collection’s file state — 99
    src/documents/store.rs:744-782

  16. [PRE-EXISTING] MCP config executes a mutable Context7 package on first use — 93
    .mcp_stdio.json:3-8

  17. [PRE-EXISTING] Relationship persistence failures count as resolved — 96
    src/indexing/pipeline/stages/write.rs, phase2.rs

  18. [PRE-EXISTING] Profile removal and team sync return false success — 99
    src/profiles/mod.rs:443-559

  19. [PRE-EXISTING] MCP notification delivery failures are discarded — 96
    src/mcp/notifications.rs, src/mcp/server.rs

  20. [PRE-EXISTING] Traversal errors silently produce incomplete indexes/watches — 97
    src/indexing/walker.rs:58-95

  21. [PRE-EXISTING] Corrupt index metadata is treated as absent and overwritten — 86
    src/storage/persistence.rs:60-200

P2 — Worth noting

  1. [NEW] Narrow layouts replace interactive mail with a screenshot — 94
    Example React mail/demo pages.

  2. [NEW] Theme color controls lack accessible names and selected state — 98
    examples/typescript/react/src/components/theming/ThemeCustomizer.tsx

  3. [NEW] Public documentation materially misstates current behavior — 84

  4. [NEW] Several tests have weak or artificial oracles — 93

  5. [PRE-EXISTING] Plugin runtime prerequisites are incomplete — 88

  6. [PRE-EXISTING] Language and MCP additions require synchronized catalogs — 89

Coverage and gaps

  • All requested specialists completed; shared stack profiling succeeded.
  • Codanna discovery was unavailable under the review runner’s no-approval constraints, so reviewers used read-only source and Git inspection.
  • Tests and builds were not run; findings are statically validated.
  • Prior GitHub review comments were unavailable because the saved token was invalid and the API was unreachable.
  • Any regression testing must use deterministic fixtures and mocked provider transports under the paid-inference policy.

Review was done by https://github.com/bpstr/deep-code-review on gpt-5.6-sol

bpstr and others added 30 commits September 12, 2026 09:49
* hardening: validate semantic vector layout before loading

* fix: auto-apply rustfmt, clippy, and cargo fix

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* ci: apply upstream bartolli#123 hardening patch

* fix: remove million-symbol relationship cache cap

* ci: recheck symbol cache fix against current main

* ci: remove symbol cache recheck marker

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* ci: apply upstream bartolli#122 relative-import guard

* fix: fail closed on unresolved relative imports

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* ci: apply upstream bartolli#126 macOS watcher fix

* fix: use recursive handler-root watches on macOS

* ci: add macOS watcher witness

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* hardening: make watcher overflow nonblocking and recoverable

* fix: auto-apply rustfmt, clippy, and cargo fix

* ci: recheck watcher hardening against current main

* ci: remove recheck marker

* ci: recheck watcher against current hardening main

* ci: remove watcher recheck marker

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* ci: apply upstream bartolli#133 stdio shutdown fix

* fix: stop stdio watcher tasks when transport closes

* ci: revalidate stdio shutdown against current main

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* hardening: surface parse worker panics as fatal pipeline errors

* ci: revalidate parse panic hardening against current main

* ci: revalidate parse panic after stdio lifecycle merge
* hardening: reject non-finite vector values

* hardening: reject non-finite remote embeddings

* ci: revalidate non-finite embedding hardening on batch-2 base
* ci: apply upstream bartolli#130 buffered writes on zero-copy path

* ci: replace invalid buffered-write patch workflow

* ci: add buffered-write patch script

* ci: run buffered-write patch script

* perf: buffer vector payload writes

* ci: revalidate buffered vector writes on batch-2 base

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hardening: fail indexing when a read worker panics
hardening: reject legacy non-finite vectors and total-order semantic scores
bpstr and others added 19 commits September 12, 2026 11:24
perf: select semantic top-k before sorting
perf: batch vector candidate lookups
* ci: stage vector layout hardening

* hardening: validate vector storage physical layout

* ci: stage vector header invariant checks

* ci: assert remap version error structurally

* hardening: validate vector header invariants on reopen

* ci: remove vector layout patch workflow

* ci: remove vector header invariant patch workflow

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…ment

hardening: replace vector generations atomically
@bpstr bpstr closed this Sep 12, 2026
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