Skip to content

feat(agent-context): refresh Agent Mode project context when included folder contents change#2658

Open
Emt-lin wants to merge 15 commits into
v4-previewfrom
issue-208-context-staleness
Open

feat(agent-context): refresh Agent Mode project context when included folder contents change#2658
Emt-lin wants to merge 15 commits into
v4-previewfrom
issue-208-context-staleness

Conversation

@Emt-lin

@Emt-lin Emt-lin commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Closes logancyang/obsidian-copilot-preview#208 (both confirmed repros).

Problem

Agent Mode captured a project's context once per session: the manifest rode only the first prompt, and dirty tracking hashed config fields only. A file added to an included folder stayed invisible to ongoing conversations (Repro A, .md) and to re-entered projects reusing the pre-created empty landing (Repro B, PDF — no snapshot in the manifest). Both repros confirmed via frame logs.

What this does

Detection foundation

  • ProjectContentTracker (host layer): vault watcher with a batch debounce (Set semantics — bulk bursts keep every path; same-path delete→re-create resolves as a replace, not a delete), per-project monotonic content epoch + capped change log, metadataCache tag source with conservative dirtying. Live create/modify match via the shared resolveAffectedProjects (extracted to searchUtils; Chat mode's watcher routes through it, behavior identical); delete/rename/folder-rename match path-based (rename-out invalidates via oldPath; markdown/folder deletes conservatively dirty tag-pattern projects).
  • Composite dirty key (configSignature + "#" + epoch) with a guarded clear (stored key must equal the kickoff-captured key AND its config part must match what the run actually read) — a second event landing mid-materialization can never be wiped.
  • Revision-aware materializer single-flight: a content-dirty caller supersedes (never joins) a run that started before the file existed.

Delivery, three cases

  • Case 1 — ongoing conversations (non-blocking): each send flushes the debounce, compares the session's lastDeliveredEpoch (advanced only after the backend accepts; fan-out never advances), and injects a wire-only <project_context_updates> block — changed PATHS only, never contents. Text changes carry a re-enumerate nudge; binaries ride a probe-classified status (pending → raw path now + background warm, snapshot pointer on a later turn; failed → announced once; deleted/renamed-out → "old snapshot is invalid"). Binary tracking-set mutations are staged and applied only on backend accept, so a thrown prompt re-announces.
  • Case 2 — in-place first send on a stale empty landing (blocking): the composer's send handler consults holdActiveSessionForFreshContext (flushes the debounce first, so "add a file, send immediately" holds), which re-materializes via the existing queue-and-hold and refreshes the landing's manifest before the queued send flushes. A failed materialization keeps the stale-but-usable manifest, leaves the delivery cursor behind, and arms the hold at most once per dirty revision (no flush→re-hold loop).
  • Case 3 — re-enter: content events now reach isProjectContextDirty, so a stale landing is not reused — a fresh session's first manifest includes the new sources.
  • Resumed sessions keep the projectContextBlock their resume path already materializes (previously discarded) and deliver it on the first send with a re-check preamble.

Testing

  • ~4,460 unit tests green (+~100 new across tracker/matcher/builder/probe/manager/session/composer), npm run format / npm run lint clean.
  • Validated end-to-end in a live vault with frame-log evidence: natural-question .md discovery, natural-question PDF via snapshot, rename-out (no repeated announcements), forced-failure raw-PDF fallback.

Known limitations / follow-ups

Recorded in designdocs/agent-projects/AGENT_PROJECT_CONTEXT_STALENESS.md (ships with this PR), notably: change-log compaction renders a generic re-enumerate nudge; ongoing config-change deltas are a generic re-check nudge; Case 2 covers content staleness only (config-root / system-prompt capture mismatches are a designed landing-replacement follow-up); remote same-URL content changes stay stale by design; tag-pattern projects are conservatively dirtied on any markdown re-index — precision follow-up filed as logancyang/obsidian-copilot-preview#210 (which also fixes Chat mode's existing tag-removal cache miss).

@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: f54dc4081c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/projectContentTracker.ts Outdated
@Emt-lin

Emt-lin commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 455a40c2be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/contextCacheProbe.ts
@Emt-lin

Emt-lin commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 3849304571

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/contextUpdatesBlockBuilder.ts
Comment thread src/agentMode/session/AgentSessionManager.ts Outdated
@Emt-lin

Emt-lin commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: e94883aaac

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/projectContentMatch.ts
@Emt-lin

Emt-lin commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: d7a781ae30

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agentMode/session/AgentSessionManager.ts
Comment thread src/context/contextCacheStore.ts Outdated
@Emt-lin

Emt-lin commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 6811708c65

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agentMode/session/AgentSessionManager.ts Outdated
@Emt-lin

Emt-lin commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 210969fed4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@Emt-lin Emt-lin requested a review from logancyang July 4, 2026 09:26
Emt-lin added 7 commits July 5, 2026 11:57
… folder contents change

Agent Mode captured a project's context once per session: the manifest rode
only the first prompt, and dirty tracking hashed config fields only — so a
file added to an included folder stayed invisible to ongoing conversations
and re-entered projects (obsidian-copilot-preview#208, both repros confirmed
via frame logs).

Detection foundation:
- ProjectContentTracker (host layer): vault watcher with a batch debounce
  (Set semantics — bulk bursts keep every path), per-project monotonic
  content epoch + capped change log, metadataCache tag source with
  conservative dirtying. Live create/modify match via the shared
  resolveAffectedProjects (extracted to searchUtils; Chat mode's watcher
  routes through it, behavior identical); delete/rename/folder-rename match
  path-based via projectContentMatch (rename-out invalidates via oldPath).
- Composite dirty key (configSignature + "#" + epoch): content events bump
  only the epoch, so a second event mid-materialization yields a distinct
  key the guarded clear can't wipe. The clear also requires the key's
  config part to match the config the run actually read (joined stale
  single-flight runs never clear).
- Revision-aware materializer single-flight: ensureProjectContextMaterialized
  takes a revisionKey; the in-flight join compares revision keys, so a
  content-dirty caller supersedes (never joins) a run that started before
  the file existed.

Delivery, three cases:
- Case 1 (ongoing conversations, non-blocking): each send flushes the
  debounce, compares the session's lastDeliveredEpoch (advanced only after
  the backend accepts; fan-out never advances), and injects a wire-only
  <project_context_updates> block on both assembly paths — changed PATHS
  only, never contents. Text changes carry a re-enumerate nudge; binaries
  ride a probe-classified status (pending: raw path now + background warm,
  snapshot pointer on a later turn if conversion succeeds; failed: announced
  once; deleted/renamed-out: "old snapshot is invalid" warning, out-of-scope
  paths dropped from tracking). Binary tracking-set mutations are staged and
  applied only on backend accept, so a thrown prompt re-announces.
