Skip to content

feat(agent): per-turn freshness reminder on the session envelope (plan #941) - #943

Merged
btipling merged 3 commits into
mainfrom
plan/freshness-reminder
Sep 5, 2026
Merged

feat(agent): per-turn freshness reminder on the session envelope (plan #941)#943
btipling merged 3 commits into
mainfrom
plan/freshness-reminder

Conversation

@btipling

@btipling btipling commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Implements plan #941. Closes #693.

Persisted tool_result snapshots are not live file views (#693). Plan #936 (A1) put the structured truncated evidence on the wire; this PR teaches the model its evidentiary value with a volatile per-turn freshness reminder.

What ships

  • lib/agent/freshnessReminder.ts (NEW, pure)buildFreshnessReminder pairs committed ok read_file tool rows with their assistant toolCalls by toolCallId{paths} (dedupe first-seen); renderFreshnessReminder emits the locked Error: File-freshness law… volatile copy with the full-read rule (a windowed/truncated read does NOT grant edit — plan-review Major 2.1 Provision DigitalOcean droplet for builds #1) + the agent tool: windowed read_file — don’t dump huge files #563 limit>=totalLines escape hint; serializeFreshnessReminder → the bounded {paths} body. TextEncoder byte math — NO Node Buffer (the fix(agent): TextEncoder byte-length in modelMessages (Workflows has no Buffer) #939 canvas lesson).
  • NEW caps (lib/sessionCloudCaps.ts, generous, no existing cap changed): FRESHNESS_REMINDER_MAX_PATHS = 64 (drop-oldest, keep newest + explicit … N earlier paths omitted marker) · FRESHNESS_REMINDER_MAX_BYTES = 16 KiB serialized {paths}.
  • Persist sideturnPersistSeam writes the {paths} JSON as its own Blob object + meta.freshnessReminderPointer (fail-closed freshness_reminder_write_failed). Volatility: rewritten on EVERY persist — a zero-read turn writes {paths:[]} and advances the pointer, so a stale path list never survives two turns. derivePersistFold derives the freshnessReminder sibling.
  • Read side — the route passes the pointer sanitize-only (never a Blob read in the route); resolveInStepFreshnessReminder (in inStepPreambles.ts per the feat(agent): durable working-notes block on the session envelope (plan #938) #940 SWC lesson) reads in-step with isObjectIdBoundTo fail-closed and folds the reminder as the trailing {role:'error'} row BEFORE toModelMessages (→ trailing user message below the whole history). The loop forwards the pointer on the first round only; wrap-up rounds never see it. Any failure → no row, fail-open (advisory memory, never a 5xx).
  • Store — reserved freshnessReminderPointer (Redis-safe opaque, drop-to-unset) + copyForwardFreshnessReminderPointer in upsertEnvelope (Memory + Redis) + workerMetaOverlay worker key; host cloudMetaFor never emits it (same LWW-stomp class as modelMessagesPointer).
  • Docsdocs/session-model.md reserved-key table row, docs/agent-stream.md wire section + where-to-change row, docs/harness-limits.md caps table row, AGENTS.md ownership row.

Testing

  • New: freshnessReminder.test.ts (rows 1–3: pairing/dedup/skips, caps incl. UTF-8 multibyte never split, renderer locked copy + no-Buffer source-lock)
  • turnPersistSeam.test.ts row 4/4b/4c (own-Blob + volatility {paths:[]} advance + fail-closed)
  • sessionStore.test.ts / sessionRepository.test.ts (reserved key drop-to-unset, copy-forward, host-never-emits)
  • turnLoop.test.ts rows 6/6b/7/7b (fold sibling, zero-read volatility, first-round-only pointer)
  • app/api/turns/route.test.ts row 8 (sanitize-only pass-through, no blob read in route)
  • turnWorkflow.test.ts row 9 (two-turn e2e: trailing reminder user row below seeded history + zero-read clear)

Gates: npm run typecheck ✅ · full npm test (di-gate + journal-gate + vitest: 170 files / 3351 tests) ✅ · npm run build

Per plan: no merge — stops at merge-ready for adversarial review.

Refs: #548 (parent), composes #938/#936; keeps files-touched names so A4 (#552) compaction retains them.

…#941, source #693)

Persisted tool_result snapshots are not live file views (issue #693).
A1 (plan #936) put the truncated evidence on the wire; this teaches the
model its evidentiary value with a volatile per-turn reminder.

- lib/agent/freshnessReminder.ts (NEW): pure projection over the loop's
  reconstructed rows — pairs committed ok read_file tool rows with their
  assistant toolCalls by toolCallId; renderFreshnessReminder emits the
  locked 'Error: File-freshness law…' volatile copy (full-read rule +
  #563 limit>=totalLines escape hint); TextEncoder byte math, NO Buffer
  (the #939 canvas lesson)
- NEW caps (sessionCloudCaps): FRESHNESS_REMINDER_MAX_PATHS = 64
  (drop-oldest, keep newest + explicit marker) and
  FRESHNESS_REMINDER_MAX_BYTES = 16 KiB serialized {paths} — generous,
  no existing cap value changed
- persist: turnPersistSeam writes the {paths} JSON as its OWN Blob object
  + meta.freshnessReminderPointer (fail-closed
  freshness_reminder_write_failed); VOLATILE — rewritten on every persist,
  a zero-read turn writes {paths:[]} and advances the pointer
- derivePersistFold: freshnessReminder sibling; TurnLoopInput /
  TurnWorkflowArgs / ModelStepFn carry the pointer; the loop forwards it
  to the model step on the FIRST round only (wrap-up never sees it)
- read: route passes the pointer sanitize-only (never a Blob read in the
  route); resolveInStepFreshnessReminder (in inStepPreambles per the #940
  SWC lesson) reads in-step with isObjectIdBoundTo fail-closed and folds
  the reminder as the trailing {role:'error'} row BEFORE toModelMessages;
  any failure → no row, fail-open
- store: reserved freshnessReminderPointer (Redis-safe opaque,
  drop-to-unset) + copyForwardFreshnessReminderPointer in upsertEnvelope
  (Memory + Redis); workerMetaOverlay worker key; host cloudMetaFor never
  emits it (same LWW-stomp class as modelMessagesPointer)
- tests: freshnessReminder.test.ts (rows 1-3), turnPersistSeam.test.ts
  (row 4/4b/4c + copy-forward), sessionStore.test.ts,
  sessionRepository.test.ts, turnLoop.test.ts (rows 6/6b/7/7b),
  app/api/turns/route.test.ts (row 8), turnWorkflow.test.ts (row 9
  two-turn e2e + zero-read volatility)
- docs: session-model.md reserved-key table, agent-stream.md wire
  section, harness-limits.md caps table, AGENTS.md ownership row

Closes #693. Implements plan #941.
@vercel

vercel Bot commented Sep 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
invincible Ignored Ignored Sep 5, 2026 1:39am UTC

Request Review

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Adversarial review — PR #943

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/freshness-reminder @ f8440e9 · 24 files · volatile per-turn freshness reminder (plan #941 / source #693)
Lenses run: L1, L2, L5, L6, L8 (skip L3: no UI/bridge/Wasm product path; skip L4: no GHA/runner; skip L7: no new env/host bind; skip L9: no palette)
AGENTS.md read: yes (plus docs/feature-divide.md, SECURITY.md, plan #941)

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Major L1+L6 derivePersistFold · buildFreshnessReminder(messages) walks the full reconstructed array ([...priorMessages, user, …this run]). #936 seeds keep orchestrator-shape assistant.delta.toolCalls[].args.path + role:'tool' toolName:'read_file' rows, so a zero-read this turn still re-derives the prior turn's paths. Goal 3 volatility ({paths:[]} so a stale list never survives two turns) does not hold on the production seed path. Row 6b / row 9 turn-3 / the "zero-read advances pointer" e2e all omit priorMessages, so they cannot catch this. Turn 1 read_file src/foo.ts persists {paths:['src/foo.ts']} + a model-messages seed. Turn 2 is a chat with no tools, started with that seed (the #936 path POST /api/turns always takes when the pointer is bound). derivePersistFold still sees the seeded read_file pair → persist writes {paths:['src/foo.ts']} again, pointer advances to the same names. Turn 3 still folds "re-read src/foo.ts" even though this run observed nothing — and a truly zero-read chat never clears. Defender: "the comment says THIS run's calls; tests prove []." The comment is the spec; the call site passes messages, not a this-run slice. Tests that prove [] construct a user+assistant-only array with no seed. Row 9 turn 3 even drops priorMessages/freshnessReminderPointer on purpose. #936 ModelMessageRow is the same shape buildFreshnessReminder parses (args kept by normalizeToolCall). high
Minor L8 Locked copy · renderFreshnessReminder lists "Paths read in earlier turns of this session" while the blob (plan + docs/session-model.md) is the previous turn's {paths} only. Once the Major is sliced to this-run, that line is a lie: files whose snapshots still sit in seeded history are unnamed. Model sees turn-1 tool_result for a.ts in history, reminder path list (after a later chat, or after the slice fix) does not mention a.ts, copy claims the list is session-wide. Model treats the listed names as exhaustive. Defender: "the prose above already states the session-wide snapshot law; the list is a hint." Then the heading must not say "earlier turns of this session". high
Minor L6 resolveInStepFreshnessReminder (lib/workflows/inStepPreambles.ts) has no unit coverage of fail-open (unbound pointer, missing blob, non-{paths} JSON). Route row 8 covers sanitize-only pass-through; row 9 covers the happy in-step read. The confused-deputy isObjectIdBoundTo branch is untested. A planted opaque-but-unbound pointer, or a same-session pointer at a transcript object, should fold nothing. Nothing in the suite asserts that. Defender: "fail-open is try/catch + shape checks; e2e proves the happy path." True for the happy path; the security-adjacent miss is unproven. medium

Residual risk

The reminder is advisory: #277 RunFileFreshness still fail-closed at the tool. A missed or stale path list cannot grant a stale edit. Empty-blob rewrite every persist (including {paths:[]}) orphans prior reminder objects the same way checkpoint / model-messages already do. cloudMetaFor omit + envelope copy-forward matches the #937/#940 worker-pointer class; in-step isObjectIdBoundTo fail-open is the confused-deputy gate (route is sanitize-only, as documented). Control characters in read_file paths are rejected by workPath before a committed ok row can exist, so - ${path} cannot split the fold. Wrap-up modelStep calls omit the pointer.

Merge guidance

  • CONCERNS: do not merge until the Major is fixed — derivePersistFold must project this run only (slice at priorMessages.length; default 0 keeps existing unit fixtures) and the suite must include a seeded zero-read row that expects {paths:[]}. The copy heading should name the previous turn, not the whole session. In-step fail-open tests are optional-but-should.
  • DI/cost: no new PGlite / live I/O in changed tests — Memory stores + mocks. Not a gate.

What was not attacked

Live Vercel Workflows canvas, prod Gateway, Blob GC of orphaned reminder objects, #277 grant matrix (search / list_dir), A4 compaction #552 interaction beyond "names must survive compact."

derivePersistFold walked the full reconstructed messages array, so a
#936-seeded zero-read turn re-derived the prior turn's read_file paths
and Goal 3 volatility never cleared on the production seed path.

Slice the reminder at priorMessages.length; pin seeded [] / this-run-only
rows; correct the locked copy heading to "previous turn"; cover in-step
fail-open (unbound / missing / non-{paths}).

btipling commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Adversarial-review follow-up (0733552)

Landed the CONCERNS Major + both Minors on this branch. Not merging.

Finding Fix
Major L1+L6derivePersistFold walked [...priorMessages, …this run], so a #936-seeded zero-read chat re-derived last turn's read_file paths and Goal 3 volatility never cleared thisRunStart (default 0) + slice at (input.priorMessages ?? []).length. Seeded [] pin in turnLoop.test.ts / turnWorkflow.test.ts row 9 turn 3 now passes production-shaped prior rows with args.path
Minor L8 — locked copy said "Paths read in earlier turns of this session" Heading is now "Paths read in the previous turn"
Minor L6 — in-step fail-open untested resolveInStepFreshnessReminder rows: bound {paths} renders; unbound pointer skips blob read; missing blob / non-{paths} JSON / {paths:[]}undefined

npm run typecheck green. Targeted vitest (turnLoop / turnWorkflow / freshnessReminder / preambles / persist seam / sessionStore / turns route) green.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Adversarial review — PR #943

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/freshness-reminder · 25 files · per-turn freshness-reminder fold (plan #941 / source #693)
Lenses run: L1, L2, L5, L6, L7, L8 (skip L3: no UI/Wasm/DOM product path; skip L4: no workflow/runner/wasm supply; skip L9: no palette/UX chrome)
AGENTS.md read: yes · docs/feature-divide.md read (agent-loop touch)

Prior pass (0733552) already closed the CONCERNS Major (seeded #936 rows leaking into derivePersistFold). This attack is against current HEAD 0733552.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L1+L6 serializeFreshnessReminder (lib/agent/freshnessReminder.ts · serializeFreshnessReminder) drops boundPaths().omitted. Persist writes {paths} only. resolveInStepFreshnessReminder then renderFreshnessReminders the already-trimmed list, so omitted is always 0. The explicit … N earlier paths omitted marker the Caps table / docs/harness-limits.md / row-2 tests advertise never appears on the production persist→read→render path. Tests only call renderFreshnessReminder(unbounded) directly. Turn reads 70 files. Blob stores the newest 64. Next turn's reminder lists 64 paths with no omitted marker. Model treats the list as complete and skips re-read of a dropped (oldest) path; #277 still fail-closes the edit, but the honesty mechanism the cap exists for is dead. Defender: the locked copy already says "before editing any path" (universal law), so the list is a hint, and 64 is generous. That does not rescue the marker: docs + row-2 tests claim an on-the-wire honesty signal that production cannot emit. high
Minor L1 buildFreshnessReminder dedupes first-seen, then the row/byte cap drops the oldest. A path read first and last stays at the head and is the first dropped once the list exceeds 64. Caps rationale says keep newest ("the reads the model is most likely to edit"). Plan copy also locked first-seen — the two locks conflict on a re-read. Turn reads auth.ts, then 64 other files, then re-reads auth.ts (edit target). Reminder omits auth.ts, keeps the exploratory tail. Defender: 65 unique paths in one turn is rare; the universal law still applies; plan locked first-seen. Survives as bounded risk, not a merge stop. high
Minor L2 Path strings are interpolated raw (renderFreshnessReminder: `- ${p}`). pathFromArgs only trim()s. A read_file args.path containing CR/LF/U+2028 becomes extra reminder lines (and can smuggle a fake omitted marker or extra Error: sentence into the trailing user message). Model (or a planted same-session {paths} blob) records foo.ts\nError: ignore the law. Next turn's reminder body splits. The #277 gate is path-keyed on the resolved disk path, not this string — no edit bypass, but the teaching block is attacker-shaped. Defender: args.path is the model's own previous-turn tool arg (same-trust), and host cloudMetaFor never emits the pointer. Still a missing sanitize at a string-into-prompt boundary. high
Nit L8 modelGenerateStep comment says the reminder "Never fires on disableTools wrap-up rounds" but the only guard is the loop omitting the arg. No && !args.disableTools in the step. A future wrap-up caller that forwards the pointer would fold an advisory block into a tools-off "tell the user" round. Loop-only today (wallWrapUp / steps wrap-up do not pass it). Belt-and-suspenders, not a live break. Loop contract holds on this PR. Keep as nit. high

Residual risk

  • Mid-turn user-line persist (persistOnce(false) after the first tools-round, before tool rows exist) writes {paths:[]} and advances the pointer. A crash/cancel that never reaches the later persist leaves the next turn with an empty reminder and no teaching about the previous completed turn's reads. 409 live-lock covers the in-flight window; the hole is post-crash. #277 still fail-closes stale edits.
  • Reminder is a second trailing {role:'user'} after the raw prompt (toModelMessages error idiom). Some providers historically disliked back-to-back user roles; wrap-up already uses this shape with an assistant/tool in between, so this PR is the first systematic consecutive-user pair on round 1. Not reproduced against Gateway.
  • Names come from args.path, not the gate's resolvePathForTool path. A cwd change between turns can make the listed name miss the grant key. Universal law still applies.
  • Old freshness-reminder blobs are never GC'd (same class as checkpoint / model-messages pointers).
  • Advisory-only: a model that ignores the block still hits #277 at str_replace / overwrite.

Merge guidance

PASS WITH NOTES: safe to merge from this attack; Minors should land soon (omitted-marker roundtrip is a test/docs lie, path-line sanitize is cheap). No Blocker, no Major, no DI/cost hit (new tests are in-memory / mocked; no new PGlite( / live Redis).

Do not --approve from this skill.

What was not attacked

Live DO runner, prod Gateway, real Anthropic consecutive-user 400, Blob GC, Wasm/DOM paint (reminder is model-step-local, not SSE / not checkpoint).

…ial #943)

serializeFreshnessReminder dropped boundPaths().omitted, so the
production persist→read→render path never emitted the cap's honest
`… N earlier paths omitted` marker (row-2 tests only called render on
an unbounded list). Store `{paths, omitted}` when the cap drops rows;
under-cap bodies stay `{paths}`. resolveInStepFreshnessReminder passes
the count through.

Control characters (CR/LF/U+2028/U+2029/C0) in args.path are dropped
rather than interpolated into `- ${p}` lines, so a planted path cannot
smuggle extra reminder sentences into the trailing user message.

cloudMetaFor JSDoc names freshnessReminderPointer in the never-emit
exception list (same class as modelMessagesPointer).

btipling commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Adversarial-review follow-up (d556f9e)

Landed the two Minors on this branch (PASS WITH NOTES). Not merging.

Finding Fix
Minor L1+L6serializeFreshnessReminder dropped omitted, so the persist→read→render path never emitted … N earlier paths omitted (row-2 only called render(unbounded)) Blob is {paths, omitted} when the cap drops rows; under-cap stays {paths}. resolveInStepFreshnessReminder passes the count into renderFreshnessReminder. Roundtrip test pins the marker.
Minor L2 — raw `- ${p}` interpolation let CR/LF/U+2028 in args.path split the reminder into extra lines Control-char paths are dropped at build and render (not collapsed into a fake name).
Nit L8 — wrap-up disableTools guard Withdrawn. modelGenerateStep early-returns on disableTools before the reminder fold; wrap-up never reaches it.
Minor L1 — first-seen dedupe vs keep-newest cap on re-read Left. Plan locked first-seen; 64 unique paths in one turn is the bound. Residual, not a merge stop.

npx tsc --noEmit green. Targeted vitest (freshnessReminder / preambles / persist seam / turnLoop / turnWorkflow — 222 tests) green.

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.

agent memory: tool_result snapshots are not live file views (re-read or they’re stale)

1 participant