fix(ingest): tie each code graph to its root and prune graphs of deleted roots (#711, slice 1) by cdeust · Pull Request #869 · cdeust/Cortex

cdeust · 2026-11-04T22:28:12Z

Summary

Refs #711 (slice 1 of 4). ingest_codebase now ties each graph under ~/.cache/cortex/code-graphs/ to the project root it was built from, and removes the graphs whose root no longer exists.

Root cause. Nothing recorded which project a graph directory belongs to. The only cleanup, _prune_precedent_graphs, matched siblings by name prefix: a deleted worktree's graph (fix-665-windows-registry-44d852f6) never matched the prefix of a session that was still running, so it stayed on disk forever. The same rule also deleted unrelated projects: for cortex-ab12cd34 the prefix is cortex-, which matches cortex-viz-*.

Change.

New mcp_server/infrastructure/code_graph_cache.py: write_root_marker writes cortex-root.json ({"project_root": <resolved path>}, written atomically) and prune_orphan_graphs removes each graph whose recorded root is gone. A directory with no readable marker is logged as unattributed and kept, never deleted on a guess.
ingest_codebase writes the marker into output_dir once a graph exists there, then prunes the default cache. The name-prefix prune is removed.
Why a separate marker and not meta.json: meta.json is written by automatised-pipeline (ADR-0052, "for cortex-viz"), and its schema is not defined in this repository. Cortex does not read or rewrite a file owned by another tool.

Remaining slices of #711 (not in this PR):
2. Bound the total size, with an eviction rule that never touches a graph in use.
3. Prune when a worktree is removed (hook), not only on the next ingest.
4. Let a worktree session reuse the parent project's current graph, or ingest it incrementally, instead of building a full one.

The ~43 existing directories with no marker are reported, not migrated. Tying them to a root would mean guessing from meta.json, which is why it needs an owner decision.

Type of change

Bug fix (non-breaking change which fixes an issue)

Test plan

New tests_py/infrastructure/test_code_graph_cache.py, 7 passed. It covers: deleted root → pruned; live root → kept; no marker → reported and kept (the warning names the directory); unreadable marker → unattributed; cortex / cortex-viz prefix collision → untouched; missing cache dir → empty report.
Run file by file, all pass: tests_py/test_ingest_progress.py 14 passed, tests_py/handlers/test_ingest_codebase.py 16 passed and 2 skipped, tests_py/hooks/test_ingest_codebase_background.py 3 passed, tests_py/scripts/test_composition_root_wiring_coverage.py 6 passed, tests_py/infrastructure/test_pipeline_discovery.py 30 passed, tests_py/scripts/test_craftsmanship_layer_table.py 12 passed.
ruff check + ruff format --check on the changed files (system ruff and the uv.lock version): clean. python scripts/check_craftsmanship.py: OK. python scripts/check_doc_claims.py: OK.
Pre-existing issue, not introduced here: test_ingest_codebase.py fails 8 tests when it runs in the same process after test_ingest_progress.py. It is identical on origin/main (stash check: 8 failed there too).

Not verified: the full pytest suite (the local env was installed with uv pip, not uv sync --locked, because the proxy blocked the torch CPU index) and a real end-to-end ingest against an automatised-pipeline binary. CI is the receipt.

Audit notes

Layers: handlers → infrastructure, and infrastructure imports stdlib only. The craftsmanship gate passes.
No new numeric constant.

Breaking changes

Graph directories that share a name prefix with the current project are no longer deleted. A legacy directory with no marker is now reported in the log, not removed.

Reviewer checklist

CHANGELOG.md not updated (left to the release).
docs/module-inventory.md lists the new module.
No secrets / credentials / PII in the diff.

Generated by Claude Code

…ted roots (#711, slice 1) ingest_codebase now writes cortex-root.json (the resolved project root) next to the graph it builds, and prunes every graph under ~/.cache/cortex/code-graphs whose recorded root no longer exists. A directory without the marker is logged as unattributed and kept, never deleted on a guess. This replaces the name-prefix prune, which removed any sibling sharing the project name's first segment (cortex-viz-* when cortex ran) and never removed the graph of a deleted worktree, whose name differs. Refs #711 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYt8uiGG3xcmvaug2eG

…of deleted roots (#711, slice 1) Route the root marker through gated_files.write_json (ADR-1110). Check unit, run 41925545304: test_storage_redaction_gate found a raw write_text in infrastructure/code_graph_cache.py. A marker altered by redaction is removed, so the graph stays unattributed instead of being pruned. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYt8uiGG3xcmvaug2eG

cdeust · 2026-11-04T22:41:50Z

unit, run 41925545304: failure caused by this PR, fixed in 5ed7909 (fixup! of 46eb2ad).

tests_py/architecture/test_storage_redaction_gate.py requires every file write under mcp_server/ to go through shared/gated_files (ADR-1110). code_graph_cache.write_root_marker called write_text directly. It now uses gated_files.write_json. Redaction could alter the path, and the graph would then look orphaned and be deleted; to prevent that, the marker is removed and the graph stays unattributed, which means kept (new test).

Run locally, file by file: test_storage_redaction_gate.py 6 passed, test_code_graph_cache.py 8 passed, test_ingest_progress.py 14 passed, test_ingest_codebase.py 16 passed and 2 skipped. ruff and the craftsmanship gate are clean. The full suite was not rerun locally: the run on 46eb2ad had only this failure (1 failed, 11526 passed). The CI for 5ed7909 is the receipt.

Generated by Claude Code

chatgpt-codex-connector · 2026-11-05T15:04:32Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T16:05:19.503834Z	`551041a`	New commits
🔒 Security Review	✅ Completed 2026-11-05T16:07:59.005950Z	`551041a`	New commits

Security findings

Advisory findings (1)

🟡 Restrict pruning to the managed graph cache · Medium

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-11-05T15:04:57Z

Claude finished @cdeust's task in 2m 2s —— View job

Review of PR #869: review 1 of 3 (ADR-1119)

Read the diff and the repository rules
Ran five reviews: code quality, security, performance, test coverage, documentation accuracy
Posted the review: fix(ingest): tie each code graph to its root and prune graphs of deleted roots (#711, slice 1) #869 (comment)

There are no Blockers. Three Notes are posted under "Notes, no action before merge":

In code_graph_cache.py:87, Path(root).exists() is not guarded. An unreadable root can make the ingest fail after its graph is built, and on Python 3.14 that root, or one on an unmounted volume, is treated as missing and its graph is deleted.
The handler tests reach the real cache prune. The old prune did the same.
The ADR-0399 wiki page is stale, and the new docs/module-inventory.md entry sits in the middle of the managed_pg_* block.

Verdict: approve

claude · 2026-11-05T15:07:04Z

Review 1 of 3 (ADR-1119): PR #869, commit `5ed7909`

I ran five reviews: code quality, security, performance, test coverage and documentation accuracy. None found a Blocker.

Things I checked and found sound:

rmtree cannot reach outside the cache. It refuses a symlinked entry, and that OSError is caught.
The marker goes through gated_files.write_json, so it passes the redaction gate.
The redaction gate only removes secrets. An ordinary project path is written unchanged, so markers will not be dropped in normal use.
The layers are handlers → infrastructure → shared.
ROOT_MARKER is a string, and the craftsmanship gate only checks numeric constants.
check_doc_claims is not affected.

Notes, no action before merge

Path(root).exists() at code_graph_cache.py:87 is not guarded, and neither is the call at ingest_codebase.py:491. On Python 3.11 and 3.12, a marker naming a root the user cannot read, or containing a NUL byte, raises an error, and the ingest fails even though its graph was already built. On 3.14 the same root, or one on an unmounted volume, reads as "missing" and the graph is deleted. Consider calling os.stat, pruning only on FileNotFoundError, and logging any other error.
The handler tests in tests_py/handlers/test_ingest_codebase.py reach the real prune of ~/.cache/cortex/code-graphs, because HOME is not isolated in tests. The old prefix prune did the same, so this is not new. An autouse fixture that points default_cache_dir at tmp_path would keep pytest away from a developer's cache.
The ADR-0399 wiki page (wiki/adr/migration/0399-…) still describes _prune_precedent_graphs as the current behaviour, and the new entry in docs/module-inventory.md:263 sits in the middle of the managed_pg_* block.

Verdict: approve

cdeust · 2026-11-05T15:07:19Z

The three notes from review 1 are left out of this PR, following ADR-1119: a Note does not justify a push.

An unguarded Path(root).exists(): an unreadable or unmounted root should be tested with os.stat, and only FileNotFoundError should lead to a prune. This goes into slice 2 of The code-graphs cache grows without bound and is never pruned when a worktree is removed #711 (bounded size and eviction), which rewrites the eviction rule anyway.
The real ~/.cache/cortex/code-graphs is reached from the handler tests. This predates the PR, as the review notes. An autouse fixture on default_cache_dir will go into the same slice.
The ADR-0399 wiki page and the position of the inventory entry: documentation, to fix in the next slice.

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ed7909d7c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-05T15:07:27Z


-    # source: ADR-0399
-    _prune_precedent_graphs(output_dir)
+    _record_and_prune_graphs(project_path, output_dir)


Record ownership at the actual graph location

When ensure_graph reuses a memoised graph, it returns the cached path without using the requested output_dir (ingest_codebase_graph.py, cached-return branch). Recording ownership against output_dir therefore attributes a directory that was not necessarily used: the actual graph can remain unmarked, or an existing graph belonging to another project can have its marker overwritten. In the latter case, deleting the newly recorded project root causes a later pruning pass to delete the other project's graph even though its root still exists. Pass the returned graph_path into the ownership-recording step and identify the directory containing that graph instead of assuming the requested output directory was used.

Useful? React with 👍 / 👎.

Traced on 5ed7909. In the default path I did not find a case where another project's graph gets deleted:

With no output_dir argument, output_dir is default_cache_dir()/project_key(project_path). The key hashes the resolved path, so this directory can only belong to this project. If a marker is written there, it names the right root.

In the cached branch, find_cached_graph only returns graphs memoised under this project's tag (code_graph_tag(project_key)). If that graph lives elsewhere, the default output_dir usually does not exist, the is_dir() check writes nothing, and the real graph stays unattributed, which means kept. That is the conservative case, not a deletion.

The scenario you describe needs a caller who explicitly passes as output_dir the cache directory of another project, while this project's cached graph is reused. Without the cache, the same argument already makes automatised-pipeline overwrite the other graph, so this is the explicit argument's own risk, not a new one.

The fix is still right in principle: mark the directory of the graph_path that is actually used. Under ADR-1119 this is not a Blocker (no CI, security or non-negotiable rule is broken in the default path), so it goes into the next slice of #711 rather than a push now. I am leaving the thread open for the owner's call.

Generated by Claude Code

Addressed in 456b038. The marker is no longer written by the handler after ensure_graph. It is written in _call_analyze, right after upstream has built the graph in the output_dir it was given. A reused graph is never re-marked, so another project's marker can no longer be overwritten. Test: tests_py/handlers/test_ingest_codebase_graph_cache.py::test_analyze_call_records_the_root_of_the_graph_it_builds.

Generated by Claude Code

…the NullProgress test test_null_progress_does_not_alter_handler_result assigned icb.graphmod.ensure_graph and icb.cypher.fetch_files directly, so the real ingest_codebase_graph and ingest_codebase_cypher modules kept the fakes for the rest of the session. Run before test_ingest_codebase.py (the order mutmut's clean run uses), 8 handler tests failed, and the diff-scoped mutation gate stopped with "Failed to run clean test" (check `unit`, run 42051227576). The test now swaps the module references through monkeypatch, as the stage-ordering test above does. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYs2Ko7tuSGHN7Qfbf7

…of deleted roots (#711, slice 1)

cdeust · 2026-11-05T15:43:03Z

unit (et l'agrégat merge), run 42051227576 : gate de mutation diff-scoped (ADR-1122), deux causes, poussé en 1922fb6.

Clean run de mutmut en échec, défaut préexistant sur main révélé par cette PR. test_null_progress_does_not_alter_handler_result (tests_py/test_ingest_progress.py) assignait directement icb.graphmod.ensure_graph et icb.cypher.fetch_files, donc les vrais modules ingest_codebase_graph / ingest_codebase_cypher gardaient les faux pour toute la session. Reproduit hors bac à sable sur origin/main : pytest tests_py/test_ingest_progress.py tests_py/handlers/test_ingest_codebase.py → 8 échecs ('/tmp/graph' == …existing-graph, assert 2 == 3) ; dans l'ordre inverse, tout passe. Correctif db4953b : le test remplace les références de module via monkeypatch, comme le test d'ordre des étapes juste au-dessus.
9 mutants survivants dans code_graph_cache.default_cache_dir (aucun test ne figeait le chemin). Correctif 1922fb6 (fixup! de 46eb2ad) : test_default_cache_dir_is_under_the_user_cache fige Path.home()/.cache/cortex/code-graphs.

Vérifié localement : l'ordre fautif passe (39 passed, 2 skipped sur les trois fichiers de test) ; mutmut run limité à x_default_cache_dir__mutmut_* → 9/9 killed ; le clean run de scripts/mutation_check.sh sur ingest_codebase.py passe désormais et les mutants s'exécutent ; ruff 0.17.3 et check_craftsmanship.py OK sur les fichiers touchés.
Non vérifié : le verdict du gate sur les 3 fonctions modifiées d'ingest_codebase.py (le CI ne l'avait jamais atteint ; mon run local mute le fichier entier et sa re-vérification des survivants diverge du CI, donc pas de conclusion). Aucun check vert annoncé avant le run de 1922fb6.

Note : la branche a 14 commits de retard sur main ; le rebase n'a pas pu être poussé depuis ce passage de l'autopilot (push --force-with-lease refusé par la politique de permissions de la session).

Generated by Claude Code

claude · 2026-11-05T15:43:05Z

Claude finished @cdeust's task in 1m 51s —— View job

Review 2 of 3 (ADR-1119): PR #869

Check the Blockers from review 1 (it had none)
Find the commits pushed after 5ed7909: db4953b and 1922fb6 (tests only, +7 lines)
Ran five reviews: code quality, security, performance, test coverage and documentation accuracy. None found a Blocker.
Posted the review: fix(ingest): tie each code graph to its root and prune graphs of deleted roots (#711, slice 1) #869 (comment)

What the two commits do:

db4953b stops test_ingest_progress.py from overwriting functions in the real graphmod and cypher modules. That leak is why 8 tests in test_ingest_codebase.py failed when they ran after this file.
1922fb6 adds a correct test for default_cache_dir().

I could not run pytest or ruff in this job because those commands were not allowed. The Pipeline run was still going when I posted the review.

Verdict: approve

claude · 2026-11-05T15:44:56Z

Review 2 of 3 (ADR-1119): PR #869, commits after `5ed7909`

Blockers from review 1: none were raised, so nothing needed fixing.

Scope: only the commits pushed after 5ed7909, db4953b and 1922fb6. Both change tests only (+7 lines). I did not re-audit unchanged code, including the code behind the open Codex thread on ingest_codebase.py:491.

I ran five reviews: code quality, security, performance, test coverage and documentation accuracy. None found a Blocker.

What I traced:

db4953b (tests_py/test_ingest_progress.py:317,323): icb.graphmod and icb.cypher are now swapped for a MagicMock through monkeypatch.setattr before the fakes are assigned. Before, the test overwrote ensure_graph and fetch_files on the real modules and never put them back. tests_py/handlers/test_ingest_codebase.py uses those real functions, which explains its 8 failures when it runs after this file. Every direct assignment left in the file now lands on a mock that monkeypatch restores. With the test's stubs, the handler awaits no other mock attribute, and _record_and_prune_graphs is stubbed, so the real cache is not touched.
1922fb6 (tests_py/infrastructure/test_code_graph_cache.py:21): Path.home is patched with a classmethod that returns tmp_path, and monkeypatch restores it. The assertion matches default_cache_dir() exactly, and the test creates nothing on disk.

Not verified here: I could not run the tests or ruff locally, because the job did not allow those commands. The Pipeline run for 1922fb6 was still running when I posted this.

Verdict: approve

…of deleted roots (#711, slice 1) Check unit, run 42051227576 (ADR-1122 mutation gate). Keep handler() byte-identical to main, so the gate does not charge this slice with the 182 pre-existing survivors of the whole handler. The marker is now written in _call_analyze (the graph Cortex asks upstream to build, which also answers the Codex P2 finding about the reused-graph path). _prune_precedent_graphs keeps its call site and prunes orphans beside output_dir. default_cache_dir is gone (no caller). Tests pin the marker, prune and log behaviour. Local ci_mutation_diff.py: 3 file(s), 0 failed, 0 untested. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYt8uiGG3xcmvaug2eG

cdeust · 2026-11-05T15:48:37Z

unit, run 42051227576: failure caused by this PR, fixed in 456b038 (fixup! of 46eb2ad, on top of db4953b and 1922fb6 from another session).

Merged with main, the ADR-1122 mutation gate found three problems:

In default_cache_dir, no test killed the mutants.
The clean run of ingest_codebase.py failed because of a test leak in test_ingest_progress.py, which db4953b fixed.
The changed line in handler() charged this slice with the whole function: 182 survivors that predate the PR (measured locally).

So handler() is again identical to main:

The marker is written in ingest_codebase_graph._call_analyze, where Cortex asks upstream to build the graph in output_dir. That location is the one actually used, which also answers the Codex P2 comment.
_prune_precedent_graphs(output_dir) keeps its call site and prunes the orphans beside output_dir.
default_cache_dir is removed, because nothing calls it any more.
Tests pin the marker, the prune and the log messages.

Run locally: GITHUB_BASE_REF=main python scripts/ci_mutation_diff.py → 3 file(s), 0 failed, 0 untested. The four touched test files pass in both orders (50 passed, 2 skipped). test_storage_redaction_gate.py passes (6), and ruff and the craftsmanship gate are clean. The full suite was not rerun locally; the CI for 456b038 is the receipt.

Generated by Claude Code

claude · 2026-11-05T15:49:01Z

Claude finished @cdeust's task in 2m 13s —— View job

Review 3 of 3 (ADR-1119): PR #869

Check the Blockers from reviews 1 and 2 (they had none)
Find the commits pushed after 1922fb6: 456b038 only
Ran five reviews on 456b038: code quality, security, performance, test coverage and documentation accuracy. None found a Blocker.
Posted the review: fix(ingest): tie each code graph to its root and prune graphs of deleted roots (#711, slice 1) #869 (comment)

What 456b038 changes:

The marker is now written in _call_analyze. That fixes Codex P2: a reused cached graph gets no marker, so it is kept.
The prune scans the parent of output_dir, the same place main scanned. It removes a sibling only when that sibling's marker names a deleted root.
default_cache_dir and _record_and_prune_graphs are gone, and nothing references them any more.

Accepted: the whole PR. No file or commit carries an open Blocker.

Not run: ruff and pytest, because this job did not allow those commands. The Pipeline run for 456b038 was still running when I posted the review.

Verdict: approve

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 456b038fdd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-05T15:50:22Z

            "language": language,
        },
    )
+    code_graph_cache.record_root(output_dir, project_path)


Record ownership only after analysis succeeds

When an explicit output_dir already contains another project's graph and upstream returns a status: error payload without replacing it, this call still overwrites the existing ownership marker. ensure_graph checks the error only afterward, so the failed ingest leaves the original graph attributed to the wrong root. Removing that root then causes a later pruning pass to delete the original graph even while its actual project still exists. Normalize and validate the analysis response before recording ownership, preserving the existing marker on failure.

Useful? React with 👍 / 👎.

The trace is correct, but only on the explicit path. For the marker to name the wrong root, a caller must pass as output_dir a directory that holds another project's graph, and upstream must fail without replacing it. In the default path, output_dir is code-graphs/<project_key(project_path)>, so a marker written after a failure still names the right root.

The fix is to record ownership only after ensure_graph has validated the response. That moves the write into ensure_graph, and under ADR-1122 this PR would then own all of ensure_graph's mutants. This is the third and last review (ADR-1119), so the change goes into the follow-up slice of #711 (a refinement PR with its own budget) together with the guarded os.stat from review 1. Thread left open for the owner.

Generated by Claude Code

claude · 2026-11-05T15:51:14Z

Review 3 of 3 (ADR-1119): PR #869, commit `456b038`

Blockers from reviews 1 and 2: none were raised, so nothing needed fixing.

Scope: only 456b038, the one commit pushed after 1922fb6. I did not re-audit unchanged code.

I ran five reviews: code quality, security, performance, test coverage and documentation accuracy. None found a Blocker.

What I traced in 456b038:

Codex P2 is fixed. The marker is now written in _call_analyze (ingest_codebase_graph.py:55), so only a directory that Cortex has just asked upstream to build gets one. The cached-reuse branch writes nothing, so a reused graph stays unattributed, which means it is kept.
The prune deletes less than main did. _prune_precedent_graphs keeps its call site and still scans the parent of output_dir, as on main. It now removes a sibling only when that sibling has a cortex-root.json naming a root that no longer exists. The old code deleted siblings by name prefix with no marker. rmtree refuses a symlinked entry, and that OSError is caught.
Errors are contained. gated_files.write_json only raises OSError, which record_root catches. marker.unlink() without missing_ok is safe because the file was just written, and any OSError it raises is caught too.
Nothing still uses the removed names. default_cache_dir and _record_and_prune_graphs have no references left in mcp_server/, tests_py/, docs/ or wiki/. The only default_cache_dir hits are flashrank's, an unrelated setting.
Layers and size limits hold. The direction is handlers → infrastructure → shared, and every file and method is under its limit.
The tests should pass as written. I compared each log string, the sorted order, PruneReport equality and the resolved call_upstream args against the code. @pytest.mark.asyncio is enough in strict mode.

Notes, no action before merge

Path(root).exists() (code_graph_cache.py:94) still treats an unreadable or unmounted root as deleted, and the graph is then pruned. Review 1 raised this and it is planned for slice 2.
record_root runs before the upstream error status is checked (ingest_codebase_graph.py:55), so a failed build still gets a marker. This is harmless because its root exists.
The # source: ADR-0399 comment at ingest_codebase.py:490 still cites the name-prefix rule, which The code-graphs cache grows without bound and is never pruned when a worktree is removed #711 replaced.

Accepted: the whole PR, all 5 commits and 7 files. No file or commit carries an open Blocker.

Not verified: I could not run ruff or pytest because this job does not allow those commands. The Pipeline run for 456b038 (42058709581) was still running when I posted this, so it is the receipt for the ADR-1122 mutation gate.

View job run

Verdict: approve

chatgpt-codex-connector

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 456b038fdd

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector · 2026-11-05T15:55:46Z

+        elif not Path(root).exists():
+            try:
+                shutil.rmtree(entry)


🛡️ Codex Security Review · Automatically triggered

Security: Restrict pruning to the managed graph cache

Required conditions: the codebase upstream is enabled and a local MCP caller (including a content-influenced agent) can make two calls with chosen paths. _call_analyze writes cortex-root.json even after a returned status:error, so the first call can mark any existing user-writable directory using a nonexistent project_path; a later successful call with a sibling output_dir makes this loop trust the marker and rmtree that directory. Restrict pruning to the canonical cache, reject non-directory roots, and record only validated successful graph outputs. Cortex runs with the user's filesystem permissions.

SECURITY.md reference: SECURITY.md:L5-L16

Useful? React with 👍 / 👎.

Fixed in 551041a. The trace is right: a failed analyze wrote a marker in any output_dir with a nonexistent root, and a second call with a sibling output_dir would then rmtree that directory. Two guards:

_prune_precedent_graphs prunes only when the parent of the resolved output_dir is the managed cache ~/.cache/cortex/code-graphs. An explicit output_dir elsewhere never deletes its siblings.

record_root writes no marker when project_root is not an existing directory, so a failed call on a nonexistent path leaves no orphan marker behind.

Tests: test_prune_never_touches_siblings_outside_the_managed_cache and test_record_root_refuses_a_root_that_is_not_a_directory. Local ci_mutation_diff.py (main's scripts): 3 file(s), 0 failed, 0 untested.

Process note: GitHub lost the push event, so the PR stayed on 456b038. I closed and reopened the PR to resync the head (autopilot rule 10).

Generated by Claude Code

…of deleted roots (#711, slice 1) Codex security review on 456b038 (P2, Medium): a failed analyze could mark any user-writable output_dir with a nonexistent root, and a second call with a sibling output_dir would then rmtree it. Pruning is now limited to the managed cache (~/.cache/cortex/code-graphs), and no marker is written for a root that is not an existing directory. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYt8uiGG3xcmvaug2eG

cdeust · 2026-11-05T17:25:36Z

CI autopilot : branche en retard de 19 commits sur main (8ef4813), rebase prêt mais non poussé.

Le rebase local sur origin/main avec autosquash passe sans conflit (6 commits → 2 : d4ac833 fix(ingest) et 5707aee fix(tests)) ; les tests touchés passent (tests_py/handlers/test_ingest_codebase_graph_cache.py, tests_py/infrastructure/test_code_graph_cache.py, tests_py/test_ingest_progress.py : 38 passed), ruff check/ruff format --check sur les fichiers modifiés et scripts/check_craftsmanship.py : OK.
Le push --force-with-lease=fix/711-code-graph-orphan-prune:551041a a été refusé par la politique de permissions de l'environnement de l'autopilot (pas par GitHub) ; la branche reste à 551041a.
À faire par un humain : git fetch origin && git checkout fix/711-code-graph-orphan-prune && GIT_SEQUENCE_EDITOR=true git rebase -i --autosquash origin/main && git push --force-with-lease, ou autoriser ce push pour la routine. Pas de hold posé.

Generated by Claude Code

…ests nothing (#711, slice 2) Slice 1 (#869) pruned graphs whose recorded root is gone, but only after ingest_codebase had built a graph. A session with a fresh graph, no pipeline or a failed analyze left every deleted worktree's graph (0.6 to 1.1 GB each, measured 2026-10-10) on disk. The SessionStart hook now reads the cortex-root.json markers of the managed cache (~/.cache/cortex/code-graphs) and, when at least one names a root that no longer exists, starts the detached code_graph_prune_background worker, so the removal never runs inside the hook's time budget. A directory without a marker is still reported and kept. Refs #711 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYsdTexaL2aS5FFzdeC

cdeust mentioned this pull request Nov 4, 2026

The code-graphs cache grows without bound and is never pruned when a worktree is removed #711

Open

cdeust marked this pull request as ready for review November 5, 2026 15:04

chatgpt-codex-connector Bot reviewed Nov 5, 2026

View reviewed changes

claude added 2 commits November 5, 2026 15:42

fixup! fix(ingest): tie each code graph to its root and prune graphs …

1922fb6

…of deleted roots (#711, slice 1)

chatgpt-codex-connector Bot reviewed Nov 5, 2026

View reviewed changes

cdeust closed this Nov 5, 2026

cdeust reopened this Nov 5, 2026

cdeust merged commit 17378cd into main Nov 5, 2026
18 checks passed

cdeust deleted the fix/711-code-graph-orphan-prune branch November 5, 2026 20:33

cdeust mentioned this pull request Nov 5, 2026

fix(hooks): SessionStart prunes orphaned code graphs even when it ingests nothing (#711, slice 2) #897

Merged

5 tasks done

Conversation

cdeust commented Nov 4, 2026

Summary

Type of change

Test plan

Audit notes

Breaking changes

Reviewer checklist

Uh oh!

cdeust commented Nov 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Security findings

Advisory findings (1)

Uh oh!

claude Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review of PR #869: review 1 of 3 (ADR-1119)

Uh oh!

claude Bot commented Nov 5, 2026

Review 1 of 3 (ADR-1119): PR #869, commit 5ed7909

Uh oh!

cdeust commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

cdeust Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

cdeust Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

cdeust commented Nov 5, 2026

Uh oh!

claude Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review 2 of 3 (ADR-1119): PR #869

Uh oh!

claude Bot commented Nov 5, 2026

Review 2 of 3 (ADR-1119): PR #869, commits after 5ed7909

Uh oh!

cdeust commented Nov 5, 2026

Uh oh!

claude Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review 3 of 3 (ADR-1119): PR #869

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

cdeust Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Nov 5, 2026

Review 3 of 3 (ADR-1119): PR #869, commit 456b038

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

🛡️ Codex Security Review · Automatically triggered

Uh oh!

chatgpt-codex-connector Bot Nov 5, 2026

Choose a reason for hiding this comment

🛡️ Codex Security Review · Automatically triggered

Uh oh!

cdeust Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

cdeust commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 5, 2026 •

edited

Loading

claude Bot commented Nov 5, 2026 •

edited

Loading

Review 1 of 3 (ADR-1119): PR #869, commit `5ed7909`

claude Bot commented Nov 5, 2026 •

edited

Loading

Review 2 of 3 (ADR-1119): PR #869, commits after `5ed7909`

claude Bot commented Nov 5, 2026 •

edited

Loading

Review 3 of 3 (ADR-1119): PR #869, commit `456b038`