- Case 2 (in-place first send on a stale empty landing, blocking): the
  composer's send handler consults holdActiveSessionForFreshContext, which
  re-materializes via the existing queue-and-hold and refreshes the landing's
  manifest before the queued send flushes; the auto-flush re-consults the
  gate, and a failed materialization arms the hold at most once per dirty
  revision (no flush -> re-hold loop).
- Case 3 (re-enter): content events now reach isProjectContextDirty, so a
  stale landing is not reused — a fresh session re-materializes and its
  first manifest includes the new sources.
- Resumed sessions keep the projectContextBlock their resume path already
  materialized (previously discarded) and deliver it on the first send with
  a re-check preamble; the delivery cursor is seeded from the epoch captured
  at materialization kickoff.

Known limitations are recorded in the design doc (ships with this change):
change-log compaction renders a generic re-enumerate nudge (no
mini-manifest); ongoing config-change deltas are a generic re-check nudge;
Case 2's in-place refresh covers content staleness only (config-root /
system-prompt capture mismatches are a designed landing-replacement
follow-up); remote same-URL content changes stay stale by design.

Docs: designdocs/agent-projects/AGENT_PROJECT_CONTEXT_STALENESS.md — the
single design doc (three review rounds + implementation record + known
limitations). Tests: +100 units across tracker/matcher/builder/probe/manager/
session/composer, including the composite-key clear guards, the revision-key
join/supersede matrix, transactional delivery, rename-out regression, and
Chat-mode resolver equivalence. Validated end-to-end in a live vault
(natural-question .md and PDF discovery, rename-out, failed-conversion raw
fallback) with frame-log evidence.
…materialized binaries

