Skip to content

fix: use canonical instant execution step - #1919

Merged
simple-agent-manager[bot] merged 4 commits into
mainfrom
sam/small-focused-fix-instant-8snbfk
Aug 26, 2026
Merged

fix: use canonical instant execution step#1919
simple-agent-manager[bot] merged 4 commits into
mainfrom
sam/small-focused-fix-instant-8snbfk

Conversation

@simple-agent-manager

@simple-agent-manager simple-agent-manager Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix Instant runtime’s successful launch writer to persist the canonical shared execution step running instead of out-of-contract agent_running.
  • Add taskExecutionStep() at the shared contract boundary so write sites can opt into compile-time rejection of invalid execution-step literals where Drizzle exposes the column as string | null.
  • Add an API writer-inventory guard for the Instant launch writer and update the existing recovery test seed to the canonical contract value.

Validation

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • Additional validation run (if applicable)
  • If this PR changes candidate selection for a sweep/cron/alarm loop (WHERE clause, status set, join, or equivalent), expected candidate volume and worst-case per-candidate cost are stated in the summary or validation notes (see .claude/rules/47-control-loop-io-budget.md)

Additional validation:

  • Guard-red verified before fixing the writer: pnpm --filter @simple-agent-manager/api test -- tests/unit/services/instant-execution-step-contract.test.ts failed while agent_running was still present.
  • Focused passing check: pnpm --filter @simple-agent-manager/shared build && pnpm --filter @simple-agent-manager/api test -- tests/unit/services/instant-execution-step-contract.test.ts tests/unit/services/instant-session.test.ts
  • Package checks: pnpm --filter @simple-agent-manager/shared typecheck && pnpm --filter @simple-agent-manager/api typecheck && pnpm --filter @simple-agent-manager/shared lint && pnpm --filter @simple-agent-manager/api lint
  • Full repo gate: pnpm lint && pnpm typecheck && pnpm test && pnpm build
  • Source contract gate: pnpm quality:source-contract-tests
  • Note: a local worker-pool-only retry of pnpm --filter @simple-agent-manager/api test:workers -- tests/workers/instant-runtime-recovery-failure.test.ts timed out during local workerd/vitest-pool startup. No test assertion failed; full repo tests and live staging verification passed.
  • N/A for control-loop candidate volume: this PR does not change a sweep/cron/alarm candidate query.

Staging Verification (REQUIRED for all code changes — merge-blocking)

  • Staging deployment greenDeploy Staging workflow triggered manually and passed for this branch
  • Live app verified via Playwright — logged into app.sammy.party (staging) using test credentials and actively tested the application
  • Existing workflows confirmed working — navigated dashboard via Playwright/API-backed browser context, listed projects/profiles, confirmed no new console errors
  • New feature/fix verified on staging — Instant session start persisted and exposed the canonical step
  • Infrastructure verification completed — N/A: no infra changes
  • Mobile and desktop verification notes added for UI changes — N/A: no UI changes

Staging Verification Evidence

  • GitHub Actions Deploy Staging run 32928732612 completed successfully for branch sam/small-focused-fix-instant-8snbfk at current head 980b36e20ebdbdd0f5b5d18dd9168b3860d87eaa. The deploy/app/API/health jobs passed in the original attempt; the failed smoke-tests job was rerun and passed at 2026-08-26T05:01Z, leaving the run conclusion success.
  • Staging smoke-tests job 98066892079 passed the Playwright smoke suite on rerun.
  • Targeted API/D1 verifier created staging session e042e533-035b-4671-9715-7801767e56de with task 01M0Y7BEFHKM6J2S89PEWCQZQF using cf-container profile 01KX2N3REWHCAT1QDG927BR52M, then stopped it successfully.
  • Staging D1 task row returned execution_step = 'running'.
  • Public API task detail returned executionStep = 'running'.
  • Cleanup evidence: session stop returned HTTP 200 with workspaceDeleted = true.

UI Compliance Checklist (Required for UI changes)

  • Mobile-first layout verified — N/A: no UI changes
  • Accessibility checks completed — N/A: no UI changes
  • Shared UI components used or exception documented — N/A: no UI changes
  • Playwright visual audit run locally — N/A: no UI changes

End-to-End Verification (Required for multi-component changes)

  • Data flow traced from user input to final outcome with code path citations (see .claude/rules/10-e2e-verification.md)
  • Capability test exercises the complete happy path across system boundaries
  • All spec/doc assumptions about existing behavior verified against code (not just "read the code")
  • If any gap exists between automated test coverage and full E2E, manual verification steps documented below

