Skip to content

fix(store): resolve path-identity and watcher races behind base CI red - #1148

Merged
ScriptedAlchemy merged 6 commits into
codex/tracedecay-total-redesign-plan-reopenedfrom
grok/ci-base-red-residual
Sep 9, 2026
Merged

ScriptedAlchemy merged 6 commits into
codex/tracedecay-total-redesign-plan-reopenedfrom
grok/ci-base-red-residual

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 9, 2026 •

Copy link
Copy Markdown
Owner

Production-side root causes behind the red on the #707 tip (9ca6759a8 run 34317967355). fable/test-reds already landed test-side isolation for the same symptoms; this PR carries only the defects that remain in shipped code, rebuilt on top of ea098cba8 with duplicates dropped.

  • fix(store): LocalProjectEnrollmentAuthorityV1 compared raw PathBufs, so macOS /var vs /private/var registered one project as two authorities (DuplicateProjectAuthority, dashboard graph tests). Roots are canonicalized before sort/dedup. Test: explicit tmp/link alias must equal the real root and must not be a duplicate authority.
  • fix(sessions): path_identity_key folds Windows display syntax only; Unix symlink aliases were stored byte-exact while scoped reads keyed on the canonical spelling → zero rows (dashboard analytics sessions_read 0 vs 5). Writes and the project-scope predicate now use durable_project_path_key; the analytics subagent tree matches both opened and canonical spellings. Test: link and real spellings produce one stored key.
  • fix(code-index): FSEvents can deliver callbacks after shutdown() joined the watcher; the callback still marked WatchState dirty. Classification, notify-failure marking, and pending reconciliation no-op once retired.
  • fix(runtime-core): fixture git also inherited GIT_INDEX_FILE / GIT_OBJECT_DIRECTORY; cleared alongside GIT_DIR, and fixture failures now surface stderr.

Dropped from the lane as superseded: the DaemonEngine cfg gate (landed as e9fd69741), the retained_project_store_db identity compare (landed as 7973458dc), the GitHub fixture blocking-socket fix (b0c40ee75), and an LSP initialize-write budget swap that the landed test-side fix made unnecessary.

  • fix(mcp): inline projectless Hermes replay reported a receipt the concurrent replay worker had already committed as accepted_for_replay, which the caller rejects (fail-if-flaky retargeted_client_profile_root_keeps_hermes_receipt_under_pinned_profile). When the target seq is absent from the spool, the replay now proves settlement through the spool watermark (replay.commit(seq) is Ok iff seq <= committed_through) and reports Committed; a never-committed seq stays the typed spool_ack_conflict. No ExactDuplicate fabrication. RED→GREEN test; projectless suite 5/5 ×3 under --test-threads=8; Hermes module 6/6.

Local: clippy -D warnings --tests on store-runtime, sessions, dashboard-api, code-index-runtime, runtime-core; enrollment alias test 1/1; durable path key test 1/1; worktree tests 15/15; git_watch shutdown and dashboard analytics tree runs in flight.

macOS /var and /private/var are one directory. Two registrations of the same project differing only by that alias were rejected as DuplicateProjectAuthority; roots are now canonicalized before sort/dedup. Falsifiable with an explicit tmp/link alias.
path_identity_key folds Windows display syntax only, so Unix symlink aliases (/var vs /private/var, tmp/link vs tmp/real) were stored byte-exact and scoped session reads keyed on the canonical spelling returned zero rows. Session writes and the project-scope predicate now go through durable_project_path_key; the analytics subagent tree matches both the opened and canonical spellings so rows written under either alias are one project.
FSEvents can deliver callbacks after shutdown() has joined the watcher; the callback still held WatchState and marked it dirty. Classification, notify-failure marking, and pending reconciliation now no-op once the state is retired.
GIT_INDEX_FILE and GIT_OBJECT_DIRECTORY retarget fixture git the same way GIT_DIR does; clear them too, and surface stderr when a fixture git command fails.
@changeset-bot

changeset-bot Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b970e92

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-09T10:08:29.700218Z 86bd169 PR opened
ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86bd169bad

ℹ️ 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".