Obsidian fires ONE rename event for a folder, but Case-1 binary delivery is
entry-based: with only the folder-path entry in the change log, binary
children under a renamed folder never received deleted/ready statuses — a
rename-in silently missed the new snapshot pointers and a rename-out left a
seemingly-usable out-of-scope pointer alive (Codex PR review, P2).

The tracker now expands a TFolder rename at flush time into per-child rename
entries for materialized-binary children only (live enumeration; text
children keep riding the folder entry's re-enumerate nudge to protect the
change-log cap). The children flow through per-child path matching and the
manager's existing renamed branch — old pointer dropped as deleted, new path
tracked iff still a live materialize candidate — so rename-out, rename-in,
and rename-within all resolve correctly with no manager changes.

isMaterializedBinaryPath moves to materializeCandidates as a shared export.
…ot is retained

An edited binary whose re-parse fails kept its older snapshot but wrote NO
failure marker — on disk that state was identical to "conversion still in
progress", so the Case-1 probe classified it pending forever: every send
re-announced "conversion in progress" AND re-warmed the materializer, which
re-attempted the relay parse per send (Codex PR review, P2).

upsertFile's failure path now always writes the fingerprinted marker. The
probe then resolves the edit-then-fail state to "failed" (announced once,
dropped from pending — the warm loop stops) with zero probe changes. The
fingerprint gate keeps the original no-stale-marker rationale intact: probe
and negative cheap-skip honor a marker only while it matches the live file,
the cheap-skip additionally stays gated on "no snapshot" so stale-snapshot
files still re-attempt on every run (transient failures heal without a
manual Retry), and a successful parse deletes the marker. The status UI's
resting state for this case changes from "Queued" to "failed" — the honest
reading. upsertRemote keeps its no-marker-when-stale behavior (remotes are
outside the probe's scope).
…r seed on failed create-materialization

Two Codex PR-review findings:

Cross-boundary renames leaked out-of-scope paths into the updates block
(P1): the tracker emits a rename when EITHER side matches, and the block
builder lists both sides — a rename-out directed the agent at the
out-of-scope new path, a rename-in listed a never-in-scope dead old path.
Entries are now scoped per project at build time: rename-out becomes
deleted(oldPath), rename-in becomes created(newPath), within-scope stays
renamed, and a tag-only conservative match degrades to the broad tag marker
instead of concrete paths. The builder stays scope-blind; the manager's
binary branches receive the strictly-better-typed entries (deleted → drop
old pointer, created → probe new).

A fresh session whose materialization failed swallowed the backlog (P2):
createSession seeds the delivery cursor to the kickoff epoch on the
assumption the first-turn manifest covers it; a whole-materialization
failure captures no manifest, so the seed suppressed the first send's
updates block for those changes. The ready-chain now rolls the seed back
when the result carries no contextSignature (same never-mark-undelivered
principle as the Case-2 hold failure path).

Test-harness fix rider: the Case-1 describe's beforeEach restores the
success-shaped materializer mock — earlier describes install persistent
failing implementations that mockClear does not reset, which the new
rollback would otherwise trip on in every Case-1 test.
…atching

Live create/modify events run through shouldIndexFile, whose
isInternalExcludedPath gate keeps Copilot's internal files (the log file,
project.md configs, AGENTS.md mirrors, unsupported/ backups) out of project
matching. The new path-based matcher for dead events (delete/rename/folder)
skipped that gate, so under a broad inclusion (*.md, or a folder covering
the projects folder) deleting a project or renaming a project folder
recorded internal config paths as user content changes — leaking them into
<project_context_updates> and falsely dirtying the project (Codex PR
review, P2).

filePathInScope now applies isInternalExcludedPath (exported from
searchUtils), which every path-scoped decision inherits: delete/rename file
matching, the cross-boundary rename scoping, and the folder-subtree
extension/note fallback (now routed through the same helper). The
folder-pattern intersection branch stays unguarded on purpose — internal
exclusion is file-level, and only concrete file entries reach the change
log.

