feat(agent): per-turn freshness reminder on the session envelope (plan #941) - #943
Conversation
…#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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #943
Verdict: CONCERNS
Repo: btipling/invincible
Scope: main ← plan/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 —
derivePersistFoldmust project this run only (slice atpriorMessages.length; default0keeps 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}).
|
Adversarial-review follow-up ( Landed the CONCERNS Major + both Minors on this branch. Not merging.
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #943
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/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.#277still fail-closes stale edits. - Reminder is a second trailing
{role:'user'}after the raw prompt (toModelMessageserror 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'sresolvePathForToolpath. 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
#277atstr_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).
|
Adversarial-review follow-up ( Landed the two Minors on this branch (PASS WITH NOTES). Not merging.
|
Implements plan #941. Closes #693.
Persisted
tool_resultsnapshots 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) —buildFreshnessReminderpairs committed okread_filetool rows with their assistanttoolCallsbytoolCallId→{paths}(dedupe first-seen);renderFreshnessReminderemits the lockedError: 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 #563limit>=totalLinesescape hint;serializeFreshnessReminder→ the bounded{paths}body. TextEncoder byte math — NO NodeBuffer(the fix(agent): TextEncoder byte-length in modelMessages (Workflows has no Buffer) #939 canvas lesson).lib/sessionCloudCaps.ts, generous, no existing cap changed):FRESHNESS_REMINDER_MAX_PATHS= 64 (drop-oldest, keep newest + explicit… N earlier paths omittedmarker) ·FRESHNESS_REMINDER_MAX_BYTES= 16 KiB serialized{paths}.turnPersistSeamwrites the{paths}JSON as its own Blob object +meta.freshnessReminderPointer(fail-closedfreshness_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.derivePersistFoldderives thefreshnessRemindersibling.resolveInStepFreshnessReminder(ininStepPreambles.tsper the feat(agent): durable working-notes block on the session envelope (plan #938) #940 SWC lesson) reads in-step withisObjectIdBoundTofail-closed and folds the reminder as the trailing{role:'error'}row BEFOREtoModelMessages(→ 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).freshnessReminderPointer(Redis-safe opaque, drop-to-unset) +copyForwardFreshnessReminderPointerinupsertEnvelope(Memory + Redis) +workerMetaOverlayworker key; hostcloudMetaFornever emits it (same LWW-stomp class asmodelMessagesPointer).docs/session-model.mdreserved-key table row,docs/agent-stream.mdwire section + where-to-change row,docs/harness-limits.mdcaps table row,AGENTS.mdownership row.Testing
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.tsrow 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.tsrows 6/6b/7/7b (fold sibling, zero-read volatility, first-round-only pointer)app/api/turns/route.test.tsrow 8 (sanitize-only pass-through, no blob read in route)turnWorkflow.test.tsrow 9 (two-turn e2e: trailing reminder user row below seeded history + zero-read clear)Gates:
npm run typecheck✅ · fullnpm 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.