feat: gate index/DB creation on consent (no .cartog.toml ⇒ no .cartog/)#144
Merged
Conversation
cartog no longer creates a .cartog/ index for a config-less project on its own. Consent is any one of: a present .cartog.toml, an existing index (Branch 1), or CARTOG_AUTO_INIT (indexes with defaults, writes no config). A Rejected TOML is not consent. An explicit `cartog pull` you typed is itself consent. - cartog-db: add Database::open_existing (no create_dir_all; DbError::NotFound when absent). open()/open_memory() unchanged. - config: pure allow_index_creation(db_path, config_present) predicate; CARTOG_AUTO_INIT env. Evaluated once in main.rs, threaded as allow_create into run_server -> WatchConfig/CartogServer (mirrors walk_filter/redact). - CLI: index / rag index / watch refuse before dispatch (is_gated_write_command = indexes_with_config minus Serve, so the set is defined once). Write commands use open_db_create; reads fall back to open_memory() via open_db. - MCP: serve starts degraded when consent is absent + no DB (empty in-memory DB, no .cartog/); refuse_if_degraded gates the 2 write tools; cartog_stats reports degraded (serialized only when true) + a banner; staleness banners suppressed while degraded. A read-only secondary against an absent DB (its primary is degraded) also starts degraded instead of failing open_readonly, so the serve-for-all-clients flow never leaves a second client without a server. - watch: WatchConfig.allow_create; stays degraded when unmet and pre-builds the index once consent appears. watcher_consents re-checks live each pass and walks up from the watch root to the git root for a .cartog.toml (subdir- launched serve still pre-builds on a git-root init). No live swap. - doctor: the "database not found" hint is consent-aware — points at cartog init when no config, since cartog index would now refuse. - hook: ensure_indexed.sh gate = _has_toml OR db-file-exists OR CARTOG_AUTO_INIT, with a full cwd->git-root walk-up for .cartog.toml (intermediate-dir configs). - docs + site: consent-gate section, CARTOG_AUTO_INIT env var, serve degraded note, push/pull clarified as explicit-action opt-in (not "never gated"), SKILL.md fresh-repo rewrite. Tests: cartog-db open_existing, config predicate (serialized env tests), CLI integration (incl. degraded-serve contrast), MCP degraded (incl. read-only secondary against absent DB), watcher consent (.cartog.toml trigger + git-root walk-up + stops-at-git-root), doctor consent-aware hint, hook db-exists + intermediate-dir walk-up branches.
|
Warning Review limit reached
More reviews will be available in 30 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds an index-creation consent gate: ChangesIndex-creation consent gate
Sequence Diagram(s)sequenceDiagram
participant User
participant main as cartog main
participant config as config::allow_index_creation
participant CartogServer
participant Database
User->>main: cartog index . (no .cartog.toml, no DB)
main->>config: allow_index_creation(db_path, false)
config-->>main: false
main-->>User: error: run cartog init or set CARTOG_AUTO_INIT=1
User->>main: cartog serve (no .cartog.toml, no DB)
main->>CartogServer: new(allow_create=false)
CartogServer->>Database: open_existing(path)
Database-->>CartogServer: DbError::NotFound
CartogServer->>Database: open_memory()
CartogServer-->>main: Server{degraded=true}
main-->>User: MCP server running (degraded)
User->>CartogServer: cartog_index tool call
CartogServer-->>User: error: no index yet, run cartog init
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
crates/cartog-mcp/src/tests/degraded.rs (1)
116-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThis test never exercises the
allow_create=falsestartup branch.It only rechecks
Database::open_existing(...)=NotFound, socrates/cartog-mcp/src/lib.rs:948-1002could regress while this still passes. Please either drive the realCartogServer::new(..., allow_create=false)path through an injectable test provider, or rename the test to match its narrower DB-layer contract.🤖 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 `@crates/cartog-mcp/src/tests/degraded.rs` around lines 116 - 128, This test only checks Database::open_existing and does not cover the real allow_create=false startup path, so regressions in CartogServer::new can slip through. Update the test to exercise CartogServer::new with allow_create=false via the injectable test provider used by degraded startup, and assert the server starts degraded without creating .cartog/; if you keep the current DB-only check, rename the test to reflect that narrower contract.crates/cartog/src/config/load.rs (1)
344-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
#[must_use]toallow_index_creation.This predicate is the consent gate; dropping its return value would silently skip the opt-in check.
As per coding guidelines, "Put
#[must_use]on builder methods returningSelfand on any function whose ignored return value would always be a bug."♻️ Suggested change
+#[must_use] pub fn allow_index_creation(db_path: &Path, config_present: bool) -> bool { config_present || db_path.exists() || auto_init_enabled() }🤖 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 `@crates/cartog/src/config/load.rs` around lines 344 - 346, Add #[must_use] to allow_index_creation in the config load module so callers cannot accidentally ignore the consent-gate result; update the function signature for allow_index_creation(db_path, config_present) accordingly and keep its predicate logic unchanged.Source: Coding guidelines
crates/cartog-watch/src/lib.rs (1)
89-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark
WatchConfig::newas#[must_use].Dropping a freshly-constructed config is always a caller bug, and this constructor is exactly the kind of
Self-returning API the guideline calls out.As per coding guidelines, "Put
#[must_use]on builder methods returningSelfand on any function whose ignored return value would always be a bug."♻️ Suggested change
+#[must_use] pub fn new(root: PathBuf) -> Self { Self { root,🤖 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 `@crates/cartog-watch/src/lib.rs` around lines 89 - 96, `WatchConfig::new` returns a fully constructed `Self` and should not be silently ignored, so add `#[must_use]` to the constructor to match the guideline for builder-like APIs. Apply the attribute directly to `WatchConfig::new` in the `impl WatchConfig` block so callers are warned when they drop the returned config.Source: Coding guidelines
crates/cartog-mcp/src/single_writer.rs (1)
150-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd rustdoc for
run_server.This public entrypoint now exposes the consent gate through
allow_create, but it still has no///docs describing degraded startup and watcher behavior under that flag. As per coding guidelines,**/*.rs: Add///doc comments to all public functions.🤖 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 `@crates/cartog-mcp/src/single_writer.rs` around lines 150 - 160, The public entrypoint run_server is missing rustdoc, including the new allow_create consent gate behavior. Add /// documentation directly above run_server describing its purpose, the meaning of allow_create, and how startup/watch behavior is degraded when creation is disallowed. Also ensure any other public items in single_writer.rs follow the same /// doc-comment requirement.Source: Coding guidelines
crates/cartog/src/commands/manage.rs (1)
345-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd rustdoc for
cmd_watch.This public function now exposes the consent gate via
allow_create, but it still has no///docs explaining when watch may create.cartog/versus stay consent-blocked/degraded. As per coding guidelines,**/*.rs: Add///doc comments to all public functions.🤖 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 `@crates/cartog/src/commands/manage.rs` around lines 345 - 355, Add rustdoc comments to the public cmd_watch function, since it currently has no /// documentation. Describe the watch command’s behavior and explicitly note how allow_create controls whether it may create .cartog/ or remain consent-blocked/degraded, so the public API matches the crate’s doc requirements.Source: Coding guidelines
🤖 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 `@crates/cartog-db/src/store/lifecycle.rs`:
- Around line 43-48: open_existing currently relies on db_path.exists() before
calling open_no_create_dir, which can race and still allow creating a new
database; update open_existing in lifecycle::Lifecycle to use open_with_flags
without SQLITE_OPEN_CREATE so it performs a true non-creating open, and keep the
shared migrate/setup path only after that open succeeds. Ensure the existing
open_no_create_dir/open_with_flags flow is used so a missing or removed db_path
returns NotFound instead of creating a fresh DB.
In `@crates/cartog/src/commands/doctor.rs`:
- Around line 470-475: The missing-database hint in doctor is still using
config_present instead of the real consent decision, so it can suggest cartog
init even when cartog index is already allowed via CARTOG_AUTO_INIT. Update the
consent plumbing in doctor.rs by threading the full predicate from
allow_index_creation(...) / auto_init_enabled() into check_database, and use
that value instead of the current config_present-only check so check_git_repo,
check_config, and check_database stay aligned with runtime behavior.
In `@crates/cartog/src/commands/shared.rs`:
- Around line 283-290: Strengthen open_db’s existing-index test by verifying it
reopens the on-disk database instead of falling back to open_memory(). In
open_db_opens_an_existing_index, after creating the real DB with Database::open,
insert a sentinel row before calling open_db, then assert that the reopened
Database still contains that row and not just that it is empty or that db.sqlite
exists. This should be done in the open_db test helper path in
crates/cartog/src/commands/shared.rs.
In `@crates/cartog/tests/consent_gate_test.rs`:
- Around line 103-115: The fresh-repo read-command test in
read_command_no_create_on_fresh_repo only checks exit status and that .cartog is
not created, so it should also assert the empty-index hint is returned. Update
this test to inspect the command output from sb.cmd(&["search", "foo"]) and
verify the user-facing empty-index guidance appears alongside the empty result,
using the existing stderr/stdout helpers and keeping the assertions tied to
read_command_no_create_on_fresh_repo.
In `@skills/cartog/scripts/ensure_indexed.sh`:
- Around line 356-368: The _find_cartog_toml helper only treats .git directories
as the repo boundary, so it can walk past git worktrees and submodules where
.git is a file. Update the boundary check in _find_cartog_toml to stop on either
a .git directory or a .git file before continuing upward, while keeping the
existing .cartog.toml lookup behavior intact.
In `@skills/cartog/SKILL.md`:
- Around line 44-46: The restart guidance is too unconditional and conflicts
with the degraded-server flow described in this section. Update the later “After
indexing…” sentence in the Cartog SKILL.md guidance to distinguish normal
attached-server behavior from the degraded case, and make it clear that only
servers already using the on-disk DB pick up new symbols on the next call while
a degraded MCP server still requires a relaunch to load the freshly built index.
---
Nitpick comments:
In `@crates/cartog-mcp/src/single_writer.rs`:
- Around line 150-160: The public entrypoint run_server is missing rustdoc,
including the new allow_create consent gate behavior. Add /// documentation
directly above run_server describing its purpose, the meaning of allow_create,
and how startup/watch behavior is degraded when creation is disallowed. Also
ensure any other public items in single_writer.rs follow the same ///
doc-comment requirement.
In `@crates/cartog-mcp/src/tests/degraded.rs`:
- Around line 116-128: This test only checks Database::open_existing and does
not cover the real allow_create=false startup path, so regressions in
CartogServer::new can slip through. Update the test to exercise
CartogServer::new with allow_create=false via the injectable test provider used
by degraded startup, and assert the server starts degraded without creating
.cartog/; if you keep the current DB-only check, rename the test to reflect that
narrower contract.
In `@crates/cartog-watch/src/lib.rs`:
- Around line 89-96: `WatchConfig::new` returns a fully constructed `Self` and
should not be silently ignored, so add `#[must_use]` to the constructor to match
the guideline for builder-like APIs. Apply the attribute directly to
`WatchConfig::new` in the `impl WatchConfig` block so callers are warned when
they drop the returned config.
In `@crates/cartog/src/commands/manage.rs`:
- Around line 345-355: Add rustdoc comments to the public cmd_watch function,
since it currently has no /// documentation. Describe the watch command’s
behavior and explicitly note how allow_create controls whether it may create
.cartog/ or remain consent-blocked/degraded, so the public API matches the
crate’s doc requirements.
In `@crates/cartog/src/config/load.rs`:
- Around line 344-346: Add #[must_use] to allow_index_creation in the config
load module so callers cannot accidentally ignore the consent-gate result;
update the function signature for allow_index_creation(db_path, config_present)
accordingly and keep its predicate logic unchanged.
🪄 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: 89eeb601-5698-4fec-8ce4-448356c968ae
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
AGENTS.mdcrates/cartog-db/src/lib.rscrates/cartog-db/src/store/lifecycle.rscrates/cartog-db/src/tests/errors.rscrates/cartog-mcp/src/lib.rscrates/cartog-mcp/src/single_writer.rscrates/cartog-mcp/src/tests/degraded.rscrates/cartog-mcp/src/tests/mod.rscrates/cartog-mcp/src/tests/schema.rscrates/cartog-mcp/src/tools/index.rscrates/cartog-mcp/src/tools/search.rscrates/cartog-mcp/src/types.rscrates/cartog-watch/Cargo.tomlcrates/cartog-watch/src/lib.rscrates/cartog/src/commands/doctor.rscrates/cartog/src/commands/index.rscrates/cartog/src/commands/manage.rscrates/cartog/src/commands/rag.rscrates/cartog/src/commands/shared.rscrates/cartog/src/config/load.rscrates/cartog/src/config/tests/load.rscrates/cartog/src/main.rscrates/cartog/tests/consent_gate_test.rsdocs/reference/cli.mddocs/reference/config.mddocs/troubleshooting.mddocs/usage.mdsite/src/pages/usage.astroskills/cartog/SKILL.mdskills/cartog/scripts/ensure_indexed.shskills/cartog/tests/test_ensure_indexed.sh
- CI: build_minimal_index ran `cartog index` on a config-less repo; the consent gate refused it. Set CARTOG_AUTO_INIT=1 (the documented config-less opt-in) for that fixture index. Fixes the red Coverage / Test-Remote jobs. - cartog-db: open_existing now does a true non-creating open (SQLITE_OPEN_READ_WRITE, no CREATE) so a file vanishing between any pre-check and the open errors as NotFound instead of materializing a fresh DB (closes a TOCTOU window). Shared post-open setup factored into setup_opened_conn. - doctor: the "database not found" hint now uses the full allow_index_creation predicate, so it suggests `cartog index` (not init) when CARTOG_AUTO_INIT alone already permits indexing. - hook: _find_cartog_toml stops on a `.git` file too (worktrees/submodules), matching the binary's local_config_path. - SKILL.md: clarified that only an attached (on-disk) MCP server picks up new symbols on the next call; a degraded server needs a relaunch. - docs: run_server / cmd_watch rustdoc note allow_create; SERVE_LOCK_SLOT documented; #[must_use] on allow_index_creation and WatchConfig::new. Tests: open_existing no-create-when-parent-dir-exists (TOCTOU); open_db existing-index reopen asserted via a sentinel symbol; fresh-repo read asserts the empty-index hint; degraded open_existing test renamed to its narrow contract (full path covered by the read-only-secondary test).
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 87.59% 88.03% +0.44%
==========================================
Files 96 96
Lines 31790 32200 +410
==========================================
+ Hits 27846 28347 +501
+ Misses 3944 3853 -91 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cartog no longer creates a
.cartog/index for a config-less project on its own. A fresh, config-less repo stays untouched until the user opts in. Consent is any one of:.cartog.toml(cartog init);.cartog/db.sqlite(or legacy.cartog.db) exists, the project is opted in and steady-state updates keep working;CARTOG_AUTO_INITset — indexes with in-memory defaults, writes no config file.A
Rejected(broken).cartog.tomlis not consent. An explicitcartog pullyou typed is consent (it's an explicit action, likeinit).This closes the gap where any command run on a config-less repo —
index,serve, even asearch— would materialize a.cartog/nobody asked for.Behavior when no consent signal holds
cartog index/rag index/watch.cartog/createdcartog serve/serve --watch(MCP plugin default).cartog/, read tools return empty + hint, write tools refuse,cartog_statsreports"degraded": true. With--watch, the watcher pre-builds the index the momentcartog initruns (or a DB /CARTOG_AUTO_INITappears); the running server stays degraded until relaunch (no live swap).cartog/createdinit/config/doctor/completions/manpage/self *pull/pushpullmaterializes.cartog/because that's its jobInstallation / editor wiring (
install,ide, the plugin MCP entry) are independent of the gate — binary + MCP config files only.Implementation
Database::open_existing(nocreate_dir_all;DbError::NotFoundwhen absent).allow_index_creation(db_path, config_present)+CARTOG_AUTO_INIT; evaluated once inmain.rs, threaded asallow_createintorun_server→WatchConfig/CartogServer(mirrorswalk_filter/redact).open_memory().refuse_if_degradedgates the 2 write tools,cartog_stats.degraded, staleness banners suppressed while degraded. A read-only secondary against an absent DB also starts degraded (so the second client in a multi-window setup never crashes).watcher_consentswalks cwd→git-root for.cartog.toml(subdir-launchedservestill pre-builds on a git-rootinit).cartog init).ensure_indexed.sh): gate = toml-walk-up OR db-exists ORCARTOG_AUTO_INIT.CARTOG_AUTO_INITenv var, serve-degraded note,push/pullclarification, SKILL.md fresh-repo rewrite.Review
Reviewed scenario-by-scenario (MCP no-config / legacy-DB / with-config / config-added-mid-session) and via an adversarial multi-angle code review. All 7 confirmed findings fixed (flaky test, 2nd-peer degraded attach, doctor hint, pull/docs contradiction, hook walk-up, degraded stale-banner) plus cleanups.
Tests
cartog-db
open_existing; config predicate (serialized env tests); CLI integration (refuse / no-create read / AUTO_INIT / after-init / existing-DB / degraded-serve contrast); MCP degraded (write-tool refusal, stats, read-only-secondary-against-absent-DB); watcher consent (.cartog.tomltrigger, git-root walk-up, stops-at-git-root); doctor consent-aware hint; hook db-exists + intermediate-dir branches.Gates:
cargo fmt, clippy (default +--no-default-features),cargo test --workspace+--no-default-features,make check-skill— all green. End-to-end verified with the real binary (refuse/init/AUTO_INIT/existing-DB, degraded serve + mid-session init pre-build, multi-peer degraded attach).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
CARTOG_AUTO_INIT.cartog servecan now start in a degraded mode when no index exists, still allowing reads and reporting status clearly.Bug Fixes
.cartog/unexpectedly for uninitialized projects.Documentation