SPL-121: guided Experiment creation into Run 1 (+ SPL-251 Run-start Idempotency-Key) - #248
Conversation
…SPL-251) Experiment creation is a guided draft flow: step 1 writes a real `draft` Experiment so the wizard resumes from the Control Plane rather than from browser state, and the last step opens Run 1 through the SAME draft -> Start machinery every later Run uses. `useExperimentRunStart` is the Panel's only caller of `stageAndStartControlPanelExperimentRun`, and SPL-120's next-Run dialog now consumes the same shared draft fields, Start confirmation, and hook. SPL-251 / SPL-242: Run-start was refused with "Idempotency-Key header is required for this route" because the Control Plane SDK put the key in the body only. The fix is in the shared client, not in the Panel: `withIdempotencyHeader` mirrors `idempotency_key` onto the header for experiment create, update, and start, and a test asserts the header rather than only the happy path. The Panel mints the key once per mounted form, so a retried Start replays instead of opening a second Run. Fail loud (ADR-0036): - a Start whose Approval Request did not apply reports that no Run exists and does not offer a retry that would open a second request - an unhonourable `horizon` is refused instead of coerced to sequential, and a proposal with no horizon is refused instead of inferred - `StartRunInput` requires `horizon` so the column default can never stand in - the locked decision spec is attributed to the live Run by id, and an absent live Run throws instead of rendering an editable form the Worker will refuse Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…identity `horizon` and `sampleSizeLocked` were written into the Approval Request's `proposed` record but were absent from `proposalInput`, so `proposalRequestHash` did not cover them. A Start retried under the same Idempotency-Key with a DIFFERENT stopping rule matched the stale hash, replayed the earlier proposal and answered 200 while the Run froze the original intent. Both the hash input and the `proposed` record are now derived from one `startProposalFields` call, so they cannot drift apart again. Also defaults a proposal with no recorded `horizon` to `sequential` rather than refusing it: refusing bricked every already-pending `experiment_start` Approval Request with a remedy no operator can perform (ADR-0036). Deletes the tautological `runStartLanded` arm and its factually wrong copy, and adds per-call-site Idempotency-Key header tests for create and update. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review fixes — head
|
| Mutation | Result |
|---|---|
| Both server fixes reverted together | 2 failed — expected 200 to be 409 (the exact repro) and expected 409 to be 200 |
Horizon default only (proposed.horizon ?? "sequential" → proposed.horizon) |
1 failed — expected 409 to be 200 |
Drift only (destructure horizon/sampleSizeLocked out of the hash input, keep them in proposed) |
1 failed — expected 200 to be 409 |
Strip withIdempotencyHeader from the create and update legs |
2 failed — expected null to be 'create-exp-1', expected null to be 'update-exp-1' |
Remove { ignoreMismatch: true } from replayExperimentStart |
5 passed — not load-bearing, so it was kept for consistency with the other four handlers |
verify:ci (one run, from the worktree)
Tasks: 85 successful, 85 total
Cached: 56 cached, 85 total
@splitch/control-plane-api:test executed live: 90 test files, 480 tests passed. The lint/typecheck cache hits on the touched packages are legitimate — the pre-commit verify:commit had just executed them at these exact contents.
#248 (SPL-121/SPL-251) landed a second, private `withIdempotencyHeader` inside `experiments-client.ts` for the same defect. Two mechanisms for one job is the footgun; the private one is deleted and all three Experiment legs now route through the shared `idempotency-header.ts` helper. The shared helper takes the operationId and consults the route registry, which changes two of #248's legs: - `experiments_create` declares `optional` and its request schema carries `idempotency_key`, so it keeps mirroring the key onto the header. - `experiments_update` declares `idempotency: "none"` and `PatchExperimentRequestSchema` is `.strict()` with no `idempotency_key` field. The private helper's mirror there could never fire for a legal caller and the runtime guard returns before reading the header either way, so the call site goes back to plain `hcRequestOptions` and #248's update-header test is deleted: it only went green because it cast an illegal field through `as never`. #248's Start and create header tests are kept. They drive the PANEL client, whose mutations delegate to `createExperimentsClient` — a path the contract test does not walk — and `experiments_create` is `optional`, so the contract test's required-route sweep never reaches it. #248 changed no route contracts, so the contract test's coverage assertion has nothing new to report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Closes SPL-121. Fixes SPL-251 (trunk-red: Panel could not Start a Run) and the SPL-242 class of gap in the Control Plane SDK.
What this ships
Guided Experiment creation (SPL-121). Step 1 writes a real
draftExperiment, so the flow resumes from the Control Plane rather than from browser state. The last step opens Run 1 through the same draft → Start machinery every later Run uses:useExperimentRunStartis the Panel's only caller ofstageAndStartControlPanelExperimentRun, and SPL-120's next-Run dialog now consumes the same extractedExperimentRunDraftFields,useRunDraftState, andExperimentRunStartConfirmation. There is no creation-only Start path.SPL-251 / SPL-242 — the defect is in the shared client, not the Panel. Run-start was refused with
Idempotency-Key header is required for this routebecause@splitch/control-plane-sdkputidempotency_keyin the body only.withIdempotencyHeaderinexperiments-client.tsnow mirrors it onto the header for experimentcreate,update, andstart, andpanel-experiments.test.tsasserts the header itself rather than only that the happy path passes. The Panel mints the key once per mounted form, so a retried Start replays instead of opening a second Run.Fail loud (ADR-0036)
runStartLandedis the single rule:null(allow) orappliedmeans a Run exists;pending/declined/staledo not.horizonis refused instead of coerced tosequential; a stored proposal with no horizon is refused instead of inferred.StartRunInputrequireshorizon, so the column default can never silently stand in for a stopping rule nobody chose.runs[0], and a live Run id absent from the Run history throws rather than rendering an editable form the Worker will refuse.Approval gate seam
The Panel sends
review: { action: "approve_and_apply" }inline, so the Approval Request is created and applied in one call and there is no pending request to gate. No confirm gate is authored here — SPL-118'scomponents/approval/**remains the owner if a deferred-approval path is added later.Verification
pnpm verify:ci: 84/85 tasks successful, 20/85 cached. The one failure isapps/control-plane-api test/live-update-do.test.ts > "closes a silent expired panel session after hibernation"— a WebSocket upgrade returningnullunder load; it passes in isolation and touches nothing in this diff. Same class as the known SPL-231 / SPL-238 load-sensitive failures.--workers=1, hard timeout): 3 passed, 1 failed.experiments-setup.spec.ts:13— SPL-120's spec and the SPL-251 target — passes unmodified. The gatedconfirm-Environment creation path passes. The remaining failure is a local Miniflare D1internal errorthrown fromD1DatabaseObject.queryExecute, hitting a different query on each run; not reproducible as an application defect.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.