Skip to content

feat(agent): assert the request path against stored history - #149

Merged
hung12ct merged 1 commit into
mainfrom
feat/request-derivation-invariant
Aug 17, 2026
Merged

feat(agent): assert the request path against stored history#149
hung12ct merged 1 commit into
mainfrom
feat/request-derivation-invariant

Conversation

@hung12ct

Copy link
Copy Markdown
Owner

What & why

The messages sent to a provider are not stored history: enforceTokenBudget applies the budget policy, then buildMsgsForLLM layers on five injections (soft-landing hint, memory notes, tool-chaining hint, plan-mode hint, dynamic context) plus the AutoCacheSystem stamp.

Two properties keep that pipeline honest, and both were held by convention alone:

  1. Stored history is read-only along the request path. Every stage returns a derived slice; none may write through to the caller's messages, which are the source of truth for what gets persisted.
  2. The derivation is pure. Re-running it on the same stored history yields the same messages, so what a request carried stays reconstructable.

Property 1 has been violated before — buildMsgsForLLM copies before stamping CacheHint precisely because the stamp used to leak into the caller's session-loaded slice, and its own comment records that. A doc comment cannot hold that invariant; a check can. Today, any edit that threads the derived slice into saveSession would silently corrupt adopter history with nothing to catch it.

Off by default. Nil handler means no snapshot and one nil comparison per iteration.

Changes

  • deriveRequestMessages(stored, maxTokenBudget) contextDerivation — the extracted pure half of enforceTokenBudget: no receiver, no emit, same input/output. enforceTokenBudget is now a thin emitting wrapper over it, and shorter for it.
  • WithRequestInvariant(RequestViolationFunc) — enables the check; the handler runs synchronously on the loop goroutine and the turn continues either way, so a handler that fails the build is the intended use.
  • snapshotRequest / checkRequestInvariant in a new request_invariant.go.
  • iterationState.sentMessages records the exact list handed to the provider, populated only when the invariant is on.

The check compares the conversation, not the whole request

Two of the injectors do more than rewrite the system message: withPlanModeHint prepends a system message and withDynamicContext appends a user message. An index-aligned comparison would false-positive on both, and a noisy invariant is a useless one. So:

  • System messages are excluded — they are the declared framing surface, and four stages legitimately reshape them.
  • Every derived non-system message must reach the provider in order, byte for byte.
  • A non-system message that is not in the derivation must carry dynamicContextSentinel, or it is content reaching the model that no re-derivation accounts for.

Comparison is field-wise rather than reflect.DeepEqual: Parts can carry megabytes of media (compared by length — the pruning and injection stages only rewrite Content), and CacheHint is stamped on the request copy by design.

Testing

  • TestRequestInvariant_CleanRunReportsNothing — the false-positive gate, and the test that matters most. Four subtests across the paths that legitimately reshape a request (defaults, budget-forced pruning, dynamic-context user injection, memory notes), all required to report nothing. Verified non-vacuous: a temporary probe inside checkRequestInvariant fired 8 times across these subtests, confirming the check is genuinely reached and genuinely silent.
  • TestStoredUnchanged — rewritten content, dropped message, rewritten tool call.
  • TestDerivationReproduces — system reshaping and declared injection admitted; undeclared injection, rewritten conversation, and dropped conversation all caught.
  • TestDeriveRequestMessages_IsPure — across four budgets, the derivation is deterministic and does not mutate its input.
  • TestRequestInvariant_DisabledTakesNoSnapshot — no allocation when off, and the check is inert rather than panicking on a nil snapshot.
  • gofmt -l . empty, make lint 0 issues, make build, make test, make test-race all clean.

Follow-up not in this PR

Fork / ForkAtLastUser could carry the same assertion at the branch point, where a snapped boundary could drop a tool-call pair. Not built — no evidence it is needed yet.

Checklist

  • Title is a Conventional Commit
  • gofmt -l . prints nothing
  • make lint is clean
  • make build passes
  • make test (and make test-race) passes
  • Errors wrapped with a package prefix
  • CHANGELOG.md updated if this PR is cut as a release tag
  • No changes to pkg/llm/, pkg/history/, or pkg/telemetry/

Extracts the pure deriveRequestMessages and adds opt-in
WithRequestInvariant, which checks that the pipeline neither writes
through to stored history nor sends conversation a re-derivation cannot
reproduce. Off by default.
@hung12ct
hung12ct force-pushed the feat/request-derivation-invariant branch from 6d28a5d to eb41598 Compare August 17, 2026 16:29
@hung12ct
hung12ct merged commit 6b3ce4e into main Aug 17, 2026
2 checks passed
@hung12ct
hung12ct deleted the feat/request-derivation-invariant branch August 17, 2026 16:31
@hung12ct hung12ct mentioned this pull request Aug 17, 2026
8 tasks
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