Skip to content

fix(compadre): bound delivery retries and keep request images out of Postgres - #55

Merged
imsherrill merged 4 commits into
mainfrom
fix/compadre-reliability
Sep 11, 2026
Merged

imsherrill merged 4 commits into
mainfrom
fix/compadre-reliability

Conversation

@imsherrill

@imsherrill imsherrill commented Sep 11, 2026 •

Copy link
Copy Markdown

Request persistence failures could strand a running turn, large inline images amplified Postgres writes, and failed native delivery could retry indefinitely while central state stayed working.

Persist private S3 references before creating a run; bound delivery retries and expose a durable blocked state; retry transient central SQL errors with stable command IDs. Add API-key-protected, canary-only fault injection and inspection through the canonical central command/read path, with runbook and skill updates.

Verification: targeted controller, central ingestion, native event, Temporal policy, and real Postgres persistence tests passed. Controller and server typechecks passed. Production preflight found 535 stored requests and zero inline-image records, so no backfill is currently needed; legacy inline reads/trimming were removed. Existing Temporal histories retain the required replay-safe versioning.

After merge, verify both Render deployments and exercise live API canaries; no browser verification is claimed. Recheck inline records after old-instance drain.

Model/harness: GPT-5 / Codex.

# Conflicts:
#	apps/server/src/compadre/NativeThreadEvents.test.ts
#	apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts
#	apps/server/src/orchestration/decider.ts
#	docs/internals/compadre-fork.md
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XL labels Sep 11, 2026
@imsherrill

Copy link
Copy Markdown
Author

Release validation: controller CI passed on 508ce64 (full controller tests, Postgres persistence and migration checks). Local integrated central tests passed (74), focused controller regression tests passed, Temporal policy tests passed, server/controller typechecks passed, and scoped lint passed.

Root Check repeats the same 18 lint errors already present on merged PR #54, in untouched Sidebar, backup/MCP tests, hosted CLI PATH, and install-hosted-gh files. Not suppressing rules or folding unrelated cleanup into this reliability change. Remaining root test results are being reviewed before merge.

Production preflight found zero inline-image request rows. The two confirmed orphaned runs from today had neither a workflow nor persisted request; normal finalization now records aborted state and closes their streams without deleting history. Live fault canaries follow deployment.

@github-actions

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 13.5 KiB — 15.1 KiB ✅
Codex Thread snapshot wire — 7.1 KiB — 7.3 KiB ✅
Codex Live turn WebSocket wire — 6.5 KiB — 7.8 KiB ✅
Codex Live turn WebSocket decoded — 56.3 KiB — 66.4 KiB ✅
Codex Live turn messages — 9 — 21 ✅
Claude Total thread wire — 13.6 KiB — 15.1 KiB ✅
Claude Thread snapshot wire — 7.1 KiB — 7.3 KiB ✅
Claude Live turn WebSocket wire — 6.5 KiB — 7.8 KiB ✅
Claude Live turn WebSocket decoded — 57.2 KiB — 66.4 KiB ✅
Claude Live turn messages — 9 — 21 ✅

Baseline: unavailable · PR result: 508ce64 · Source CI: failure

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 111.9 KiB
  • Claude decoded thread snapshot: 112.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@imsherrill

Copy link
Copy Markdown
Author

Final CI comparison: all controller/client checks and server shards 1–2 passed. Server shard 3 has exactly the same 15 ProviderCommandReactor test failures as merged PR #54 (compared normalized failing test names). Root Check has the same 18 pre-existing lint errors. No newly failing tests or lint errors were found. Proceeding with the authorized rollout, without changing or disabling those unrelated baseline checks.

@imsherrill
imsherrill merged commit eebb368 into main Sep 11, 2026
17 of 19 checks passed
@imsherrill

Copy link
Copy Markdown
Author

Live verification completed on web eebb368 / API bd59531 (replacement instance r29v6), using the API only:

  • Codex gpt-5.6-sol: ten valid PNGs / 20,980,470 bytes persisted as ten S3 references; request JSON 5,610 bytes, physical PG value 1,600 bytes. Private compadre bucket object Get/Head succeeded, AES256 encryption and SHA-256 matched. Run 40cdc102-9ef5-4be1-8f1c-6b42d3b686f1 completed with one VERIFICATION_OK response.
  • Claude claude-sonnet-4-6: healthy run completed. Injected permanent rejection failed the delivery workflow on attempt 1, retained cursor 22, and set central error/activeTurnId null despite the completed worker. Explicit resume advanced cursor to 38 and delivered its answer with the SAME run 378d87a4-b859-4151-8213-0c74a674b2da.
  • Codex transient error recovered on attempt 2; run 84078a13-9216-43d1-bfb6-7c8518785eca completed, central ready, cursor 34, one answer.
  • Claude lost acknowledgment after real central commit recovered on attempt 2; cursor 38 to 54, run d537bfe0-7af5-40ca-9a6e-24ace3b95a65 completed, three total answers for three successful turns, six unique message IDs. Duplicate message submission returned accepted/duplicate and did not launch again.
  • UUID canary c0decafe-f094-4f80-9cbb-172ddd324709 injected request-write failure: no run, no workflow, central error. Unauthorized verification GET returned 401.
  • Temporal history confirms maximumAttempts=5 and NativeDeliveryRejectedError non-retryable. No application Postgres kill/recovery log during the image/fault tests.

Live checks caught and fixed the helper UUID issue (#56) and a missing narrowly scoped native-input S3 grant (documented in #57). The first API deployment failed on a transient Temporal connection outage affecting both old/new instances; retry succeeded after dependency recovery. Canaries are being stopped; final readback follows the docs-only deployment. No browser rendering or shared-database crash was simulated.

@imsherrill

Copy link
Copy Markdown
Author

Final post-deployment readback passed (2026-09-11). API is live at aa57ba9 on srv-da73bogae00c738hgl3g-b6cf59d94-64gf5; web remains on eebb368. API /health and web /healthz return 200. Both successful Codex/Claude canaries retain completed runs, ready sessions with no active turn, cancelled delivery workflows and zero pending activities; persistence-failure canary is stopped with no run/workflow. All injected faults are cleared. Message counts remain 5/6/1 after deployment. Read-only Postgres inspection confirms zero inline-image request records; the 20,980,470-byte image test retains ten references in 5,610 bytes of JSON. Three ordinary delivery consumers are STARTED and heartbeating on the new instance (attempt 4 reflects deployment handoffs, not an active retry loop). Follow-ups #56 and #57 are merged and deployed. Earlier evidence and known baseline CI failures remain documented above. No browser verification was performed; live verification used the authenticated API, durable workflow state, S3 integrity checks, and central read models. Database capacity/alerting changes were not part of this rollout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant