diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot.trace.json b/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot.trace.json new file mode 100644 index 00000000..35c7ee5d --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot.trace.json @@ -0,0 +1,77 @@ +{ + "version": "1.0.0", + "id": "632139b6-0bda-4558-a8ad-31a2aa1fa606", + "timestamp": "2026-08-18T09:19:00.798Z", + "trajectory": "traj_7j4ob7xwg0ot", + "files": [ + { + "path": "packages/engine/src/engine/__tests__/workspace.test.ts", + "conversations": [ + { + "contributor": { + "type": "ai" + }, + "ranges": [ + { + "start_line": 9, + "end_line": 15, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 24, + "end_line": 30, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 84, + "end_line": 98, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 128, + "end_line": 134, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 147, + "end_line": 153, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 180, + "end_line": 212, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + } + ] + } + ] + }, + { + "path": "packages/engine/src/engine/workspace.ts", + "conversations": [ + { + "contributor": { + "type": "ai" + }, + "ranges": [ + { + "start_line": 14, + "end_line": 24, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 73, + "end_line": 90, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + }, + { + "start_line": 195, + "end_line": 221, + "revision": "f2b371bd1267a9bd41a2221b4e110772c40258a1" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot/summary.md b/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot/summary.md new file mode 100644 index 00000000..b9ebb7c3 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot/summary.md @@ -0,0 +1,45 @@ +# Trajectory: Address late PR #333 review findings + +> **Status:** ✅ Completed +> **Task:** relaycast#333 +> **Confidence:** 96% +> **Started:** August 18, 2026 at 11:15 AM +> **Completed:** August 18, 2026 at 11:19 AM + +--- + +## Summary + +Separated exact-pair readback outages from proven ID mismatches, decoupled collision tests from Snowflake internals, and corrected validation provenance. + +**Approach:** Standard approach + +--- + +## Key Decisions + +### Model committed-pair readback as match, mismatch, or unavailable +- **Chose:** Model committed-pair readback as match, mismatch, or unavailable +- **Reasoning:** A unique conflict plus a failed read cannot establish an identifier collision; only a successful mismatched read can. The unavailable state must preserve the documented indeterminate 503 outcome. + +### Stub createWorkspace's generated IDs in collision tests +- **Chose:** Stub createWorkspace's generated IDs in collision tests +- **Reasoning:** Explicit IDs exercise the real call boundary without depending on the Snowflake singleton's private sequence arithmetic or call ordering. + +--- + +## Chapters + +### 1. Work +*Agent: default* + +- Model committed-pair readback as match, mismatch, or unavailable: Model committed-pair readback as match, mismatch, or unavailable +- Stub createWorkspace's generated IDs in collision tests: Stub createWorkspace's generated IDs in collision tests +- The readback-outage regression failed before the fix with workspace_id_collision and now returns the documented 503; collision stubs exercise explicit IDs, and focused plus full-engine gates pass. + +--- + +## Artifacts + +**Commits:** f2b371b +**Files changed:** 2 diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot/trajectory.json b/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot/trajectory.json new file mode 100644 index 00000000..60466059 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_7j4ob7xwg0ot/trajectory.json @@ -0,0 +1,97 @@ +{ + "id": "traj_7j4ob7xwg0ot", + "version": 1, + "task": { + "title": "Address late PR #333 review findings", + "source": { + "system": "plain", + "id": "relaycast#333" + } + }, + "status": "completed", + "startedAt": "2026-08-18T09:15:58.632Z", + "completedAt": "2026-08-18T09:19:00.741Z", + "agents": [ + { + "name": "default", + "role": "lead", + "joinedAt": "2026-08-18T09:16:06.956Z" + } + ], + "chapters": [ + { + "id": "chap_uhrrwzdpr86v", + "title": "Work", + "agentName": "default", + "startedAt": "2026-08-18T09:16:06.956Z", + "endedAt": "2026-08-18T09:19:00.741Z", + "events": [ + { + "ts": 1787044566957, + "type": "decision", + "content": "Model committed-pair readback as match, mismatch, or unavailable: Model committed-pair readback as match, mismatch, or unavailable", + "raw": { + "question": "Model committed-pair readback as match, mismatch, or unavailable", + "chosen": "Model committed-pair readback as match, mismatch, or unavailable", + "alternatives": [], + "reasoning": "A unique conflict plus a failed read cannot establish an identifier collision; only a successful mismatched read can. The unavailable state must preserve the documented indeterminate 503 outcome." + }, + "significance": "high" + }, + { + "ts": 1787044567210, + "type": "decision", + "content": "Stub createWorkspace's generated IDs in collision tests: Stub createWorkspace's generated IDs in collision tests", + "raw": { + "question": "Stub createWorkspace's generated IDs in collision tests", + "chosen": "Stub createWorkspace's generated IDs in collision tests", + "alternatives": [], + "reasoning": "Explicit IDs exercise the real call boundary without depending on the Snowflake singleton's private sequence arithmetic or call ordering." + }, + "significance": "high" + }, + { + "ts": 1787044740501, + "type": "reflection", + "content": "The readback-outage regression failed before the fix with workspace_id_collision and now returns the documented 503; collision stubs exercise explicit IDs, and focused plus full-engine gates pass.", + "raw": { + "focalPoints": [ + "readback-outage", + "collision-semantics", + "test-isolation", + "provenance" + ], + "confidence": 0.96 + }, + "significance": "high", + "tags": [ + "focal:readback-outage", + "focal:collision-semantics", + "focal:test-isolation", + "focal:provenance", + "confidence:0.96" + ] + } + ] + } + ], + "retrospective": { + "summary": "Separated exact-pair readback outages from proven ID mismatches, decoupled collision tests from Snowflake internals, and corrected validation provenance.", + "approach": "Standard approach", + "confidence": 0.96 + }, + "commits": [ + "f2b371b" + ], + "filesChanged": [ + "packages/engine/src/engine/__tests__/workspace.test.ts", + "packages/engine/src/engine/workspace.ts" + ], + "projectId": "AgentWorkforce/relaycast", + "tags": [], + "_trace": { + "startRef": "af9c61fd33dfdd8e3ba3578a7d5d850804121596", + "endRef": "f2b371bd1267a9bd41a2221b4e110772c40258a1", + "traceId": "632139b6-0bda-4558-a8ad-31a2aa1fa606" + } +} \ No newline at end of file diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68.trace.json b/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68.trace.json new file mode 100644 index 00000000..1fec26a7 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68.trace.json @@ -0,0 +1,25 @@ +{ + "version": "1.0.0", + "id": "4cce42d1-fcdf-41c1-8b46-7b792240ef1e", + "timestamp": "2026-08-18T09:04:41.523Z", + "trajectory": "traj_a8ldf56q5b68", + "files": [ + { + "path": "packages/engine/src/engine/__tests__/workspace.test.ts", + "conversations": [ + { + "contributor": { + "type": "ai" + }, + "ranges": [ + { + "start_line": 179, + "end_line": 185, + "revision": "7b4794c14a95f657bb925612bef0b6de36bf569f" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68/summary.md b/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68/summary.md new file mode 100644 index 00000000..ea645631 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68/summary.md @@ -0,0 +1,40 @@ +# Trajectory: Stabilize PR #333 lost-response recovery regression + +> **Status:** ✅ Completed +> **Task:** relaycast#333 +> **Confidence:** 96% +> **Started:** August 18, 2026 at 11:03 AM +> **Completed:** August 18, 2026 at 11:04 AM + +--- + +## Summary + +Stabilized the lost-response retry regression under full-suite load and revalidated all local gates. + +**Approach:** Standard approach + +--- + +## Key Decisions + +### Auto-advance the fake retry clock +- **Chose:** Auto-advance the fake retry clock +- **Reasoning:** The test started timer advancement before asynchronous key hashing had necessarily scheduled the first retry; auto-advancing fake time preserves fast deterministic backoff under full-suite load. + +--- + +## Chapters + +### 1. Work +*Agent: default* + +- Auto-advance the fake retry clock: Auto-advance the fake retry clock +- The full monorepo test exposed a timer-start race in the lost-response regression; auto-advancing fake time removed the race, and focused, full-engine, build, lint, and test gates now pass. + +--- + +## Artifacts + +**Commits:** 7b4794c +**Files changed:** 1 diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68/trajectory.json b/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68/trajectory.json new file mode 100644 index 00000000..fa9bddc9 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_a8ldf56q5b68/trajectory.json @@ -0,0 +1,82 @@ +{ + "id": "traj_a8ldf56q5b68", + "version": 1, + "task": { + "title": "Stabilize PR #333 lost-response recovery regression", + "source": { + "system": "plain", + "id": "relaycast#333" + } + }, + "status": "completed", + "startedAt": "2026-08-18T09:03:51.053Z", + "completedAt": "2026-08-18T09:04:41.430Z", + "agents": [ + { + "name": "default", + "role": "lead", + "joinedAt": "2026-08-18T09:03:56.543Z" + } + ], + "chapters": [ + { + "id": "chap_7yrkxksb7ani", + "title": "Work", + "agentName": "default", + "startedAt": "2026-08-18T09:03:56.543Z", + "endedAt": "2026-08-18T09:04:41.430Z", + "events": [ + { + "ts": 1787043836544, + "type": "decision", + "content": "Auto-advance the fake retry clock: Auto-advance the fake retry clock", + "raw": { + "question": "Auto-advance the fake retry clock", + "chosen": "Auto-advance the fake retry clock", + "alternatives": [], + "reasoning": "The test started timer advancement before asynchronous key hashing had necessarily scheduled the first retry; auto-advancing fake time preserves fast deterministic backoff under full-suite load." + }, + "significance": "high" + }, + { + "ts": 1787043881146, + "type": "reflection", + "content": "The full monorepo test exposed a timer-start race in the lost-response regression; auto-advancing fake time removed the race, and focused, full-engine, build, lint, and test gates now pass.", + "raw": { + "focalPoints": [ + "test-determinism", + "retry-coverage", + "verification" + ], + "confidence": 0.96 + }, + "significance": "high", + "tags": [ + "focal:test-determinism", + "focal:retry-coverage", + "focal:verification", + "confidence:0.96" + ] + } + ] + } + ], + "retrospective": { + "summary": "Stabilized the lost-response retry regression under full-suite load and revalidated all local gates.", + "approach": "Standard approach", + "confidence": 0.96 + }, + "commits": [ + "7b4794c" + ], + "filesChanged": [ + "packages/engine/src/engine/__tests__/workspace.test.ts" + ], + "projectId": "AgentWorkforce/relaycast", + "tags": [], + "_trace": { + "startRef": "b2eadefa957626305a54d641d4baa3989f395341", + "endRef": "7b4794c14a95f657bb925612bef0b6de36bf569f", + "traceId": "4cce42d1-fcdf-41c1-8b46-7b792240ef1e" + } +} \ No newline at end of file diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_dcfqsp8aaew3/summary.md b/.agentworkforce/trajectories/completed/2026-08/traj_dcfqsp8aaew3/summary.md new file mode 100644 index 00000000..b5ec4501 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_dcfqsp8aaew3/summary.md @@ -0,0 +1,53 @@ +# Trajectory: Finish relaycast#333 review threads + +> **Status:** ✅ Completed +> **Task:** relaycast#333 +> **Confidence:** 92% +> **Started:** August 18, 2026 at 10:27 AM +> **Completed:** August 18, 2026 at 10:36 AM + +--- + +## Summary + +At commit `f3af1d8`, closed the eight PR #333 findings present during this run: +required atomic workspace writes, added mid-batch and bare-handle coverage, +normalized/redacted HTTP errors, retried plain-object D1 causes, recovered exact +committed pairs after retry exhaustion, documented the 503 contract, and +corrected changelog/PR claims. + +Local focused, engine, and non-container monorepo gates were green at that +commit. A separate injected container proof against the starting revision +`1795e4e5` had demonstrated the `status: 0` leak; `f3af1d8` changes that code and +its regression passes, but that separate injected proof was not rerun as part +of this trajectory. + +**Approach:** Standard approach + +--- + +## Key Decisions + +### Use isolated PR worktree and require atomic workspace writes +- **Chose:** Use isolated PR worktree and require atomic workspace writes +- **Reasoning:** The supplied checkout is on an unrelated branch; workspace creation must reject handles without transaction or batch capability instead of silently degrading to sequential writes. + +### Recover exact committed workspace pairs after retry exhaustion +- **Chose:** Recover exact committed workspace pairs after retry exhaustion +- **Reasoning:** The generated workspace id, channel id, and API key remain available inside the same call; exact row readback can distinguish the reviewed commit-then-exhaust case without claiming whole-request idempotency. + +### Document storage exhaustion as an indeterminate outcome +- **Chose:** Document storage exhaustion as an indeterminate outcome +- **Reasoning:** Workspace names are intentionally non-unique and a total write-plus-read outage cannot prove absence of a commit, so README, OpenAPI, changelogs, and PR wording must not promise arbitrary retry deduplication or credit the stopped incident. + +--- + +## Chapters + +### 1. Work +*Agent: default* + +- Use isolated PR worktree and require atomic workspace writes: Use isolated PR worktree and require atomic workspace writes +- Recover exact committed workspace pairs after retry exhaustion: Recover exact committed workspace pairs after retry exhaustion +- Document storage exhaustion as an indeterminate outcome: Document storage exhaustion as an indeterminate outcome +- At `f3af1d8`, all eight review findings present during the run were addressed; focused regressions, full engine tests, and non-container monorepo build/lint/test gates were green. diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_dcfqsp8aaew3/trajectory.json b/.agentworkforce/trajectories/completed/2026-08/traj_dcfqsp8aaew3/trajectory.json new file mode 100644 index 00000000..e36c86d2 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_dcfqsp8aaew3/trajectory.json @@ -0,0 +1,117 @@ +{ + "id": "traj_dcfqsp8aaew3", + "version": 1, + "task": { + "title": "Finish relaycast#333 review threads", + "source": { + "system": "plain", + "id": "relaycast#333" + } + }, + "status": "completed", + "startedAt": "2026-08-18T08:27:39.965Z", + "completedAt": "2026-08-18T08:36:09.939Z", + "agents": [ + { + "name": "default", + "role": "lead", + "joinedAt": "2026-08-18T08:27:40.106Z" + } + ], + "chapters": [ + { + "id": "chap_25p6a7fxjtra", + "title": "Work", + "agentName": "default", + "startedAt": "2026-08-18T08:27:40.106Z", + "endedAt": "2026-08-18T08:36:09.939Z", + "events": [ + { + "ts": 1787041660107, + "type": "decision", + "content": "Use isolated PR worktree and require atomic workspace writes: Use isolated PR worktree and require atomic workspace writes", + "raw": { + "question": "Use isolated PR worktree and require atomic workspace writes", + "chosen": "Use isolated PR worktree and require atomic workspace writes", + "alternatives": [], + "reasoning": "The supplied checkout is on an unrelated branch; workspace creation must reject handles without transaction or batch capability instead of silently degrading to sequential writes." + }, + "significance": "high" + }, + { + "ts": 1787042148836, + "type": "decision", + "content": "Recover exact committed workspace pairs after retry exhaustion: Recover exact committed workspace pairs after retry exhaustion", + "raw": { + "question": "Recover exact committed workspace pairs after retry exhaustion", + "chosen": "Recover exact committed workspace pairs after retry exhaustion", + "alternatives": [], + "reasoning": "The generated workspace id, channel id, and API key remain available inside the same call; exact row readback can distinguish the reviewed commit-then-exhaust case without claiming whole-request idempotency." + }, + "significance": "high" + }, + { + "ts": 1787042148934, + "type": "decision", + "content": "Document storage exhaustion as an indeterminate outcome: Document storage exhaustion as an indeterminate outcome", + "raw": { + "question": "Document storage exhaustion as an indeterminate outcome", + "chosen": "Document storage exhaustion as an indeterminate outcome", + "alternatives": [], + "reasoning": "Workspace names are intentionally non-unique and a total write-plus-read outage cannot prove absence of a commit, so README, OpenAPI, changelogs, and PR wording must not promise arbitrary retry deduplication or credit the stopped incident." + }, + "significance": "high" + }, + { + "ts": 1787042149022, + "type": "reflection", + "content": "At f3af1d8, all eight review findings present during the run were addressed; focused regressions, full engine tests, and non-container monorepo build/lint/test gates were green. The separate injected container proof at the starting revision was not rerun in this trajectory.", + "raw": { + "focalPoints": [ + "atomicity", + "error-redaction", + "retry-classification", + "API-contract" + ], + "confidence": 0.92 + }, + "significance": "high", + "tags": [ + "focal:atomicity", + "focal:error-redaction", + "focal:retry-classification", + "focal:API-contract", + "confidence:0.92" + ] + } + ] + } + ], + "retrospective": { + "summary": "At f3af1d8, closed the eight PR #333 findings present during this run: required atomic workspace writes, added mid-batch and bare-handle coverage, normalized/redacted HTTP errors, retried plain-object D1 causes, recovered exact committed pairs after retry exhaustion, documented the 503 contract, and corrected changelog/PR claims. Local non-container gates were green; the separate injected container proof at the starting revision was not rerun in this trajectory.", + "approach": "Standard approach", + "confidence": 0.92 + }, + "commits": [ + "f3af1d8" + ], + "filesChanged": [ + "CHANGELOG.md", + "README.md", + "openapi.yaml", + "packages/engine/CHANGELOG.md", + "packages/engine/src/engine/__tests__/workspace.test.ts", + "packages/engine/src/engine/workspace.ts", + "packages/engine/src/lib/__tests__/d1Retry.test.ts", + "packages/engine/src/lib/__tests__/httpError.test.ts", + "packages/engine/src/lib/d1Retry.ts", + "packages/engine/src/lib/httpError.ts", + "packages/engine/src/ports/database.ts" + ], + "projectId": "AgentWorkforce/relaycast", + "tags": [], + "_trace": { + "startRef": "1795e4e5f1c55a5a150580cb2c00d782c7a381f7", + "endRef": "f3af1d8890842e02b188e3a8174014cf6ff19f41" + } +} diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya.trace.json b/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya.trace.json new file mode 100644 index 00000000..53b0cf64 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya.trace.json @@ -0,0 +1,57 @@ +{ + "version": "1.0.0", + "id": "cc836759-efe6-44bf-9a8e-d7c5b2a8f820", + "timestamp": "2026-08-18T08:57:22.065Z", + "trajectory": "traj_zy9kpj1n3qya", + "files": [ + { + "path": "packages/engine/src/engine/__tests__/workspace.test.ts", + "conversations": [ + { + "contributor": { + "type": "ai" + }, + "ranges": [ + { + "start_line": 9, + "end_line": 15, + "revision": "67e2c911ca2bb2e68be8c32969023a7d0246573f" + }, + { + "start_line": 83, + "end_line": 98, + "revision": "67e2c911ca2bb2e68be8c32969023a7d0246573f" + }, + { + "start_line": 124, + "end_line": 174, + "revision": "67e2c911ca2bb2e68be8c32969023a7d0246573f" + } + ] + } + ] + }, + { + "path": "packages/engine/src/engine/workspace.ts", + "conversations": [ + { + "contributor": { + "type": "ai" + }, + "ranges": [ + { + "start_line": 81, + "end_line": 127, + "revision": "67e2c911ca2bb2e68be8c32969023a7d0246573f" + }, + { + "start_line": 161, + "end_line": 197, + "revision": "67e2c911ca2bb2e68be8c32969023a7d0246573f" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya/summary.md b/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya/summary.md new file mode 100644 index 00000000..eeb4dde0 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya/summary.md @@ -0,0 +1,44 @@ +# Trajectory: Address follow-up relaycast#333 review threads + +> **Status:** ✅ Completed +> **Task:** relaycast#333 +> **Confidence:** 94% +> **Started:** August 18, 2026 at 10:54 AM +> **Completed:** August 18, 2026 at 10:57 AM + +--- + +## Summary + +Closed follow-up PR #333 review: mismatched workspace/channel ID conflicts now +abort atomically, while exact committed retries recover by readback; added two +rollback regressions and reran the local status-zero regression added in +`f3af1d8`. The separate injected container proof at the starting revision was +outside this trajectory. + +**Approach:** Standard approach + +--- + +## Key Decisions + +### Replace idempotent no-op upserts with conflict-failing inserts +- **Chose:** Replace idempotent no-op upserts with conflict-failing inserts +- **Reasoning:** Exact lost-response recovery can be handled by validated readback after a unique conflict; allowing mismatched generated IDs to commit makes the post-commit collision check too late. + +--- + +## Chapters + +### 1. Work +*Agent: default* + +- Replace idempotent no-op upserts with conflict-failing inserts: Replace idempotent no-op upserts with conflict-failing inserts +- Follow-up review reproduced both mismatched generated-ID commits, then conflict-failing inserts plus exact readback made both rollback regressions pass. The local status-zero regression from `f3af1d8` also passed; the separate injected container proof was outside this trajectory. + +--- + +## Artifacts + +**Commits:** 67e2c91 +**Files changed:** 2 diff --git a/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya/trajectory.json b/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya/trajectory.json new file mode 100644 index 00000000..e059f3ea --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-08/traj_zy9kpj1n3qya/trajectory.json @@ -0,0 +1,83 @@ +{ + "id": "traj_zy9kpj1n3qya", + "version": 1, + "task": { + "title": "Address follow-up relaycast#333 review threads", + "source": { + "system": "plain", + "id": "relaycast#333" + } + }, + "status": "completed", + "startedAt": "2026-08-18T08:54:31.462Z", + "completedAt": "2026-08-18T08:57:21.992Z", + "agents": [ + { + "name": "default", + "role": "lead", + "joinedAt": "2026-08-18T08:54:31.673Z" + } + ], + "chapters": [ + { + "id": "chap_ntczadc5ajqz", + "title": "Work", + "agentName": "default", + "startedAt": "2026-08-18T08:54:31.673Z", + "endedAt": "2026-08-18T08:57:21.992Z", + "events": [ + { + "ts": 1787043271673, + "type": "decision", + "content": "Replace idempotent no-op upserts with conflict-failing inserts: Replace idempotent no-op upserts with conflict-failing inserts", + "raw": { + "question": "Replace idempotent no-op upserts with conflict-failing inserts", + "chosen": "Replace idempotent no-op upserts with conflict-failing inserts", + "alternatives": [], + "reasoning": "Exact lost-response recovery can be handled by validated readback after a unique conflict; allowing mismatched generated IDs to commit makes the post-commit collision check too late." + }, + "significance": "high" + }, + { + "ts": 1787043441903, + "type": "reflection", + "content": "Follow-up review reproduced both mismatched generated-ID commits, then conflict-failing inserts plus exact readback made both rollback regressions pass. The local status-zero regression from f3af1d8 also passed; the separate injected container proof was outside this trajectory.", + "raw": { + "focalPoints": [ + "identifier-collisions", + "lost-response-recovery", + "review-evidence" + ], + "confidence": 0.94 + }, + "significance": "high", + "tags": [ + "focal:identifier-collisions", + "focal:lost-response-recovery", + "focal:review-evidence", + "confidence:0.94" + ] + } + ] + } + ], + "retrospective": { + "summary": "Closed follow-up PR #333 review: mismatched workspace/channel ID conflicts now abort atomically, while exact committed retries recover by readback; added two rollback regressions and reran the local status-zero regression added in f3af1d8. The separate injected container proof at the starting revision was outside this trajectory.", + "approach": "Standard approach", + "confidence": 0.94 + }, + "commits": [ + "67e2c91" + ], + "filesChanged": [ + "packages/engine/src/engine/__tests__/workspace.test.ts", + "packages/engine/src/engine/workspace.ts" + ], + "projectId": "AgentWorkforce/relaycast", + "tags": [], + "_trace": { + "startRef": "f3af1d8890842e02b188e3a8174014cf6ff19f41", + "endRef": "67e2c911ca2bb2e68be8c32969023a7d0246573f", + "traceId": "cc836759-efe6-44bf-9a8e-d7c5b2a8f820" + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 29997577..d1794f67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,12 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Packages without a separate changelog are covered by the cross-package notes below. -## [Unreleased] +## [Unreleased - Patch] + +### Fixed + +- Workspace creation now requires atomic storage for the workspace and default + channel, and exhausted storage failures no longer expose database internals. ## [8.0.4] - 2026-08-17 diff --git a/README.md b/README.md index 572ca82f..b2f5c898 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,13 @@ return 400; already-claimed, non-offline, or concurrently changed records return Workspace names are not globally unique. Workspace creation is idempotent for the same workspace name and API key: repeating that combination returns the existing workspace instead of creating another one. +If workspace storage remains unavailable after its transient retries and the +server cannot confirm the committed workspace/channel pair, creation returns +`503 workspace_storage_unavailable`. The outcome is indeterminate: callers +must not assume that no workspace committed, especially when retrying without +a known workspace key. Database statements and bound parameters are never +included in this or other uncoded infrastructure error responses. + If you want an explicit SDK helper that tells you whether setup returned an existing workspace or created a new one, use `ensureWorkspace()`: ```ts diff --git a/openapi.yaml b/openapi.yaml index 3778c3b8..88b5d1d4 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1202,7 +1202,9 @@ paths: idempotent by workspace name for the same bearer workspace key: if you repeat the call with the same name and the same `Authorization: Bearer rk_*` key, the existing workspace is returned with a `200` response; otherwise a new workspace is created and returned with a - `201` response. + `201` response. If transient storage attempts are exhausted and the server cannot confirm + the committed workspace/channel pair, it returns `503 workspace_storage_unavailable`. + That outcome is indeterminate; callers must not assume no workspace committed. tags: - Workspaces security: @@ -1248,6 +1250,16 @@ paths: application/json: schema: $ref: '#/components/schemas/ErrorResponse' + '503': + description: > + `workspace_storage_unavailable` — transient storage attempts were + exhausted and the committed workspace/channel pair could not be + confirmed. The response does not expose database statements or + bound parameters. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' /workspaces/by-name/{name}: get: diff --git a/packages/engine/CHANGELOG.md b/packages/engine/CHANGELOG.md index 02724e82..230c6a55 100644 --- a/packages/engine/CHANGELOG.md +++ b/packages/engine/CHANGELOG.md @@ -7,7 +7,14 @@ See the [root changelog](../../CHANGELOG.md) for cross-package release highlight The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Patch] + +### Fixed + +- Workspace creation now requires atomic storage for the workspace and default + channel, and classifies exhausted transient writes with a safe storage error. +- Uncoded server errors no longer expose SQL statements or bound parameters in + API responses. ## [8.0.3] - 2026-08-15 diff --git a/packages/engine/src/engine.ts b/packages/engine/src/engine.ts index 3726a641..67cc2d9c 100644 --- a/packages/engine/src/engine.ts +++ b/packages/engine/src/engine.ts @@ -7,7 +7,7 @@ import type { EngineDeps } from './ports/index.js'; import { engineContext } from './middleware/engine-context.js'; import { loggerMiddleware } from './middleware/logger.js'; import { getRequestLogger, toErrorDetails } from './lib/logger.js'; -import { asCodedError } from './lib/httpError.js'; +import { asCodedError, safeClientErrorMessage } from './lib/httpError.js'; import { jsonError, jsonMalformedBody, jsonNotFound } from './lib/httpResponse.js'; import { requiredOriginInfo } from './lib/origin.js'; import { emitServerEvent } from './lib/serverTelemetry.js'; @@ -245,7 +245,7 @@ export function createEngine(deps: EngineDeps): Hono { return jsonError( c, error.code || 'internal_error', - error.message || 'Internal server error', + safeClientErrorMessage(error), status as ContentfulStatusCode, ); }); diff --git a/packages/engine/src/engine/__tests__/workspace.test.ts b/packages/engine/src/engine/__tests__/workspace.test.ts new file mode 100644 index 00000000..84e76e0f --- /dev/null +++ b/packages/engine/src/engine/__tests__/workspace.test.ts @@ -0,0 +1,242 @@ +import { eq } from 'drizzle-orm'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { makeNodeStack, type TestStack } from '../../__tests__/conformance/harness.js'; +import { channels, workspaces } from '../../db/schema.js'; +import type { + AtomicWrite, + BatchCapability, + EngineDb, + TransactionCapability, +} from '../../ports/database.js'; +import * as snowflake from '../snowflake.js'; +import { createWorkspace } from '../workspace.js'; + +describe('workspace creation durability', () => { + let stack: TestStack; + let db: EngineDb; + + beforeEach(() => { + stack = makeNodeStack(); + db = stack.runtime.handle.db as unknown as EngineDb; + delete (db as Partial).withTransaction; + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + stack?.close(); + }); + + function attachD1Batch(options: { + failAfterFirstStatement?: boolean; + failAfterLostResponse?: boolean; + failBeforeFirst?: boolean; + loseFirstResponse?: boolean; + }): () => number { + let calls = 0; + const sqlite = stack.runtime.handle.sqlite; + + (db as EngineDb & Partial).batch = async (statements) => { + calls += 1; + if (calls === 1 && options.failBeforeFirst) { + throw new Error('D1_ERROR: D1 DB is overloaded. Too many requests queued.'); + } + if (calls > 1 && options.failAfterLostResponse) { + throw new Error('D1_ERROR: D1 DB is overloaded. Too many requests queued.'); + } + + sqlite.exec('BEGIN IMMEDIATE'); + try { + const results: unknown[] = []; + for (const statement of statements as ReadonlyArray) { + results.push(await statement); + if (calls === 1 && options.failAfterFirstStatement && results.length === 1) { + throw new Error('D1_TYPE_ERROR: injected channel insert failure'); + } + } + sqlite.exec('COMMIT'); + if (calls === 1 && options.loseFirstResponse) { + throw new Error('D1_ERROR: Network connection lost.'); + } + return results; + } catch (error) { + if (sqlite.inTransaction) sqlite.exec('ROLLBACK'); + throw error; + } + }; + + return () => calls; + } + + async function expectOneCompleteWorkspace(workspaceId: string): Promise { + const workspaceRows = await db + .select() + .from(workspaces) + .where(eq(workspaces.id, workspaceId)); + const channelRows = await db + .select() + .from(channels) + .where(eq(channels.workspaceId, workspaceId)); + + expect(workspaceRows).toHaveLength(1); + expect(channelRows).toHaveLength(1); + expect(channelRows[0]?.name).toBe('general'); + } + + function useGeneratedPair(): { workspaceId: string; channelId: string } { + const pair = { + workspaceId: 'generated-workspace-id', + channelId: 'generated-channel-id', + }; + vi.spyOn(snowflake, 'generateId') + .mockReturnValueOnce(pair.workspaceId) + .mockReturnValueOnce(pair.channelId); + return pair; + } + + it('retries a transient D1 failure and commits workspace plus channel atomically', async () => { + const batchCalls = attachD1Batch({ failBeforeFirst: true }); + + const created = await createWorkspace(db, 'transient-retry'); + + expect(batchCalls()).toBe(2); + await expectOneCompleteWorkspace(created.workspace_id); + }); + + it('rolls back the workspace when the channel insert fails mid-batch', async () => { + attachD1Batch({ failAfterFirstStatement: true }); + + await expect(createWorkspace(db, 'mid-batch-failure')).rejects.toThrow( + 'injected channel insert failure', + ); + + expect(await db.select().from(workspaces)).toHaveLength(0); + expect(await db.select().from(channels)).toHaveLength(0); + }); + + it('rejects a bare database handle instead of degrading to sequential writes', async () => { + delete (db as Partial).batch; + + await expect(createWorkspace(db, 'non-atomic-handle')).rejects.toThrow( + 'Atomic write capability required', + ); + + expect(await db.select().from(workspaces)).toHaveLength(0); + expect(await db.select().from(channels)).toHaveLength(0); + }); + + it('rolls back the channel when the generated workspace id collides', async () => { + attachD1Batch({}); + const { workspaceId, channelId } = useGeneratedPair(); + await db.insert(workspaces).values({ + id: workspaceId, + name: 'unrelated-workspace', + apiKeyHash: 'unrelated-workspace-hash', + }); + + await expect(createWorkspace(db, 'workspace-id-collision')).rejects.toMatchObject({ + code: 'workspace_id_collision', + }); + + await expect(db.select().from(workspaces).where(eq(workspaces.id, workspaceId))).resolves.toMatchObject([ + { name: 'unrelated-workspace', apiKeyHash: 'unrelated-workspace-hash' }, + ]); + expect(await db.select().from(channels).where(eq(channels.id, channelId))).toHaveLength(0); + }); + + it('rolls back the workspace when the generated channel id collides', async () => { + attachD1Batch({}); + const { workspaceId, channelId } = useGeneratedPair(); + const existingWorkspaceId = 'existing-channel-owner'; + await db.insert(workspaces).values({ + id: existingWorkspaceId, + name: 'channel-owner', + apiKeyHash: 'channel-owner-hash', + }); + await db.insert(channels).values({ + id: channelId, + workspaceId: existingWorkspaceId, + name: 'unrelated-channel', + topic: 'Existing channel', + }); + + await expect(createWorkspace(db, 'channel-id-collision')).rejects.toMatchObject({ + code: 'workspace_id_collision', + }); + + expect(await db.select().from(workspaces).where(eq(workspaces.id, workspaceId))).toHaveLength(0); + await expect(db.select().from(channels).where(eq(channels.id, channelId))).resolves.toMatchObject([ + { workspaceId: existingWorkspaceId, name: 'unrelated-channel' }, + ]); + }); + + it('replays idempotently when D1 commits but its response is lost', async () => { + const batchCalls = attachD1Batch({ loseFirstResponse: true }); + + const created = await createWorkspace(db, 'lost-response'); + + expect(batchCalls()).toBe(2); + await expectOneCompleteWorkspace(created.workspace_id); + }); + + it('returns storage unavailable when committed-pair readback fails', async () => { + const batchCalls = attachD1Batch({ loseFirstResponse: true }); + const failingReadbackDb = new Proxy(db, { + get(target, property) { + if (property === 'select') { + return () => { + throw new Error('D1_ERROR: D1 DB is overloaded. Too many requests queued.'); + }; + } + const value = Reflect.get(target, property, target) as unknown; + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + + await expect(createWorkspace(failingReadbackDb, 'lost-response-readback-failure')).rejects.toMatchObject({ + code: 'workspace_storage_unavailable', + status: 503, + }); + + expect(batchCalls()).toBe(2); + const workspaceRows = await db.select().from(workspaces); + const channelRows = await db.select().from(channels); + expect(workspaceRows).toHaveLength(1); + expect(channelRows).toHaveLength(1); + expect(channelRows[0]?.workspaceId).toBe(workspaceRows[0]?.id); + }); + + it('recovers a committed workspace when retries after a lost response exhaust', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const batchCalls = attachD1Batch({ + loseFirstResponse: true, + failAfterLostResponse: true, + }); + + const creation = createWorkspace(db, 'lost-response-exhausted'); + const outcome = creation.then( + (value) => ({ ok: true as const, value }), + (error: unknown) => ({ ok: false as const, error }), + ); + await vi.runAllTimersAsync(); + const settled = await outcome; + if (!settled.ok) throw settled.error; + const created = settled.value; + + expect(batchCalls()).toBe(5); + expect(created.api_key).toMatch(/^rk_live_/); + await expectOneCompleteWorkspace(created.workspace_id); + }); + + it('does not retry a non-transient database error', async () => { + let calls = 0; + (db as EngineDb & Partial).batch = async () => { + calls += 1; + throw new Error('D1_TYPE_ERROR: Type mismatch'); + }; + + await expect(createWorkspace(db, 'invalid-write')).rejects.toThrow('D1_TYPE_ERROR'); + expect(calls).toBe(1); + }); +}); diff --git a/packages/engine/src/engine/workspace.ts b/packages/engine/src/engine/workspace.ts index 3fc7cd3f..935edffe 100644 --- a/packages/engine/src/engine/workspace.ts +++ b/packages/engine/src/engine/workspace.ts @@ -4,9 +4,21 @@ import { workspaces, channels } from '../db/schema.js'; import { randomHex, sha256Hex } from '../lib/crypto.js'; import { generateId } from './snowflake.js'; import { codedError } from '../lib/httpError.js'; +import { D1WriteRetryExhaustedError, retryD1Write } from '../lib/d1Retry.js'; +import { runAtomicWrites } from '../ports/database.js'; type Db = ReturnType; +type WorkspaceWriteResult = [ + Array, + Array, +]; + +type CommittedWorkspacePairRead = + | { status: 'match'; value: WorkspaceWriteResult } + | { status: 'mismatch' } + | { status: 'unavailable' }; + type CreateWorkspaceOptions = | string | { @@ -29,6 +41,94 @@ function buildWorkspaceResponse( }; } +function isExpectedWorkspacePair( + writeResult: WorkspaceWriteResult, + expected: { + workspaceId: string; + name: string; + apiKeyHash: string; + channelId: string; + }, +): boolean { + const [workspaceRows, channelRows] = writeResult; + const createdWorkspace = workspaceRows[0]; + const createdChannel = channelRows[0]; + return Boolean( + createdWorkspace && + createdWorkspace.id === expected.workspaceId && + createdWorkspace.name === expected.name && + createdWorkspace.apiKeyHash === expected.apiKeyHash && + createdChannel && + createdChannel.id === expected.channelId && + createdChannel.workspaceId === expected.workspaceId && + createdChannel.name === 'general' + ); +} + +async function readCommittedWorkspacePair( + db: Db, + expected: { + workspaceId: string; + name: string; + apiKeyHash: string; + channelId: string; + }, +): Promise { + try { + const [workspaceRows, channelRows] = await Promise.all([ + db.select().from(workspaces).where(eq(workspaces.id, expected.workspaceId)), + db.select().from(channels).where(eq(channels.id, expected.channelId)), + ]); + const result: WorkspaceWriteResult = [workspaceRows, channelRows]; + return isExpectedWorkspacePair(result, expected) + ? { status: 'match', value: result } + : { status: 'mismatch' }; + } catch { + return { status: 'unavailable' }; + } +} + +function isUniqueConstraintError(error: unknown): boolean { + let current: unknown = error; + const seen = new Set(); + + for (let depth = 0; current !== undefined && depth < 6 && !seen.has(current); depth += 1) { + seen.add(current); + if (typeof current === 'string') { + const message = current.toLowerCase(); + if ( + message.startsWith('d1_error:') && + (message.includes('unique constraint failed') || message.includes('sqlite_constraint_unique')) + ) { + return true; + } + break; + } + if (!current || typeof current !== 'object') break; + + const candidate = current as { code?: unknown; message?: unknown; cause?: unknown }; + const code = typeof candidate.code === 'string' ? candidate.code : ''; + const message = typeof candidate.message === 'string' ? candidate.message.toLowerCase() : ''; + if ( + code === 'SQLITE_CONSTRAINT_UNIQUE' || + code === 'SQLITE_CONSTRAINT_PRIMARYKEY' || + ( + code === 'SQLITE_CONSTRAINT' && + (message.includes('unique constraint failed') || message.includes('sqlite_constraint_unique')) + ) || + ( + message.startsWith('d1_error:') && + (message.includes('unique constraint failed') || message.includes('sqlite_constraint_unique')) + ) + ) { + return true; + } + current = candidate.cause; + } + + return false; +} + export async function createWorkspace( db: Db, name: string, @@ -64,23 +164,80 @@ export async function createWorkspace( const apiKey = `rk_live_${randomHex(16)}`; const apiKeyHash = await hashApiKey(apiKey); - const [createdWorkspace] = await db - .insert(workspaces) - .values({ - id: workspaceId, - name, - apiKeyHash, - }) - .returning(); - - // Auto-create #general channel const channelId = generateId(); - await db.insert(channels).values({ - id: channelId, - workspaceId, - name: 'general', - topic: 'General discussion', - }); + const expected = { workspaceId, name, apiKeyHash, channelId }; + let writeResult: WorkspaceWriteResult; + try { + writeResult = await retryD1Write(async () => { + try { + return await runAtomicWrites( + db, + (writeDb) => [ + writeDb + .insert(workspaces) + .values({ id: workspaceId, name, apiKeyHash }) + .returning(), + writeDb + .insert(channels) + .values({ + id: channelId, + workspaceId, + name: 'general', + topic: 'General discussion', + }) + .returning(), + ], + { requireAtomic: true }, + ) as WorkspaceWriteResult; + } catch (cause) { + if (!isUniqueConstraintError(cause)) throw cause; + + // A prior attempt may have committed before its response was lost. + // Accept only that exact generated pair; mismatched IDs fail closed. + const committed = await readCommittedWorkspacePair(db, expected); + if (committed.status === 'match') return committed.value; + if (committed.status === 'unavailable') { + const error = codedError( + 'Workspace storage temporarily unavailable', + 'workspace_storage_unavailable', + 503, + ); + error.diagnostics = { + operation: 'workspace.create', + storage_error: 'readback_unavailable', + }; + throw error; + } + throw codedError('Generated workspace identifier collision', 'workspace_id_collision', 500); + } + }); + } catch (cause) { + if (!(cause instanceof D1WriteRetryExhaustedError)) throw cause; + const committed = await readCommittedWorkspacePair(db, expected); + if (committed.status === 'match') { + writeResult = committed.value; + } else { + const error = codedError( + 'Workspace storage temporarily unavailable', + 'workspace_storage_unavailable', + 503, + ); + error.diagnostics = { + attempts: cause.attempts, + operation: 'workspace.create', + storage_error: cause.storageError, + }; + throw error; + } + } + + const [workspaceRows] = writeResult; + const createdWorkspace = workspaceRows[0]; + + // A conflict can only be an idempotent replay of this exact generated pair. + if (!isExpectedWorkspacePair(writeResult, expected) || !createdWorkspace) { + throw codedError('Generated workspace identifier collision', 'workspace_id_collision', 500); + } const workspace = buildWorkspaceResponse(createdWorkspace, apiKey); return { diff --git a/packages/engine/src/lib/__tests__/d1Retry.test.ts b/packages/engine/src/lib/__tests__/d1Retry.test.ts new file mode 100644 index 00000000..a89a8101 --- /dev/null +++ b/packages/engine/src/lib/__tests__/d1Retry.test.ts @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { + D1WriteRetryExhaustedError, + retryD1Write, + retryableD1ErrorCode, +} from '../d1Retry.js'; + +describe('D1 write retries', () => { + afterEach(() => vi.useRealTimers()); + + it('classifies a nested D1 cause without inspecting the outer SQL and parameters', () => { + const cause = new Error('D1_ERROR: D1 DB is overloaded. Too many requests queued.'); + const error = new Error('Failed query: insert into "workspaces" params: secret', { cause }); + + expect(retryableD1ErrorCode(error)).toBe('queue_full'); + expect( + retryableD1ErrorCode(new Error('outer query failure', { + cause: { message: 'D1_ERROR: Network connection lost.' }, + })), + ).toBe('network_connection_lost'); + expect( + retryableD1ErrorCode( + new Error('Failed query params: D1_ERROR: D1 DB is overloaded. Too many requests queued.'), + ), + ).toBeUndefined(); + }); + + it('retries a documented transient D1 failure', async () => { + vi.useFakeTimers(); + const write = vi.fn() + .mockRejectedValueOnce(new Error('D1_ERROR: Network connection lost.')) + .mockResolvedValue('created'); + + const result = retryD1Write(write); + const expectation = expect(result).resolves.toBe('created'); + await vi.runAllTimersAsync(); + + await expectation; + expect(write).toHaveBeenCalledTimes(2); + }); + + it('retries a transient D1 message carried by a plain object', async () => { + vi.useFakeTimers(); + const write = vi.fn() + .mockRejectedValueOnce({ + message: 'D1_ERROR: D1 DB is overloaded. Too many requests queued.', + }) + .mockResolvedValue('created'); + + const result = retryD1Write(write); + await vi.runAllTimersAsync(); + + await expect(result).resolves.toBe('created'); + expect(write).toHaveBeenCalledTimes(2); + }); + + it('does not retry a non-transient D1 failure', async () => { + const write = vi.fn().mockRejectedValue(new Error('D1_TYPE_ERROR: Type mismatch')); + + await expect(retryD1Write(write)).rejects.toThrow('D1_TYPE_ERROR'); + expect(write).toHaveBeenCalledTimes(1); + }); + + it('returns only a safe classification when retries are exhausted', async () => { + vi.useFakeTimers(); + const write = vi.fn().mockRejectedValue(new Error('D1_ERROR: Network connection lost.')); + + const result = retryD1Write(write, 2); + const expectation = expect(result).rejects.toMatchObject({ + name: 'D1WriteRetryExhaustedError', + attempts: 2, + storageError: 'network_connection_lost', + } satisfies Partial); + await vi.runAllTimersAsync(); + + await expectation; + }); +}); diff --git a/packages/engine/src/lib/__tests__/httpError.test.ts b/packages/engine/src/lib/__tests__/httpError.test.ts index 82b8fdbd..e8ae0bf9 100644 --- a/packages/engine/src/lib/__tests__/httpError.test.ts +++ b/packages/engine/src/lib/__tests__/httpError.test.ts @@ -1,7 +1,7 @@ import type { Context } from 'hono'; import { describe, expect, it, vi } from 'vitest'; -import { codedError, errorResponse } from '../httpError.js'; +import { codedError, errorResponse, safeErrorDiagnostics } from '../httpError.js'; function testContext(): Context { return { @@ -45,7 +45,7 @@ describe('errorResponse', () => { const error = codedError('Directory write failed', 'internal_error', 500); error.cause = new Error('SQLITE_CONSTRAINT'); - const response = errorResponse(testContext(), error, { includeCause: true }); + const response = errorResponse(testContext(), error); expect(response.status).toBe(500); await expect(response.json()).resolves.toEqual({ @@ -56,4 +56,74 @@ describe('errorResponse', () => { }, }); }); + + it('does not expose SQL or bound parameters from uncoded server errors', async () => { + const response = errorResponse( + testContext(), + new Error('Failed query: insert into "workspaces" params: rk_live_secret_hash'), + ); + + expect(response.status).toBe(500); + await expect(response.json()).resolves.toEqual({ + ok: false, + error: { + code: 'internal_error', + message: 'Internal server error', + }, + }); + }); + + it('treats status 0 as 500 before redacting an uncoded error message', async () => { + const error = Object.assign( + new Error('Failed query: insert into "workspaces" params: rk_live_secret_hash'), + { status: 0 }, + ); + + const response = errorResponse(testContext(), error); + + expect(response.status).toBe(500); + await expect(response.json()).resolves.toEqual({ + ok: false, + error: { + code: 'internal_error', + message: 'Internal server error', + }, + }); + }); + + it('preserves legacy coded 5xx messages built without codedError', async () => { + const error = Object.assign(new Error('Service dependency unavailable'), { + code: 'service_unavailable', + status: 503, + }); + + const response = errorResponse(testContext(), error); + + expect(response.status).toBe(503); + await expect(response.json()).resolves.toEqual({ + ok: false, + error: { + code: 'service_unavailable', + message: 'Service dependency unavailable', + }, + }); + }); + + it('keeps only allowlisted primitive diagnostic fields', () => { + const error = codedError('Storage unavailable', 'storage_unavailable', 503); + error.diagnostics = { + attempts: 4, + operation: 'workspace.create', + storage_error: 'queue_full', + api_key_hash: 'must-not-leak', + query: 'insert into workspaces', + nested: { params: 'must-not-leak' }, + }; + + expect(safeErrorDiagnostics(error)).toEqual({ + attempts: 4, + operation: 'workspace.create', + storage_error: 'queue_full', + }); + }); }); diff --git a/packages/engine/src/lib/d1Retry.ts b/packages/engine/src/lib/d1Retry.ts new file mode 100644 index 00000000..5199f733 --- /dev/null +++ b/packages/engine/src/lib/d1Retry.ts @@ -0,0 +1,85 @@ +const RETRYABLE_D1_ERRORS = [ + ['network_connection_lost', 'Network connection lost'], + ['code_update_reset', 'D1 DB reset because its code was updated'], + ['startup_storage_reset', 'Internal error while starting up D1 DB storage caused object to be reset'], + ['storage_reset', 'Internal error in D1 DB storage caused object to be reset'], + ['remote_node_transient', 'Cannot resolve D1 DB due to transient issue on remote node'], + ['client_disconnected', "Can't read from request stream because client disconnected"], + ['queue_timeout', 'D1 DB is overloaded. Requests queued for too long'], + ['queue_full', 'D1 DB is overloaded. Too many requests queued'], +] as const; + +export type RetryableD1ErrorCode = (typeof RETRYABLE_D1_ERRORS)[number][0]; + +export class D1WriteRetryExhaustedError extends Error { + constructor( + readonly attempts: number, + readonly storageError: RetryableD1ErrorCode, + ) { + super('Transient D1 write failed after retry attempts'); + this.name = 'D1WriteRetryExhaustedError'; + } +} + +function errorCause(error: unknown): unknown { + if (!error || typeof error !== 'object' || !('cause' in error)) return undefined; + return (error as { cause?: unknown }).cause; +} + +function errorMessage(error: unknown): string | undefined { + if (typeof error === 'string') return error; + if (!error || typeof error !== 'object' || !('message' in error)) return undefined; + const message = (error as { message?: unknown }).message; + return typeof message === 'string' ? message : undefined; +} + +/** + * Classify only D1's own error message (including a nested Drizzle cause). + * The outer Drizzle message contains SQL and bound parameters, so matching it + * could mistake a user-controlled value for a retryable storage failure. + */ +export function retryableD1ErrorCode(error: unknown): RetryableD1ErrorCode | undefined { + let current: unknown = error; + const seen = new Set(); + + for (let depth = 0; current !== undefined && depth < 6 && !seen.has(current); depth += 1) { + seen.add(current); + const message = errorMessage(current); + + if (message?.startsWith('D1_ERROR:')) { + const match = RETRYABLE_D1_ERRORS.find(([, fragment]) => message.includes(fragment)); + return match?.[0]; + } + current = errorCause(current); + } + + return undefined; +} + +function retryDelayMs(completedAttempts: number): number { + const exponential = 50 * (2 ** (completedAttempts - 1)); + return exponential + Math.floor(Math.random() * exponential); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** Retry a D1 write only for Cloudflare's documented transient error classes. */ +export async function retryD1Write( + write: () => Promise, + maxAttempts = 5, +): Promise { + for (let attempt = 1; ; attempt += 1) { + try { + return await write(); + } catch (error) { + const code = retryableD1ErrorCode(error); + if (!code || attempt >= maxAttempts) { + if (code) throw new D1WriteRetryExhaustedError(attempt, code); + throw error; + } + await sleep(retryDelayMs(attempt)); + } + } +} diff --git a/packages/engine/src/lib/httpError.ts b/packages/engine/src/lib/httpError.ts index 6cbbba4c..90a5f771 100644 --- a/packages/engine/src/lib/httpError.ts +++ b/packages/engine/src/lib/httpError.ts @@ -11,6 +11,8 @@ import { jsonError, jsonMalformedBody } from './httpResponse.js'; export interface CodedError extends Error { code?: string; status?: number; + diagnostics?: Record; + clientSafe?: true; } /** @@ -32,15 +34,40 @@ export function asCodedError(err: unknown): CodedError { /** Build an `Error` carrying a stable `code` and HTTP `status` (no cast at the call site). */ export function codedError(message: string, code: string, status: number): CodedError { - return Object.assign(new Error(message), { code, status }); + return Object.assign(new Error(message), { code, status, clientSafe: true as const }); } -interface ErrorResponseOptions { - includeCause?: boolean; +const SAFE_DIAGNOSTIC_FIELDS = new Set([ + 'attempts', + 'operation', + 'storage_error', +]); + +/** Select primitive, explicitly allowlisted fields for logs and telemetry. */ +export function safeErrorDiagnostics(error: CodedError): Record { + const safe: Record = {}; + for (const [key, value] of Object.entries(error.diagnostics ?? {})) { + if ( + SAFE_DIAGNOSTIC_FIELDS.has(key) && + (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') + ) { + safe[key] = value; + } + } + return safe; +} + +function effectiveHttpStatus(error: CodedError): number { + return error.status || 500; } -function messageWithCause(error: CodedError) { - return error.message; +export function safeClientErrorMessage( + error: CodedError, + status = effectiveHttpStatus(error), +): string { + return status >= 500 && error.clientSafe !== true && !error.code + ? 'Internal server error' + : error.message; } /** @@ -50,15 +77,16 @@ function messageWithCause(error: CodedError) { * real `instanceof` narrowing instead of the unchecked `err as Error & {...}` * assertion that was duplicated across every route handler. */ -export function errorResponse(c: Context, err: unknown, options: ErrorResponseOptions = {}) { +export function errorResponse(c: Context, err: unknown) { const error = asCodedError(err); if (err instanceof SyntaxError) { return jsonMalformedBody(c); } + const status = effectiveHttpStatus(error); return jsonError( c, error.code || 'internal_error', - options.includeCause ? messageWithCause(error) : error.message, - (error.status || 500) as ContentfulStatusCode, + safeClientErrorMessage(error, status), + status as ContentfulStatusCode, ); } diff --git a/packages/engine/src/ports/database.ts b/packages/engine/src/ports/database.ts index d4c4fc96..b6405b06 100644 --- a/packages/engine/src/ports/database.ts +++ b/packages/engine/src/ports/database.ts @@ -81,6 +81,11 @@ export type AtomicWrite = BatchItem<'sqlite'> & PromiseLike; export type AtomicWriteBuilder = (db: EngineDb) => readonly AtomicWrite[]; export type AtomicWriteInput = readonly AtomicWrite[] | AtomicWriteBuilder; +export interface RunAtomicWritesOptions { + /** Reject before building statements when the handle cannot guarantee rollback. */ + requireAtomic?: boolean; +} + /** * D1-style atomic batch: every statement applies, or none does. * @@ -126,7 +131,7 @@ export function runAtomic(db: EngineDb, fn: (tx: EngineDb) => Promise): Pr * statements are built from the original handle and run as one atomic * batch. * 3. Neither — statements run sequentially with no rollback, the engine's - * historical behavior for bare handles. + * historical behavior for bare handles, unless `requireAtomic` is set. * * Returns the per-statement results in order, so callers can recover * `.returning()` rows by index. Statements must not depend on each other's @@ -136,8 +141,19 @@ export function runAtomic(db: EngineDb, fn: (tx: EngineDb) => Promise): Pr export async function runAtomicWrites( db: EngineDb, input: AtomicWriteInput, + options: RunAtomicWritesOptions = {}, ): Promise { const handle = db as EngineDb & Partial & Partial; + if ( + options.requireAtomic && + typeof handle.withTransaction !== 'function' && + typeof handle.batch !== 'function' + ) { + throw new Error( + 'Atomic write capability required: database handle exposes neither withTransaction nor batch', + ); + } + if (handle.withTransaction) { return handle.withTransaction(async (tx) => { const statements = buildStatements(input, tx); diff --git a/packages/engine/src/routes/workspace.ts b/packages/engine/src/routes/workspace.ts index 46a3dfed..56edc81c 100644 --- a/packages/engine/src/routes/workspace.ts +++ b/packages/engine/src/routes/workspace.ts @@ -26,7 +26,8 @@ import { import { channels } from '../db/schema.js'; import { and, eq, inArray } from 'drizzle-orm'; import { emitServerEvent } from '../lib/serverTelemetry.js'; -import { errorResponse } from '../lib/httpError.js'; +import { asCodedError, errorResponse, safeErrorDiagnostics } from '../lib/httpError.js'; +import { getRequestLogger } from '../lib/logger.js'; import { jsonCreated, jsonError, @@ -171,6 +172,26 @@ workspaceRoutes.post('/workspaces', async (c) => { } return result.created ? jsonCreated(c, result.workspace) : jsonOk(c, result.workspace); } catch (err: unknown) { + const error = asCodedError(err); + if ((error.status ?? 500) >= 500) { + const diagnostics = safeErrorDiagnostics(error); + getRequestLogger(c, 'workspace.create').error('Workspace creation failed', { + error_code: error.code ?? 'internal_error', + error_status: error.status ?? 500, + ...diagnostics, + }); + c.get('engine').telemetry.captureException( + new Error(error.code ?? 'internal_error'), + { + path: c.req.path, + method: c.req.method, + status_code: error.status ?? 500, + error_code: error.code ?? 'internal_error', + request_id: c.get('requestId'), + ...diagnostics, + }, + ); + } return errorResponse(c, err); } });