Skip to content

feat(chat): action decisions across requests and turn cost policy - #205

Closed
EmersonBraun wants to merge 12 commits into
mainfrom
EmersonBraun/chat-t04
Closed

EmersonBraun wants to merge 12 commits into
mainfrom
EmersonBraun/chat-t04

Conversation

@EmersonBraun

@EmersonBraun EmersonBraun commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Durable action decisions and turn cost policy (RF-17–29), with the second independent verification corrections. Current head: c5386dd3c3ef6db80557d97349922d24e5c7308c.

Resulting behavior

Protocol v1 adds client.action.decide, action-decide-v1, quota utilization/warning, and safe typed errors. The handler consumes upstream durable ToolDecisionStore and CostStore; PostgreSQL uses conditional claims/settlement. Parts delivery remains in stacked #206.

Criteria and current local evidence

Criterion Change and evidence
C1: two pending calls in one model turn putPending accepts evolving messages without overwriting the original snapshot. Shared real-handler regression passes with in-memory stores and real PostgreSQL.
C2: new proposal in a later turn while the earlier one is pending The same shared regression passes in memory and PostgreSQL; both calls persist and the first snapshot stays unchanged.
C3: reject decided tool-call ID reuse Existing terminal-ID rejection tests remain passing in the chat suite and PostgreSQL contract. Claimed and terminal rows are rejected.
C4: accurate quota documentation Changeset, server guide and ADR-0037 now state: charge actual usage, or the reservation when usage is unknown, after model dispatch even on failure/cancel; release only before model start. Checked against settleCost; existing cost tests pass.
C5: unique custom IDs Server guide and ADR warn that constant createId can reuse reservations and bypass quota; no runtime API change.
C6: settled-decision replay during active model resume The real PostgreSQL race contract exposed a retry that could replay incomplete memory and alter the active cursor. The handler checks the current lease before replay; the delayed-resume regression and real race contract pass.

pnpm --filter @agentskit/chat lint, pnpm --filter @agentskit/chat-server lint, and pnpm test:storage:pg:types passed. Chat coverage suite: 131 passed; server: 61 passed, 1 expected failure, 2 skipped (98.54% lines); pnpm test:storage:pg --maxWorkers=2: 2 passed, including 100 claim races and 100 concurrent-handler rounds plus both proposal regressions. All coverage floors passed.

Documentation impact: quota behavior, ID uniqueness and upstream adoption record. Definition of Done for this worker: corrections, mapped checks, propagation to #206, push, green current CI, PR evidence and node_modules cleanup; broader human reviews remain pending.

Verification scope and decisions

Local budget: package chat/server typechecks and coverage suites with --maxWorkers=2, plus the existing localhost PostgreSQL 16 acceptance contract and its typecheck. Observed machine load stayed below 60; no local full-repository test run, external provider call, merge, or publication. Builds are only prerequisites of the documented package test commands.

Upstream adoption: inspected published core 1.15.0 controller.persistPending, ToolDecisionStore, Message, ContentPart, AdapterFactory, and reused controller execution/resume plus serialize/deserialize exports. The application PostgreSQL store preserves insert-if-absent semantics; the handler owns quota policy, turn leases and reference delivery. No upstream code, dependency, coverage floor or provider protocol was copied or changed. Adoption records and ADR-0037/0038 describe the choices.

  • Partially validated: PostgreSQL is real localhost Postgres 16 on Node; no claim of Neon/Hyperdrive or production certification. Upload acceptance uses deterministic stores/adapters.
  • Not analyzed: real provider billing, remote databases, workerd and S3/R2/MinIO acceptance in this follow-up. Existing historical evidence is not current evidence.
  • Blocked: none in the local checks; current CI is listed below.
  • Not applicable: no UI source changed. Original human visual approval and independent ADR/evidence review remain pending for the broader PR; the worker does not approve its own evidence.

Conservative defaults: keep the first pending snapshot, reject claimed/terminal ID reuse, return the existing 409 for retry during an active decision turn, and retain bounded upload delivery. No merge or publication. All 21 task-installed node_modules directories were removed; the final worktree/diff is clean and pre-existing containers/artifacts are preserved. Automatic approval review rejected deletion of the task stash (task_c0d3825b51a3 parts fixes) because approval was required while this session cannot request it; the stash is retained as a cleanup limitation.

Current CI

All executed checks passed on c5386dd3c3ef6db80557d97349922d24e5c7308c; no failed or pending check remains. Auxiliary CodeQL is neutral; its JavaScript/TypeScript analysis passed.

Check State Evidence
CodeQL NEUTRAL job
quality SUCCESS job
browser-e2e SUCCESS job
ink-pty SUCCESS job
Analyze (javascript-typescript) / Analyze (javascript-typescript) SUCCESS job
Dependency Review / dependency-review SUCCESS job
doc-advisory SUCCESS job
pnpm audit / audit (.) SUCCESS job

These checks validate their declared scope; broader human visual and independent evidence review remain pending. No merge or publication was performed.

