Skip to content

fix(core): re-inject nested instructions after compaction - #43723

Merged
kitlangton merged 1 commit into
v2from
fix-nested-instruction-reinjection
Aug 21, 2026
Merged

fix(core): re-inject nested instructions after compaction#43723
kitlangton merged 1 commit into
v2from
fix-nested-instruction-reinjection

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

What

Nested AGENTS.md instructions silently vanish after compaction and never come back for the rest of the process lifetime. This fixes the dedup ledger so the next read in the affected subtree re-injects them.

Root and global AGENTS.md are unaffected — they live in the instruction epoch machinery, which survives compaction by design (advanceEpoch folds current values into the new baseline). This bug is specific to nested AGENTS.md discovered by the read tool and injected as durable synthetic messages.

Before / After

Before:

  1. The agent reads sub/deep/file.txt; SessionInstructions.load discovers sub/AGENTS.md, injects it as a synthetic message, and records the path in two dedup layers: an in-memory Ref claim and the synthetic's own metadata ledger.
  2. The session compacts. SessionHistory.load truncates model-visible history at the compaction boundary (gte(seq, compaction.seq), history.ts:43) — the synthetic carrying the rules is gone. The summarizer saw the rules as [Synthetic context]: prose, but nothing obligates the summary to retain instruction bytes.
  3. The agent reads in that subtree again. Discovery finds sub/AGENTS.md, but the in-memory claim — which nothing ever clears — still holds the session/path pair, so load returns before the durable ledger (which correctly forgot) is even consulted. No re-injection.
  4. The model stops following sub/AGENTS.md until the Location layer restarts. Long-running servers — the exact population that compacts — are hit hardest.

After:

  1. Same read, same injection, same durable metadata ledger.
  2. Same compaction truncation.
  3. The in-memory claim now only guards a load while it is in flight and is released once the load settles. The next read consults durable history, finds no surviving synthetic for the path, and re-injects the rules.

Invariant after the fix: the synthetic message metadata in durable, model-visible history is the sole lasting dedup ledger. Anything that drops a synthetic from that history (compaction, committed revert) self-heals on the next read in the subtree.

How

packages/core/src/session/instructions.ts — the Ref claim (injected → renamed inFlight) is released in Effect.ensuring after the publish settles. Release is safe because the durable publish commits the synthetic and its metadata ledger atomically with the event (Bus commit hook), so a subsequent read scans the committed ledger and dedups there. The same-step parallel-read guard is preserved: concurrent loads for the same session/path still collapse to one injection.

packages/core/test/session-instructions.test.ts — new test "re-injects nested instructions dropped from history by compaction": read → assert injected → publish Compaction.Started/Ended (projector creates the completed compaction, truncating history) → assert the synthetic is gone from model-visible history → read again → assert re-injection. Fails on v2 with Received length: 0.

sequenceDiagram
    participant R as read tool
    participant SI as SessionInstructions
    participant Ref as in-memory claim
    participant H as durable history
    R->>SI: load(session, sub/AGENTS.md)
    SI->>Ref: claim (in-flight)
    SI->>H: scan metadata ledger — not found
    SI->>H: publish synthetic (rules + ledger, atomic)
    SI->>Ref: release claim
    Note over H: compaction truncates history — synthetic dropped
    R->>SI: load(session, sub/AGENTS.md)
    SI->>Ref: claim (empty — was released)
    SI->>H: scan ledger — forgotten with the synthetic
    SI->>H: re-inject rules
Loading

Scope

Deliberately narrow. The structural fix — migrating nested AGENTS.md onto the instruction epoch machinery so nested rules survive compaction in the epoch baseline and gain change/removal narration — is a separate design (session-scoped instruction sources). This PR makes the current mechanism self-healing; a beat of staleness remains between compaction and the next read in the subtree, which the epoch migration would eliminate.

Testing

  • bun run test test/session-instructions.test.ts (packages/core): 7 pass / 0 fail; the new test fails without the fix (Received length: 0 at the re-injection assertion)
  • bun run test (packages/core): 1,915 tests, 0 fail
  • bun typecheck (packages/core): clean

@kitlangton
kitlangton force-pushed the fix-nested-instruction-reinjection branch from 872510e to 45e1bfb Compare August 20, 2026 21:16
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  • packages/core/test/session-instructions.test.ts:236 — The compaction self-heal is well covered, but nothing exercises the in-flight claim itself: two concurrent loads for the same path should yield exactly one synthetic while both are racing, and that invariant is currently pinned nowhere — add a test running two executeTool calls in parallel fibers and asserting synthetics length stays 1, since a regression in the Ref.modify claim would reintroduce duplicates rather than the loss this PR fixes.
  • .changeset/nested-instruction-reinjection.md:5 — The changeset promises self-healing for "compaction, revert" but only compaction has a test; a symmetric revert case (truncate history, read under the subtree, expect re-injection) would keep both truncation mechanisms honest, since they may drop synthetics through different code paths.
  • packages/core/src/session/instructions.ts:60 — When every claimed path comes back unreadable, the claim is released with nothing published, so the very next read retries the filesystem work immediately with no backoff — harmless for genuinely missing files, but a transiently locked/unreadable AGENTS.md under heavy parallel tool calls could cause repeated read storms; consider a short negative-cache TTL or at least a debug log when a claim round produces zero readable files.
  • packages/core/src/session/instructions.ts:74 — The direct bus.publish bypass of Session.synthetic is well-justified in comments, but it also means Session-layer validation/mapping applied to other synthetics is skipped here — if Session.synthetic ever grows normalization (text limits, metadata schema enforcement), this call site will silently diverge; a cross-reference comment pointing at the coupling (or a shared publish helper) would future-proof it.

— AI code review (automated)

@kitlangton
kitlangton merged commit 94f9d32 into v2 Aug 21, 2026
8 checks passed
@kitlangton
kitlangton deleted the fix-nested-instruction-reinjection branch August 21, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants