Skip to content

fix: prevent reconciliation check-in false deaths - #1916

Merged
simple-agent-manager[bot] merged 4 commits into
mainfrom
sam/stop-reconciliation-check-false-bj75fw
Aug 26, 2026
Merged

fix: prevent reconciliation check-in false deaths#1916
simple-agent-manager[bot] merged 4 commits into
mainfrom
sam/stop-reconciliation-check-false-bj75fw

Conversation

@simple-agent-manager

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

Copy link
Copy Markdown
Contributor

Summary

  • Prevent reconciliation_checkin attention markers from failing healthy tasks while current-generation ACP activity is prompting/runtime work is active or settling.
  • Route attention-expiry reconciliation failures through the shared terminal transition contract so D1 gets status events, timestamps, execution-step cleanup, trigger/wait hooks, workspace/admission cleanup, and idempotency/supersession behavior.
  • Archived the older backlog item for attention-expiry task status events because this PR implements it.
  • Control-loop cost: the new expiry guard is local to the same ProjectData DO alarm and runs one bounded indexed lookup for each expired reconciliation_checkin candidate before deciding whether to renew or fail. It does not expand the alarm candidate query.

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)

Validation commands run:

  • pnpm --filter @simple-agent-manager/api test -- attention-expiry task-terminal-transition — passed, 3 files / 12 tests.
  • pnpm --filter @simple-agent-manager/api test -- reconciliation — passed, 6 files / 164 tests.
  • pnpm typecheck && pnpm lint — passed; lint warnings were existing baseline warnings in packages/acp-client and apps/web.
  • pnpm --filter @simple-agent-manager/api test — passed, 606 files / 8254 tests.
  • pnpm format:check — passed.
  • git diff --check — passed.
  • pnpm quality:observability-noise — passed; D1 telemetry skipped because OBSERVABILITY_DB_ID was not set and Workers telemetry unavailable with 403, final result reported no significant log noise.

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, projects, and settings; confirmed no regressions in core flows (pages load, data displays, navigation works, no new console errors)
  • New feature/fix verified on staging — the specific changes in this PR work correctly on the live staging environment (describe what was tested below)
  • 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

  • Staging deploy passed on commit b940ed42b3ad311a1949bda6200af98d9f91bad4: https://github.com/raphaeltm/simple-agent-manager/actions/runs/32919441324
  • The deploy workflow completed Cloudflare deploy, health check, and smoke tests successfully; smoke-tests job reported 12 passed (1.4m).
  • Local live staging Playwright smoke run passed 12/12 against https://app.sammy.party and https://api.sammy.party, using token-login auth. Covered dashboard load, project navigation, settings navigation, API tokens tab, unauthenticated redirect, API health, CORS, and console-error checks.
  • Read-only staging API verification: token-login returned 200, /health returned status: healthy, /api/admin/tasks/stuck returned zero active transient tasks.
  • Feature-specific verification note: the changed behavior is a ProjectData DO alarm path for expired internal reconciliation_checkin markers. Staging has no public non-destructive endpoint to mint expired markers or advance a DO alarm clock. The exact alarm path was therefore verified with real-SQL unit coverage for: active current-generation ACP prompting renewal, active runtime-work renewal, stale activity expiry, heartbeat-only expiry, genuine no-liveness terminalization, terminal status-event/timestamp/step semantics, and idempotent retry behavior. The deployed staging branch passed health/smoke/API checks after those changes.

UI Compliance Checklist (Required for UI changes)

  • 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. Reconciliation creates a reconciliation_checkin marker in apps/api/src/durable-objects/project-data/reconciliation.ts using the shared reconciliation deadline.
  2. ProjectData alarm enters ProjectData.alarm() in apps/api/src/durable-objects/project-data/index.ts and calls processExpiredAttentionMarkers() with project-scoped hooks.
  3. apps/api/src/durable-objects/project-data/attention-expiry.ts inspects expired markers; for reconciliation_checkin, it reads current-generation session activity from local ProjectData SQL.
  4. If current-generation activity is prompting/recovering or runtime work is active/settling, attention-expiry.ts renews the marker instead of failing the task.
  5. If no current activity evidence exists, attention-expiry.ts calls handleReconciliationDeadTarget().
  6. apps/api/src/durable-objects/project-data/reconciliation-dead-target.ts calls transitionTaskToTerminal() in apps/api/src/services/task-terminal-transition.ts.
  7. transitionTaskToTerminal() performs the terminal D1 task update with status event, timestamp/step semantics, idempotency, supersession fence, trigger sync, admission cancellation, workspace stop, and task-wait hook.
  8. On successful terminal transition, ProjectData fails the local session, schedules summary sync, records activity, and resolves the marker.

Untested Gaps