Protocol event client.action.decide, handler decisions and cost options,
Drizzle decision store, tests, ADR-0037 and docs. Manifests reference
unreleased upstream versions, so the lockfile is not regenerated yet.
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Doc Bridge advisory (Layer 1)

Repository: AgentsKit-io/agentskit-chat; PR: 205.
Base: 9c431e9; head: c5386dd; index source: ci-built.

Advisory only. Blocking gates are reported independently; interpretation and acceptance remain human-owned.

Findings: 0.
Pending: 0; updated-in-this-change: 0.

Pending

Updated in this change — confirm the update

Coverage gaps / policy exclusions: not-analyzed: 16; partial: 373.
Analysis ran under the service profile; full coverage detail is in the diff artifact.

  • signature: partial; Exported non-callable bindings or dynamic initializers are not analyzed. Return types are inferred or undeclared.

Policy routing: excluded: 0; pending-version: 0; generator: 0; routed-to-L2: 0.
Historical, generated and ineligible version findings are summarized here; full details remain in the diff artifact. Changed references and migration context require review; no edits are proposed.

- keep protocol fixture positions stable; append the decide fixture
- register an action confirmation before upstream proposeToolCall resolves
- cover the Drizzle decision store's boundary checks
- accept content parts in the controlled Ink example
- refresh the public API snapshot, claim ledgers and README evidence
- raise the server bundle budget for decisions and cost
@EmersonBraun

Copy link
Copy Markdown
Member Author

Local gates against the unreleased upstream (still draft)

@agentskit/core 1.15.0 and @agentskit/observability 0.13.0 are not on npm yet, so pnpm install --frozen-lockfile cannot pass here and the lockfile in this branch is unchanged. Everything below ran at 27b890c on macOS (Node v26.8.1, pnpm 10.33.0) with an uncommitted pnpm-workspace.yaml override pointing both packages at pnpm pack tarballs of agentskit#1835 (88722d4d). Nothing from that override is committed.

CI step Result
pnpm lint (types + cross-platform) exit 0
pnpm test:conformance exit 0
pnpm exec changeset status exit 0
pnpm test (20 workspace projects, coverage floors, bundle/ecosystem/release tests) exit 0
pnpm build exit 0
pnpm check:public-api ok — 2 packages, 14 export subpaths after an additive snapshot update (8 new exports, none removed)
pnpm conformance:gate, pnpm bundle:budget exit 0
doc-bridge: index twice + cmp, gate, conformance, doctor, index test exit 0
ecosystem:claims:check, ecosystem:adoption:check, check:readme-standard, test:readme-standard, release-gate exit 0
pnpm test:storage:pg:types, pnpm test:storage:pg (PostgreSQL 16.15 (Debian 16.15-1.pgdg13+2)) 2/2
Storage + decision contract in workerd (wrangler dev --local, same database) HTTP 200, 8 criteria
pnpm test:pty (Ink in a real PTY) 10/10
pnpm build then pnpm test:e2e (Playwright, Chromium) 52 passed
pnpm test:expo (web + iOS export) exit 0
node scripts/pack-release.mjs fails, expected: it installs the packed tarball from the public registry, which has no @agentskit/core 1.15 yet

What the first full run found, and the fixes in 27b890c

  1. Protocol fixtures shifted. The new client.action.decide fixture was inserted at index 1 of validTurnEventFixtures, which moved every later fixture and broke 3 protocol tests plus the positional lookups in five renderer conformance tests. The fixture is now appended, so existing positions hold.
  2. Confirmation coordinator bypassed with core 1.15. Core now persists a pending call before proposeToolCall resolves, so the UI shows the call first. createActionConfirmation only recorded the token after the promise resolved; a decision in that window found no record and the renderers fell back to the controller's direct approve/deny, skipping TTL, session check and claimStatus. It showed up as a Svelte coverage drop (74.7% → 73.5% functions, same as main once fixed). The coordinator now registers the confirmation before it proposes; two regression tests cover the early decision and the failed proposal.
  3. @agentskit/chat coverage floor. createDrizzleDecisionStore had no unit tests (only the Postgres contract), which took the package under its floors. Boundary checks added; the SQL behaviour stays in tests/storage-pg.
  4. Ink example did not compile. ControlledChatActions.send follows the upstream signature and now receives string | ContentPart[]. The example handles both, and the changeset says so for hosts that implement send.
  5. Cross-platform check. Two new tests used split('\n'); they now split on /\r?\n/.
  6. Generated evidence was stale. ADR count 36 → 37 in the claim ledgers and the README proof table; README standard freshness refreshed; public API snapshot updated.
  7. Server bundle budget. dist/index.js is 40,080 bytes against a 40,000 budget with decisions and cost in. I raised the budget to 60,000 rather than shave strings. This is a threshold change, so please confirm it in review.

One run of pnpm test also stopped on a 10 s afterAll cleanup timeout in packages/cli/tests/generated-projects.test.ts (all 30 tests had passed; the machine was under load). It passed on the next run without changes. Two earlier E2E runs failed for reasons of my own making, not of the code: I ran test:pty (which rebuilds @agentskit/chat and drops the assembled renderer subpaths) before E2E without the pnpm build CI runs in between, and the retry then hit the stale Vite dependency cache that run left behind. In CI order, from a clean cache, all 52 pass.

Before this leaves draft

  • Publish core 1.15.0 and observability 0.13.0, then pnpm install to regenerate the lockfile and re-run CI.
  • Not exercised: Neon, Hyperdrive.
Test totals from the green pnpm test
Test Files  6 passed (6)	Tests  99 passed (99)
Test Files  6 passed (6)	Tests  128 passed (128)
Test Files  3 passed (3)	Tests  26 passed (26)
Test Files  1 passed (1)	Tests  20 passed (20)
Test Files  3 passed (3)	Tests  26 passed (26)
Test Files  2 passed (2)	Tests  30 passed (30)
Test Files  1 passed (1)	Tests  10 passed (10)
Test Files  3 passed (3)	Tests  33 passed (33)
Test Files  3 passed (3)	Tests  45 passed (45)
Test Files  2 passed (2)	Tests  23 passed (23)
Test Files  6 passed (6)	Tests  55 passed | 1 expected fail | 2 skipped (58)
Test Files  2 passed (2)	Tests  25 passed (25)
Test Files  2 passed (2)	Tests  27 passed (27)
Test Files  1 passed (1)	Tests  1 passed (1)
Test Files  2 passed (2)	Tests  27 passed (27)
Test Files  1 passed (1)	Tests  8 passed (8)
Test Files  2 passed (2)	Tests  13 passed (13)
Test Files  1 passed (1)	Tests  2 passed (2)
workerd contract response
{"runtime":"workerd-request-client","criteria":["RF-13 tenant isolation (load/save/delete; no list API)","RF-14 100 concurrent CAS races, exactly one winner","RF-15 durable lease, overlapping HTTP 200 + 409 SESSION_BUSY, release/recovery 200","409 SESSION_CONFLICT preserved at handler boundary (real stale-cursor SQL update)","pre-aborted operation rejected","tenant and session isolation; insert-if-absent pending snapshot","RF-19/RF-21 atomic claim: 100 races of 8 claims, one winner each","RF-18/RF-19/RF-20 handler: 100 rounds of 4 concurrent approvals across fresh handlers, one execution and one model resume each"]}
Bundle budget
ok   @agentskit/chat-protocol            55320 / 120000
ok   @agentskit/chat                    174927 / 180000
ok   @agentskit/chat-server              40080 / 60000
ok   @agentskit/chat-react               23034 / 80000
ok   @agentskit/chat-react-native        22368 / 80000
ok   @agentskit/chat-ink                 21930 / 80000
ok   @agentskit/chat-vue                 20002 / 80000
ok   @agentskit/chat-solid               33411 / 80000
ok   @agentskit/chat-devtools             7235 / 40000
ok   @agentskit/chat-cli                 23918 / 80000
ok   @agentskit/chat-svelte              23186 / 120000
ok   @agentskit/chat-angular             37522 / 100000

@EmersonBraun
EmersonBraun marked this pull request as ready for review October 10, 2026 14:27
EmersonBraun added a commit that referenced this pull request Oct 10, 2026
…ferenced parts (#205, #206)

* docs: record track 04 stage status

* wip(chat): action decisions across requests and turn cost policy

Protocol event client.action.decide, handler decisions and cost options,
Drizzle decision store, tests, ADR-0037 and docs. Manifests reference
unreleased upstream versions, so the lockfile is not regenerated yet.

* docs: record track 04 checkpoint PRs

* fix(chat): pass the gates against core 1.15

- keep protocol fixture positions stable; append the decide fixture
- register an action confirmation before upstream proposeToolCall resolves
- cover the Drizzle decision store's boundary checks
- accept content parts in the controlled Ink example
- refresh the public API snapshot, claim ledgers and README evidence
- raise the server bundle budget for decisions and cost

* docs: keep README review dates; refresh only the changed source hashes

* wip(chat): deliver referenced parts to the adapter

* test(chat): verify referenced uploads across Node and workerd

* fix: meter dispatched turns and reject reused decisions

* fix: bound upload delivery and preserve class adapters

* test: supply a real request to class adapter regression

* fix: deliver parts through session-aware adapters

* docs: refresh reviewed server README evidence

* fix: resolve published upstream dependencies in lockfile

* fix: exclude generated Expo and Wrangler artifacts from scans

* fix: preserve pending decisions across evolving transcripts

* fix: defer decision replay until the active turn finishes

* fix: separate upload budgets and support frozen adapters

* docs: refresh server README source hash after budget correction
@EmersonBraun

Copy link
Copy Markdown
Member Author

Incluído no #206, que foi mergeado por squash em main (este repositório não aceita merge commit, e o #206 já continha todos os commits deste PR). Revisão independente em três rodadas: os dois furos de cota e o putPending foram corrigidos e verificados.

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