Also fixes the CI lint error in contextCacheStore.test.ts (typed the parsed
marker instead of an any-flowing expect.stringContaining property).
…re-marker writes

Round-5 Codex PR review + a holistic matrix sweep of the freshness-delivery
state surfaces. Two findings, each an unfilled cell of a design-time
enumeration matrix; both fixed with their own new-cell hardening.

F-5a (P2) — a binary announced "conversion in progress" is carried in the
per-session pending set and re-probed every send. A config edit that DROPS the
source from scope surfaces only as a CONFIG_CHANGE nudge (no per-file entry), so
the stale path stayed in `pending` forever: re-announced every turn, never
resolvable (the materializer's own pattern filter now skips it), and re-warming
per send. resolveBinaryUpdateStatuses now prunes carried binaries the project no
longer includes from the LIVE pending + announced-failure sets before probing
(pruneOutOfScopePendingBinaries). Scope truth is not delivery state, so it
mutates the live sets directly — persists when the turn stages nothing, never
rolls back on a thrown prompt. Guarded on record PRESENCE: a missing record is a
transient reload window, not a scope change, so it never prunes a still-included
binary that would then never re-announce.

F-5b (P3) — upsertFile's round-2 change made the failure-marker write
unconditional, breaking the invariant runWithLifecycle documents (the only throw
past the upsert's own catch holds no usable snapshot). A marker-dir write failure
while the snapshot file is still readable would reject out of upsertFile and drop
a usable stale snapshot (present:false). The marker write is now best-effort
(logs and continues), so a usable stale snapshot survives its own marker write
failing. markerWanted stays true (preserve-if-present) so a pre-existing
fingerprint-matching marker isn't dropped from the reconcile's wanted set.
runWithLifecycle's comment updated: both upsert paths now protect a usable stale
snapshot from their own marker write.

Also documents the one consistency cell the sweep surfaced but did not fix: a
folder renamed ACROSS the scope boundary keeps both paths verbatim, so the
out-of-scope side is listed among "Changed files". No caller is harmed (the
materialized-binary children are scoped per side; the remaining folder path is
only re-enumerate nudge context, which walks the INCLUDED folders). DESIGN NOTE
added in scopedEntry.

Tests: pending-binary prune on config shrink; stale snapshot survives a
failure-marker write failure.
… failure leaves the manifest incomplete

Codex PR review round-6 (F-6a, P2) + a matrix sweep of the three delivery-cursor
seed sites. A classic high-water-mark bug: the cursor was advanced past a batch
whose manifest did not fully represent it.

When a project session materializes its first-turn `<project_context>` manifest,
a PER-SOURCE binary (PDF/image) conversion failure does NOT fail the whole run —
it still returns a `contextSignature` (undefined only on whole-run throw / no
record). But the manifest OMITS the failed binary. The delivery cursor was
seeded to the current content epoch regardless, so the first send saw
`currentEpoch === deliveredEpoch` and suppressed the `<project_context_updates>`
block that would have announced the failed binary's raw path — leaving a
newly-added PDF invisible to the first turn, the exact staleness this PR fixes.
The round-4 rollback only covered whole-run failure (`contextSignature ===
undefined`); this is the partial-success-with-source-failure cell it missed.

Fix — one completeness signal, three consumers:
- projectContextMaterializer: ContextMaterializationResult gains
  `manifestIncompleteForVaultFiles`, computed inside runMaterialize from the
  materializeSources `failures` (any `kind === "file"`). Computed THERE, not from
  the manager's progress sink, because a single-flight joiner's onProgress is
  dropped — only the shared result reaches every caller.
- AgentSessionManager: `manifestCoversEpoch(result)` (signature present AND not
  incomplete) guards all THREE cursor seed sites — fresh create (roll the seed
  back), in-place hold (skip the delivered-mark), and resume (skip the seed) —
  replacing the round-4 whole-failure-only condition. When uncovered, the first
  send's updates block announces the failed binary; the cursor advances normally
  on the next backend accept.

The flag is broad on purpose: a failure that RETAINED a usable stale snapshot
also sets it, so the first send tells the agent "this changed and reconversion
failed" instead of letting it silently trust an outdated snapshot pointer.
Remote failures never set it — remote same-URL refresh is outside the vault epoch
stream (root-cause C), so no cursor governs it.

Prior art: this is high-water-mark acknowledgement (Kafka offset-commit,
EventBridge Pipes batch checkpoint, ETL watermark) — advance the mark only when
the batch is fully represented. The cursor is a scalar epoch, so the rollback is
all-or-nothing; the first send may re-list already-covered paths (paths only,
harmless), which is the right ROI versus a per-path-ack refactor.

Tests: materializer flags/does-not-flag by failure kind (file vs remote);
fresh-session partial-failure rolls the seed back and re-announces (sibling of
the whole-failure test).
@Emt-lin Emt-lin force-pushed the issue-208-context-staleness branch from 210969f to 046bd71 Compare July 5, 2026 04:10
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 046bd71e1a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agentMode/session/AgentSessionManager.ts
…th a DESIGN NOTE

Codex round-7 flagged that config-change updates carry no newly-added
source paths. Triaged as a mid-session source-summary capability rather
than a staleness bug (no stale content is mis-cited; a new or resumed
session's manifest recovers the gap) — tracked as follow-up #2663. The
note anchors the decision at the config-change branch so future reviews
land on the rationale instead of re-litigating.
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: a779129689

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agentMode/session/AgentSessionManager.ts Outdated
The Case-1 update callbacks were wired into every session, but their mere
presence makes AgentSession.runTurn await before backend.prompt — even
though the getter immediately returns null for the global scope. That
microtask hop breaks the synchronous-dispatch contract documented in
runTurn (a same-turn programmatic cancel could observe running-with-no-
prompt-in-flight). Pass both callbacks only for project sessions, on the
create and resume paths alike.
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 7d6753c165

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/projectContentTracker.ts Outdated
Comment thread src/context/contextUpdatesBlockBuilder.ts Outdated
Emt-lin added 2 commits July 5, 2026 13:51
…ock paths absolute

Two round-9 review findings, one consolidated pass:

- The conservative tag-dirtying path only bumped projects declaring tag
  INCLUSIONS, so an excluded tag gained by an in-scope note (file leaves
  scope) never advanced the epoch — the live modify path cannot see that
  transition because shouldIndexFile already rejects the file. Projects
  declaring tag exclusions now dirty too.

- The updates block rendered vault-relative paths, but a project session's
  cwd is the project folder, so the freshness nudge pointed at unreachable
  paths. The builder now accepts a resolveAbsolutePath option (injected by
  the manager from the FileSystemAdapter; null off-desktop falls back to
  the vault-relative label) and renders text AND binary paths absolute,
  matching the first-turn manifest's convention.
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 444aa9dcf3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agentMode/session/AgentSessionManager.ts Outdated
…age is known

The create path seeded the Case-1 cursor optimistically at construction
and rolled it back on the ready chain when materialization came back
incomplete. But initialize exposes the session as idle right after
newSession — before ready resolves through the model-confirmation round
trip — so a first send in that window saw the optimistic seed and
silently swallowed the failure backlog (self-healing next turn, but the
announcement was late).

Replace optimistic-seed-plus-rollback with a coverage-gated seed chained
off contextReady: registered before AgentSession.start and awaited by
initialize before newSession, the seed always lands before any send is
possible, and an incomplete run simply never seeds. This also removes
the rollback block and its equality guard, and matches the resume path,
which already seeds only with the result in hand — all four cursor
writes now advance the high-water mark only after coverage is known.
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 966bbbd679

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agentMode/session/AgentSessionManager.ts
…ation result

A blocking hold that single-flight-joins a sink-less background warm still
owns the load atom (the warm never touches it), but its onProgress is
dropped by the join — so the accumulated failedSources stayed empty and
the terminal done publish reported a failed run as clean.

Same treatment as the manifest-coverage bit: terminal correctness signals
ride the shared result, progress events stay owner-only display. The
materializer now returns the authoritative failures list on every
completed-run path (including the whole-run-throw synthetic failure,
which no longer returns the shared empty result), and the manager's done
publish reads it off the result, falling back to progress-accumulated
state only for result shapes that carry none. Step counts and in-flight
items remain progress-only — a joiner shows no counts, a cosmetic gap.
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 739bbe85e1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/projectContentTracker.ts
Comment thread src/context/projectContentTracker.ts Outdated
… the compaction trade-off

A rename across the project boundary merged with a delete in the same
debounce window keeps the rename's oldPath on a deleted entry, and the
old verbatim pass-through leaked the out-of-scope path into the updates
block — and keyed the binary probe on the wrong side, skipping the
in-scope snapshot's removed-do-not-cite invalidation. Deleted entries
carrying an oldPath are now scoped per side like renames, announcing
only the side the project actually knew.

Compaction (change log > cap) deliberately keeps collapsing everything
into the single marker: snapshots are named by source path, so a
re-materialized binary refreshes content behind the existing pointer,
and the broad re-enumerate instruction is the pressure valve for the
dropped per-path precision. Locked with a DESIGN NOTE.
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 7c4711b37b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/context/projectContentTracker.ts
Comment thread src/search/searchUtils.ts Outdated
…rkdown; stabilize the empty affected-projects slice

flush-on-send can beat Obsidian's async metadata parse of a just-created
file: the live matcher then sees no tags, so a tag-included file was
silently missed on the first send and a tag-EXCLUDED file could leak a
concrete path that later turns out excluded. When getFileCache returns
null for a live markdown change, tag-declaring projects now receive the
broad TAG_CHANGE marker (once per flush) instead of going through the
precise matcher; non-tag projects still match precisely. The stale-cache
modify window stays a sub-second self-healing race, documented in code.

resolveAffectedProjects also returns a frozen module-level constant on
no-match instead of a fresh [], per the repo's referential-stability
rule (the helper is shared by the chat watcher and the Agent tracker).
@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 93ac0fc0a6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@Emt-lin

Emt-lin commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Review summary (for maintainer)

This PR went through 13 rounds of Codex review (final state: clean at 93ac0fc0, 0 unresolved threads). Condensed map of what happened, so you don't have to replay the threads:

What the PR does

Keeps an Agent Mode project session's context fresh after the first turn: a vault/config change while the session is open surfaces as a <project_context_updates> block on the next send (changed text files with absolute paths, binary snapshot statuses, config/tag nudges), gated by a per-session delivery cursor so nothing is re-announced. A stale empty landing holds the first send (amber "Waiting for context" queue row) while context re-materializes.

Key hardening that came out of review (9 fixes)

  • Delivery cursor is a strict high-water mark: seeded only once materialization coverage is known (manifestCoversEpoch), never optimistically; all four cursor writes follow the same rule (create seed / resume seed / hold delivery / turn-accept).
  • Terminal correctness signals ride the shared materialization result (failures, manifestIncompleteForVaultFiles) — single-flight joiners drop onProgress, so a blocking hold joining a sink-less background warm still publishes a truthful load state.
  • Scope semantics: tag exclusions participate in conservative dirtying; unparsed-metadata markdown (flush-on-send racing Obsidian's async parse) routes tag-declaring projects to the broad marker instead of the precise matcher (a tag-excluded path can't leak); debounce-merged rename→delete entries are scoped per side.
  • Global sessions keep synchronous prompt dispatch (update callbacks are wired only for project sessions).
  • Updates block renders absolute paths (matching the first-turn manifest convention) so the nudges are actually actionable from the project cwd.

Deliberate non-fixes (each locked with an in-code DESIGN NOTE)

  • Config-change updates send a generic "sources changed" nudge, not a re-delivered source list → follow-up Agent Mode: deliver a current-source summary to ongoing sessions after a project config change #2663.
  • Change-log compaction (>200 entries) collapses to one broad re-enumerate marker; snapshot files are named by source path, so refreshed binaries keep their pointer.
  • Cross-boundary folder renames stay verbatim (children are scoped per side); modify-with-stale-metadata remains a sub-second self-healing race.

If any of these come up in review, the DESIGN NOTE at the flagged line has the full rationale.

State

Ready for human review.

@Emt-lin Emt-lin requested review from logancyang and removed request for logancyang July 5, 2026 13:05
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