The internal DO alarm cannot be safely forced on staging through a public API without manufacturing and possibly killing real staging tasks. This gap is covered by real-SQL unit tests against the same ProjectData modules and by live staging deployment/smoke/API health checks proving the branch runs in Cloudflare.

Post-Mortem (Required for bug fix PRs)

What broke

A healthy Codex task could be failed by reconciliation check-in expiry after 60 seconds of silent tool work, even while ACP was actively prompting/working and the node heartbeat was fresh.

Root cause

reconciliation_checkin markers were resolved only by persisted user/assistant messages. Prompt acceptance, current-generation ACP activity, runtime work, and node heartbeats did not participate in the check-in response decision. The expiry path then wrote terminal failure directly to tasks, bypassing the shared terminal status contract.

Class of bug

State interaction race / liveness-classification bug, compounded by an inconsistent terminal writer.

Why it wasn't caught

The pre-existing tests did not discriminate between delivered active ACP work, heartbeat-only liveness, and genuine no-delivery/no-liveness expiry. They also did not assert terminal task status event/timestamp/execution-step contract behavior for attention-expiry failures.

Process fix included in this PR

Added focused regression coverage in:

  • apps/api/tests/unit/durable-objects/attention-expiry.test.ts
  • apps/api/tests/unit/services/task-terminal-transition.test.ts
  • apps/api/tests/unit/durable-objects/reconciliation.test.ts

No rule update was needed; this PR implements the existing terminal-writer requirements from .claude/rules/66-task-terminal-status-events.md.

Post-mortem file

Task file: tasks/archive/2026-08-26-fix-reconciliation-checkin-attention-expiry.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 — N/A; all completed.
Reviewer Status Outcome
cloudflare-specialist PASS D1 writes are project-scoped, batched where consistency matters, and no wrangler changes.
constitution-validator PASS No new hardcoded URLs/timeouts/limits; reconciliation deadline remains env-configured.
test-engineer PASS Added real-SQL vertical tests for alarm behavior and terminal contract coverage.
task-completion-validator PASS Checklist and acceptance criteria map to code/tests; heartbeat-only gap was addressed.

Exceptions (If any)

  • Scope: none.
  • Rationale: none.
  • Expiration: none.

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: /reliability/audits/production-stability-audit-2026-08-25.md, file id 01M0XK1XYNB34YB0X6Z41HM542.
  • Existing code paths and rules: apps/api/src/durable-objects/project-data/attention-expiry.ts, apps/api/src/durable-objects/project-data/reconciliation.ts, apps/api/src/durable-objects/project-data/message-persistence.ts, .claude/rules/47-control-loop-io-budget.md, .claude/rules/59-task-supersession.md, .claude/rules/66-task-terminal-status-events.md.

Codebase Impact Analysis

Affected components:

  • apps/api/src/durable-objects/project-data/attention-expiry.ts
  • apps/api/src/durable-objects/project-data/reconciliation-dead-target.ts
  • apps/api/src/durable-objects/project-data/index.ts
  • apps/api/src/services/task-terminal-transition.ts
  • API unit tests for ProjectData attention expiry, reconciliation, and terminal transition behavior.

Documentation & Specs

  • No public docs/spec updates required; this is an internal ProjectData reconciliation/terminalization bug fix.
  • Archived implemented backlog item tasks/backlog/2026-08-06-attention-expiry-task-status-events.md to tasks/archive/2026-08-06-attention-expiry-task-status-events.md.

Constitution & Risk Check

Checked Constitution Principle XI / no hardcoded values: the reconciliation deadline remains environment-configured through the existing shared constant/helper; no new URLs, timeouts, limits, or identifiers were hardcoded into business logic. Main risk is control-loop overhead; mitigated by a same-DO local SQL guard only on expired check-in markers, preserving genuine no-liveness expiry.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@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/stop-reconciliation-check-false-bj75fw (33bab24) with main (b966b05)

Open in CodSpeed

@simple-agent-manager
simple-agent-manager Bot force-pushed the sam/stop-reconciliation-check-false-bj75fw branch from 367ce7b to 33bab24 Compare August 26, 2026 02:22
@sonarqubecloud

Copy link
Copy Markdown

@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Post-rebase verification note: branch is rebased on origin/main at b966b05 and PR CI is green on head 33bab24. I attempted fresh post-rebase staging deploys (32922917151, 32923601168), but both were cancelled before jobs started by staging workflow concurrency churn from newer branch deploys. Prior staging deploy 32919441324 succeeded for this PR before the rebase; the rebase only incorporated PR #1905, which touched the separate task-runtime-liveness path per the audit sequencing note.

@simple-agent-manager
simple-agent-manager Bot merged commit 689e114 into main Aug 26, 2026
27 checks passed
@simple-agent-manager
simple-agent-manager Bot deleted the sam/stop-reconciliation-check-false-bj75fw branch August 26, 2026 02:43
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