query_params.push(Value::Text(project_selector.to_owned()));
let key_parameter = query_params.len();
query_params.push(Value::Text(path_identity_key(project_selector)));
query_params.push(Value::Text(durable_project_path_key(project_selector)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Match both path spellings in scoped session reads

On macOS for projects under /var, observation projection deliberately persists the public /var/... spelling while its project_key can be a typed project ID (canonical_projection.rs:65-84 and observation_projection/state.rs:1142-1169). This predicate now compares project_path only with durable_project_path_key(project_selector), which expands the selector to /private/var/..., so project-filtered counts and searches by path miss those sessions. Match both the opened and canonical spellings here, as the changed analytics route already does.

AGENTS.md reference: AGENTS.md:L130-L132

Useful? React with 👍 / 👎.

session.session_id.clone(),
session.project_key.clone(),
path_identity_key(&session.project_path),
durable_project_path_key(&session.project_path),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve non-path project sentinels on write

When the daemon's current directory contains a user directory, production profile-scoped sessions whose project_path is the sentinel "user" are canonicalized to an absolute <cwd>/user path here. This breaks the sentinel's byte-exact round trip and can cause Git-history backfill to treat a user-scoped session as belonging to the current repository; the same issue applies to "unknown" when such a child exists. Restrict filesystem canonicalization to established project paths and preserve non-path sentinels unchanged.

AGENTS.md reference: AGENTS.md:L162-L164

Useful? React with 👍 / 👎.

Inline replay must prove a missing target via the spool watermark instead of treating an empty spool as accepted_for_replay.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Performance Comparison codex/tracedecay-total-redesign-plan-reopened → grok/ci-base-red-residual

Total Elapsed Time: 4.03s → 4.39s (+8.9%)
CPU Baseline: 80.30µs → 79.29µs (-1.3%)
Benchmark ID: index-bench-timing

timing - Execution duration of functions.

+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| Function                                 | Calls                      | Avg                             | P95                             | Total                            | % Total                      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| tracedecay-index-bench                   | 1 → 1 (+0.0%)              | 4.03s → 4.39s (+8.9%)           | 4.03s → 4.40s (+9.2%)           | 4.03s → 4.39s (+8.9%)            | 100.00% → 100.00% (+0.0%)    |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.append_pages              | 13 → 13 (+0.0%)            | 124.72ms → 136.57ms (+9.5%)     | 163.58ms → 185.20ms (+13.2%)    | 1.62s → 1.78s (+9.9%)            | 40.19% → 40.41% (+0.5%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| code_index.workers.install               | 83 → 83 (+0.0%)            | 13.93ms → 17.54ms (+25.9%) ⚠️   | 46.96ms → 60.72ms (+29.3%) ⚠️   | 1.16s → 1.46s (+25.9%) ⚠️        | 28.66% → 33.13% (+15.6%)     |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| domain.canonical.sha256                  | 110414 → 110414 (+0.0%)    | 8.45µs → 9.95µs (+17.8%)        | 10.89µs → 10.94µs (+0.5%)       | 932.66ms → 1.10s (+17.9%)        | 23.12% → 25.00% (+8.1%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| code_index.build.and_publish             | 2 → 2 (+0.0%)              | 454.64ms → 512.27ms (+12.7%)    | 532.41ms → 590.35ms (+10.9%)    | 909.28ms → 1.02s (+12.2%)        | 22.54% → 23.32% (+3.5%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.batch.sqlite              | 13 → 13 (+0.0%)            | 70.79ms → 72.55ms (+2.5%)       | 93.52ms → 99.48ms (+6.4%)       | 920.32ms → 943.20ms (+2.5%)      | 22.81% → 21.47% (-5.9%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.batch.postings            | 13 → 13 (+0.0%)            | 62.20ms → 62.94ms (+1.2%)       | 84.02ms → 87.75ms (+4.4%)       | 808.56ms → 818.27ms (+1.2%)      | 20.04% → 18.62% (-7.1%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.prepare_pages             | 13 → 13 (+0.0%)            | 44.88ms → 53.63ms (+19.5%)      | 61.37ms → 75.04ms (+22.3%) ⚠️   | 583.44ms → 697.15ms (+19.5%)     | 14.46% → 15.87% (+9.8%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| code_index.chunk.index_file              | 276 → 276 (+0.0%)          | 2.10ms → 2.51ms (+19.5%)        | 3.15ms → 5.36ms (+70.2%) ⚠️     | 580.73ms → 693.75ms (+19.5%)     | 14.39% → 15.79% (+9.7%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| code_index.extract.parser_artifact       | 276 → 276 (+0.0%)          | 2.10ms → 2.43ms (+15.7%)        | 3.17ms → 5.38ms (+69.7%) ⚠️     | 578.34ms → 671.37ms (+16.1%)     | 14.33% → 15.28% (+6.6%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.finalization.advance_wake | 14 → 14 (+0.0%)            | 46.71ms → 46.80ms (+0.2%)       | 283.64ms → 286.26ms (+0.9%)     | 653.91ms → 655.20ms (+0.2%)      | 16.21% → 14.91% (-8.0%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.batch.parallel_prepare    | 13 → 13 (+0.0%)            | 36.02ms → 44.24ms (+22.8%) ⚠️   | 48.86ms → 64.19ms (+31.4%) ⚠️   | 468.32ms → 575.10ms (+22.8%) ⚠️  | 11.61% → 13.09% (+12.7%)     |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| query.artifact.batch.postings.ngram_rows | 13 → 13 (+0.0%)            | 35.00ms → 34.61ms (-1.1%)       | 47.74ms → 47.71ms (-0.1%)       | 454.97ms → 449.96ms (-1.1%)      | 11.28% → 10.24% (-9.2%)      |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| code_index.chunk.build                   | 276 → 276 (+0.0%)          | 1.35ms → 1.60ms (+18.5%)        | 2.00ms → 4.26ms (+113.0%) ⚠️    | 372.24ms → 440.46ms (+18.3%)     | 9.23% → 10.03% (+8.7%)       |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| 🆕 code_index.chunk.identify             | 0 → 276 (+100.0%) ⚠️       | 0.00ns → 1.58ms (+100.0%) ⚠️    | 0.00ns → 4.24ms (+100.0%) ⚠️    | 0.00ns → 435.89ms (+100.0%) ⚠️   | 0.00% → 9.92% (+100.0%) ⚠️   |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+
| 🗑️ code_index.build.assemble             | 2 → 0 (-100.0%) 🚀         | 203.13ms → 0.00ns (-100.0%) 🚀  | 210.76ms → 0.00ns (-100.0%) 🚀  | 406.26ms → 0.00ns (-100.0%) 🚀   | 10.07% → 0.00% (-100.0%) 🚀  |
+------------------------------------------+----------------------------+---------------------------------+---------------------------------+----------------------------------+------------------------------+

Generated with hotpath-rs

@ScriptedAlchemy
ScriptedAlchemy merged commit b9d96d0 into codex/tracedecay-total-redesign-plan-reopened Sep 9, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant