Skip to content

feat(manager): add WhatsApp channel and durable run token accounting - #148

Merged
Asaf-prog merged 20 commits into
mainfrom
codex/fix-140-cancelled-token-budget
Oct 23, 2026
Merged

Asaf-prog merged 20 commits into
mainfrom
codex/fix-140-cancelled-token-budget

Conversation

@Asaf-prog

@Asaf-prog Asaf-prog commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

This PR fixes lifetime token-budget accounting for unfinished runs and adds the opt-in WhatsApp channel through pilot packaging. Its effective diff against main includes the implementations from #149 and #150: 120 files, 13,676 additions and 178 deletions, verified at head 05b31af49d556f55af134e3d5907252a26e90f62.

Scope and rationale

This is a combined manager integration PR. The WhatsApp channel uses the generic conversation-turn preparation and existing conversation/run lifecycle, while the usage ledger preserves consumed work independently of whether an execution produces an assistant message. Approval of this PR covers both accounting and the complete channel stack below.

  • Run and conversation accounting ([Bug][P2] Cancelled runs consume tokens without counting toward the conversation budget #140): retain cumulative usage for failed/cancelled runs, pending approvals, resumed runs and inactive conversation branches. Reconcile missed retention writes from authoritative run records and retain charges after run-record expiry. Atomic (run_id, leg_id) claims prevent duplicate charges after ambiguous commits while charging distinct overlapping legs. Manager pending_approval and final SSE frames expose token totals.
  • Generic channel prerequisites (WA-01–WA-02): idempotent turn preparation, SQL channel storage, durable sender/conversation binding and a client-owned identity resolver boundary.
  • WhatsApp intake and execution (WA-03–WA-04): signed, bounded webhook normalization; durable intake before acknowledgement; an opt-in worker with exclusive ownership, fresh identity checks, conservative restart recovery, bounded concurrency, health projection and cancellation-safe shutdown.
  • Delivery and controls (WA-05): immutable ordered text parts, recipient/window/suppression checks, callback projection and deterministic stop/start/reset/help controls. Controls remain active whenever the worker runs, including sender-disabled startup without outbound credentials; missing durable control storage rejects startup.
  • Pilot operation (WA-06): sanitized status/rejection logs, a loopback-bound example deployment, runbook and fake-provider round-trip/restart rehearsal.

Intake, worker and outbound sender are separately disabled by default. Provider-specific code stays in agent_manager; the engine receives its existing typed requests and context.

Acceptance criteria

  • Consumed run usage survives failure, cancellation, approval suspension/resume, branch edits and run-record expiry without charging a leg twice.
  • Ambiguous-commit retries and overlapping legs are covered for built-in repositories; external run adapters cannot silently inherit unsafe unkeyed accounting.
  • Both manager stream endpoints expose cumulative usage in pending-approval events.
  • Authenticated webhook intake is durable and idempotent, with bounded validation and sanitized rejections.
  • Worker ownership, fresh identity checks, restart recovery and shutdown behavior have deterministic regression coverage.
  • Delivery preserves ordering, authorization and suppression; uncertain sends are held for reconciliation.
  • Worker-on/sender-off stop suppresses later engine execution; start affects only new input, reset starts a fresh conversation, and help never invents a contact. Restart into sending mode does not deliver old suppressed replies.
  • Status logs, runbook and pilot configuration match implemented behavior; fake-provider rehearsal covers intake, answer, delivery callback and restart without resending.
  • Full quality gate, offline example validation and GitHub CI pass.

Contracts and migration

  • Apply Alembic revisions 0006–0010: lifetime usage/backfill and leg keys, turn-preparation receipts, channel storage, webhook receipts and delivery/control storage.
  • External RunRepository adapters must implement the required abstract add_leg_token_usage operation with an atomic keyed claim plus usage update. Delegation to unkeyed add_token_usage does not satisfy the contract.
  • The manager SSE token fields are additive. The channel adds its opt-in webhook and configuration; agent YAML and the engine request ports retain their existing contracts.
  • Design and compatibility details: ADR 0002, ADRs 0006–0010, implementation plan, runbook, and tasks 0013–0021 plus the two PR-148 follow-up tasks.

Validation

Fresh verification on 2026-10-23, at head 05b31af:

  • make check: 1,456 tests passed, 27 dependency warnings; formatting (361 files), lint, mypy (357 files) and generated-stub checks passed.
  • make validate: starter and enterprise examples passed.
  • agentctl validate examples/whatsapp-pilot/agents.yml: passed.
  • git diff --check: passed; the verified checkout remained clean and at the same commit.
  • GitHub CI run 39812408471: both check and ui passed, including widget type checks, bundle freshness, unit tests and Playwright end-to-end tests.
  • All five inline review threads are resolved, including the atomic accounting contract and sender-disabled opt-out finding. The accounting and paused-control follow-ups record regression evidence; the WhatsApp review documents cover the channel implementation and lifecycle corrections.

Tests use fake engine/model/provider boundaries and temporary local SQLite. No live model, Meta delivery, production credentials or live PostgreSQL server was exercised by this verification.

Remaining pilot work and operating limits

Organization Google/JWT account linking, automatic channel-data retention and the authorized real-number smoke test remain pending. The pilot allowlist is an example identity plugin. Deployment requires a trusted identity implementation and current account/protocol verification. Failed, blocked and unknown delivery work needs operator reconciliation; durable conversations do not make process-local approvals/tool replay durable. This PR does not activate a real account or deploy a service.

Fixes #140

Retain cumulative per-run charges independently of assistant messages, including
failed and cancelled executions, pending approvals, and resumed runs. Preserve
charges across branch edits and run-record expiry with a durable usage ledger
and migration backfill for existing runs.

Reconcile missed retention writes from authoritative run records, prevent
cancellation and post-commit readback failures from charging a leg twice, and
include usage in pending-approval stream events. Add regression coverage for
memory and SQL repositories and document the accounting behavior.

Validation: make check passed with 1126 tests.

Fixes #140
@Asaf-prog
Asaf-prog requested a review from AmitAvital1 October 14, 2026 21:06
@Asaf-prog Asaf-prog self-assigned this Oct 14, 2026
Asaf-prog and others added 5 commits October 16, 2026 14:57
Repository-contract fixtures and SQL conversation-creation helpers created aiosqlite engines without disposing them, causing intermittent worker-thread exceptions. Each test now owns and disposes its engines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An opt-in idempotency key claims a durable receipt together with the user message, so a retried preparation recovers the same turn instead of appending another. Adds saved-response recovery for channel workers and migration 0007.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Provider-independent inbox, sender bindings, execution claims, webhook receipts, ordered outbox and delivery events, worker ownership, and the channel identity port, with migrations 0008-0010.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Opt-in authenticated Cloud API intake, a supervised channel turn worker with conservative recovery, provider delivery with text controls, and health reporting for a stopped worker.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Integration plan, ADRs 0007-0010, implementation tasks 0014-0017 and 0019-0020, review records, and index entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread src/agent_engine/engine/langgraph/engine.py
Comment thread src/agent_engine/engine/langgraph/engine.py

@AmitAvital1 AmitAvital1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found two blocking correctness issues in the token accounting path. Details and reproductions are attached to the exact lines.

Asaf-prog and others added 2 commits October 16, 2026 15:47
The worker logs one `channel status:` line per configured interval with queue
counts, the oldest unfinished and unsent ages, and unknown, failed, and blocked
reply counts. The read is bounded to five seconds so monitoring cannot stall
scheduling or a graceful drain. Every rejected webhook request logs a fixed
reason. No line contains message text, sender IDs, tokens, or provider IDs.

Adds EXTRA_WHATSAPP_STATUS_LOG_SECONDS (default 60).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The runbook covers preflight, deployment, staged enablement and pausing,
health and status logs, restart recovery, rollback, retention, and the
real-number smoke-test checklist. The example runs agent-manager on durable
storage with a tool-free assistant and an allowlist identity resolver; its port
is bound to loopback because the web conversation API is anonymous by default.

A fake-provider rehearsal proves that paused intake survives restarts, is
answered and sent exactly once, projects a delivered callback, and is not
resent after a further restart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sage

A leg's usage write retried after a commit whose acknowledgement was lost
added the same tokens twice (5 became 10). Each leg now records the run totals
seen before its first attempt and writes with compare-and-add, so a retry after
an ambiguous commit adds nothing. RunRepository gains
add_token_usage_if_unchanged with a default for existing adapters; the SQL
adapter performs the compare and add in one UPDATE ... RETURNING.

The manager's SSE schema dropped input_tokens and output_tokens, so
/messages/stream and /decision/stream omitted the documented pending usage.
Both fields are now mapped for every stream event, with an SSE-level
regression test covering both endpoints.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Asaf-prog
Asaf-prog requested a review from AmitAvital1 October 16, 2026 13:05
Comment thread src/agent_engine/engine/langgraph/engine.py Outdated

@AmitAvital1 AmitAvital1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The two original findings are fixed and their threads are resolved. The second full review found one new blocking accounting race; details and a reproduction are attached inline.

Compare-and-add could not tell a retried leg from a different leg of the same
run. RunLifecycle.begin() accepts an existing run_id, so two overlapping runs
read the same totals and one leg was dropped (10 tokens consumed, 5 stored).

Each leg write now carries a random leg key. The SQL adapter claims the key in
agent_run_token_legs and adds the usage in one transaction, and the in-memory
adapter records the key with the totals, so a retry after an unacknowledged
commit adds nothing while overlapping legs are each charged. The table is
part of migration 0006, which this pull request introduces.
RunRepository.add_leg_token_usage replaces add_token_usage_if_unchanged; its
default still serves existing adapters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Asaf-prog
Asaf-prog requested a review from AmitAvital1 October 17, 2026 11:09
Comment thread src/agent_engine/runs/repository.py

@AmitAvital1 AmitAvital1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The overlapping-leg race is fixed and I resolved that thread. One blocking contract issue remains: RunRepository.add_leg_token_usage documents at-most-once behavior but its default implementation is non-idempotent, so existing external adapters can silently double-charge after an ambiguous commit. Please require adapters to implement the atomic keyed operation instead of falling back to add_token_usage.

Make add_leg_token_usage abstract and remove its unsafe additive fallback.
Update the registration spy, reject legacy adapters without the keyed
operation, and verify that the base implementation cannot charge usage.

Document the external-adapter upgrade requirement in ADR 0002.
Validation: 101 focused tests and make check (1135 tests) passed using
an isolated temporary DATABASE_URL for the local CLI checks.
…dget' into codex/review-whatsapp-149

# Conflicts:
#	tasks/README.md
feat(manager): WhatsApp pilot packaging and operations (WA-06)
feat(manager): WhatsApp Cloud API channel
Comment thread src/agent_manager/composition.py

@AmitAvital1 AmitAvital1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The atomic run-leg contract is fixed, and I resolved that thread. The PR now also contains the full #149/#150 WhatsApp stack: 118 files and 13,456 added lines, although this PR and its follow-up task are scoped to token accounting. That imported code still has a confirmed opt-out bypass when the worker runs with sending disabled; I left the exact inline finding. Please remove the unrelated stack from this PR, or fix and explicitly review it here before merging.

Bind controls whenever the worker runs, independently of outbound transport.
Keep worker-only startup compatible with absent sender credentials and support
contact, and reject missing durable control storage. Cover signed stop/start,
reset, help and live restart behavior through the real HTTP/SQL composition.

@AmitAvital1 AmitAvital1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The opt-out bug is fixed and I resolved its thread. One general blocker remains: the title and description still present this as the #140 token-accounting fix with 1,126 tests, while the effective PR now includes the full WhatsApp stack (120 files, 13,676 additions) and 1,456 tests. Please either remove the unrelated stack or update the PR scope, rationale, acceptance criteria, and validation so reviewers can see exactly what this merge is approving.

@Asaf-prog Asaf-prog changed the title fix(manager): account for unfinished run token usage feat(manager): add WhatsApp channel and durable run token accounting Oct 23, 2026

@AmitAvital1 AmitAvital1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the final combined scope at 05b31af. All five inline findings are fixed and resolved; the updated title and description now accurately define the accounting and WhatsApp integration being merged, including contracts, acceptance criteria, migrations, limits, and validation. Focused regressions, the full local suite except the confirmed environment-only Click baseline failure, example validation, diff checks, and GitHub check/ui jobs all pass. Approved.

@Asaf-prog
Asaf-prog merged commit 6b9ada3 into main Oct 23, 2026
3 checks passed
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.

[Bug][P2] Cancelled runs consume tokens without counting toward the conversation budget

2 participants