Data Flow Trace

  1. User sends Instant chat request through POST /api/projects/:projectId/sessions/start in apps/api/src/routes/chat-start.ts.
  2. Route persists initial task with executionStep: 'instant_persistence', then calls acceptInstantSession().
  3. apps/api/src/services/instant-session.ts completes ACP startup and updates the task to status: 'in_progress', executionStep: taskExecutionStep('running').
  4. API mappers in apps/api/src/lib/mappers.ts preserve running because it is a member of TASK_EXECUTION_STEPS from packages/shared/src/types/task.ts.
  5. Staging verifier confirmed both D1 execution_step and public API executionStep are running for a live Instant session.

Untested Gaps

No known functional gap for this fix. The local worker-pool-only command timed out before executing assertions, but the same seeded invalid literal was corrected and full repo tests plus live staging covered the contract behavior.

Post-Mortem (Required for bug fix PRs)

What broke

Instant runtime marked successfully launched tasks with executionStep: 'agent_running', which is not in the shared execution-step contract. API mappers converted that invalid literal to null, so Instant task progress was silently lost in clients.

Root cause

Commit d590ec5626 (Make every chat task-backed and forkable, #1572) introduced the raw Instant writer literal without a guard tying it to TASK_EXECUTION_STEPS.

Class of bug

Contract drift between a D1 string column writer and the shared TypeScript/API execution-step contract.

Why it wasn't caught

The write site was typed as a raw Drizzle string field, so TypeScript accepted invalid literals. One existing test seeded agent_running, reinforcing the invalid value instead of rejecting it.

Process fix included in this PR

Added a focused writer-inventory guard in apps/api/tests/unit/services/instant-execution-step-contract.test.ts and a shared taskExecutionStep() helper for write sites that need compile-time literal validation.

Post-mortem file

tasks/archive/2026-08-26-instant-execution-step-contract.md

Specialist Review Evidence (Required for agent-authored PRs)

  • All local reviewers completed and findings addressed before merge
  • If any reviewer did NOT complete: needs-human-review label added and merge deferred to human
Reviewer Status Outcome
task-completion-validator PASS Planned audit deliverables, implementation checklist, git diff, and test evidence align.
test-engineer PASS Guard-red behavior, canonical writer assertion, and corrected recovery seed are sufficient for this focused contract fix.
constitution-validator PASS No hardcoded URL/timeout/limit/config violations introduced; running is a semantic enum value from the shared contract.
cloudflare-specialist PASS No D1 schema, migration, wrangler, KV, R2, or infra changes required; local worker-pool timeout noted but staging passed.

Exceptions (If any)

  • Scope: Worker-pool-only local retry timed out during startup.
  • Rationale: Full repo tests, source-contract tests, staging deploy, and targeted live D1/API verification passed; no assertion failure indicated a code regression.
  • Expiration: Revisit only if CI/staging worker-pool evidence fails.

Agent Preflight (Required)

  • Preflight completed before code changes

Classification

  • external-api-change
  • cross-component-change
  • business-logic-change
  • public-surface-change
  • docs-sync-change
  • security-sensitive-change
  • ui-change
  • infra-change

External References

  • SAM library audit file /reliability/audits/production-stability-audit-2026-08-25.md, file id 01M0XK1XYNB34YB0X6Z41HM542, P2 priority table and sequenced step 6.
  • N/A external web docs: this fix uses repo-local shared contract and runtime code only.

Codebase Impact Analysis

  • packages/shared/src/types/task.ts: canonical execution-step contract and typed helper.
  • packages/shared/src/types/index.ts: helper export.
  • apps/api/src/services/instant-session.ts: Instant successful launch task writer.
  • apps/api/tests/unit/services/instant-execution-step-contract.test.ts: writer-inventory guard.
  • apps/api/tests/unit/services/instant-session.test.ts: canonical writer assertion.
  • apps/api/tests/workers/instant-runtime-recovery-failure.test.ts: canonical seed value.

Documentation & Specs

N/A: small runtime contract fix; no public contract/docs/spec behavior changed. The API now returns the already-canonical shared execution-step value instead of mapper-normalized null.

Constitution & Risk Check

Checked Principle XI (No Hardcoded Values). The only new literal used in runtime logic is running, a canonical shared enum member, not a configurable deployment value. Risk is low and scoped to Instant task progress reporting.

@codspeed-hq

codspeed-hq Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing sam/small-focused-fix-instant-8snbfk (980b36e) with main (05ea68f)

Open in CodSpeed

@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Refreshing PR event after correcting preflight metadata; no code change.

@sonarqubecloud

Copy link
Copy Markdown

@simple-agent-manager
simple-agent-manager Bot merged commit 05e4d22 into main Aug 26, 2026
32 of 33 checks passed
@simple-agent-manager
simple-agent-manager Bot deleted the sam/small-focused-fix-instant-8snbfk branch August 26, 2026 05:45
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