Skip to content

refactor(training-agent): adopt @adcp/client 5.1 state-store helpers#2283

Merged
bokelley merged 2 commits into
mainfrom
bokelley/adcp-state-5.1-helpers
Apr 17, 2026
Merged

refactor(training-agent): adopt @adcp/client 5.1 state-store helpers#2283
bokelley merged 2 commits into
mainfrom
bokelley/adcp-state-5.1-helpers

Conversation

@bokelley

@bokelley bokelley commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #2263. Now that @adcp/client 5.1 landed (via #2277), retire our hand-rolled state-store helpers in favor of the SDK equivalents.

Closes #2269.

What changed

  • serializeSession / deserializeSession → SDK structuredSerialize / structuredDeserialize. Disk format is now the SDK canonical form — tagged __adcpType envelopes for Map/Date instead of our Object.fromEntries + ISO-string hand-roll. Net ~40 fewer lines of type-coercion code.
  • 5 MB size cap: removed our manual check in flushDirtySessions. PostgresStateStore.put / InMemoryStateStore.put enforce DEFAULT_MAX_DOCUMENT_BYTES at the boundary and throw StateError('PAYLOAD_TOO_LARGE') automatically.
  • RESERVED_KEYS prototype-pollution filter: removed. structuredDeserialize walks tagged envelopes, not Object.entries on untrusted JSONB, and SDK key validation (validateId/validateCollection) protects the store boundary.

Migration

server/src/db/migrations/411_clear_training_sessions.sql:

DELETE FROM adcp_state WHERE collection = 'training_sessions';

The serialization format changed, so pre-existing sessions won't deserialize cleanly with the new code. Clearing is safe:

  • Training sessions are sandbox state with a 1-hour TTL
  • No cross-system references into training_sessions rows
  • Equivalent to a single-machine restart — on deploy, any in-flight storyboard run will lose session context and need to restart (same behavior as any Fly redeploy historically)

Other collections in adcp_state (if any are ever added) are unaffected.

Kept (out of scope for this PR)

  • sessionKeyFromArgs + safeKey stay — they're coupled to this repo's AccountRef/BrandRef shapes and provide the training-specific "collapse to open:default" UX on malformed input. Would naturally retire when we adopt createAdcpServer's resolveSessionKey hook, which is a larger migration.
  • Whole-session JSONB blob layout — store.scoped(sessionKey) is a per-entity pattern; adopting it would require rewriting every handler to use ctx.store collections individually. Tracked as a future refactor.

Test plan

  • npm run typecheck — clean
  • npx vitest run server/tests/unit/training-agent.test.ts — 302 / 302 pass (including read-only-no-flush, snapshot round-trip, DNS-case-insensitive session keys)
  • npx vitest run --dir tests/ — 587 / 587 pass
  • Local roundtrip: create + get across separate HTTP requests against npm start / standalone — passes
  • Precommit hook (full unit suite + typecheck) green
  • CI green
  • Post-deploy: storyboards still green on test-agent.adcontextprotocol.org (no regression), migration 411 runs, fresh sessions in new format

Line count

🤖 Generated with Claude Code

bokelley and others added 2 commits April 17, 2026 15:50
@bokelley @claude
refactor: adopt @adcp/client 5.1 state-store helpers in training agent
40c1812 
Closes #2269.

- serializeSession / deserializeSession replaced by structuredSerialize /
  structuredDeserialize from @adcp/client/server. Disk format is now the
  SDK canonical form (tagged __adcpType envelopes for Map/Date) instead
  of our hand-rolled Object.fromEntries + ISO-string approach.
- MAX_SESSION_JSON_BYTES manual check in flushDirtySessions removed.
  PostgresStateStore.put / InMemoryStateStore.put enforce
  DEFAULT_MAX_DOCUMENT_BYTES (5 MB) at the boundary and throw
  StateError('PAYLOAD_TOO_LARGE') automatically.
- RESERVED_KEYS prototype-pollution filter removed. structuredDeserialize
  walks tagged envelopes, not Object.entries on untrusted JSONB, and the
  SDK's validateId / validateCollection protects the store boundary.

Migration 411 clears `training_sessions` rows on deploy because the
serialization format changed. Training sessions are sandbox state with
a 1-hour TTL — losing them is equivalent to a machine restart.

Net: server/src/training-agent/state.ts goes from 384 → 356 lines.
Tests: 302/302 training-agent + 587/587 tests/ suite pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley @claude
review: document invariants + pin disk format with explicit test
8505d5e 
Applying feedback from code + security review on #2283:

- Document the security invariant around RESERVED_KEYS removal: structured-
  Deserialize DOES walk untrusted JSONB, so prototype pollution is closed
  at the INPUT boundary (handlers never spread raw request payloads into
  session state) — not by the SDK. Comment warns maintainers to keep that
  invariant if adding new handlers.
- Document the Map-field maintenance invariant on deserializeSession: each
  Map in SessionState needs an explicit `asMap(...)` override, or it will
  hydrate as a raw envelope object.
- Lift the "products dropped from persistence" intent onto the call site
  where the override happens (code-reviewer Should Fix #2).
- Add explicit disk-format test that verifies:
    - Maps serialize as { __adcpType: 'Map', entries: [...] }
    - Dates serialize as { __adcpType: 'Date', value: ISO }
    - Round-trip hydrates back to real Map/Date instances
  Pins the wire format against future SDK regressions.

303/303 training-agent tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 4171801 into main Apr 17, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Adopt @adcp/client 5.1 state-store helpers once upstream #554 lands

1 participant

@bokelley