Skip to content

fix(server): declare 400/404 on the session fork route - #1101

Merged
Astro-Han merged 1 commit into
devfrom
claude/i936-session-fork-errors
Jun 3, 2026
Merged

fix(server): declare 400/404 on the session fork route#1101
Astro-Han merged 1 commit into
devfrom
claude/i936-session-fork-errors

Conversation

@Astro-Han

Copy link
Copy Markdown
Owner

What

POST /session/:sessionID/fork forks an existing session and calls Session.get(sessionID) first, which raises NotFoundError (mapped to 404 by ErrorMiddleware) when the parent session does not exist. But the route's OpenAPI contract only declared a 200 response, so the 404 (and the validator's 400) were undocumented: an SDK consumer reading the contract had no way to know session.fork could fail with not-found.

Change

Add errors(400, 404) to the fork route so the published contract matches what the running server already serves.

Runtime behavior is unchanged — this is a purely additive contract declaration. The 404 already happened; now it is declared.

On the SDK snapshot

Per this line's strategy, the hand-maintained packages/sdk/openapi.json snapshot and the generated SDK are not regenerated in this PR. That checked-in snapshot is already ~24 operations behind the live code (it has no committed generator and is batch-resynced occasionally), so regenerating it here would bury this one-line change under a 150KB+ accumulated-drift diff. This PR changes only the live served OpenAPI source (Server.openapi()); the snapshot/SDK can be resynced in a separate mechanical pass.

Verification

  • bun test test/server/session-core-routes.test.ts — new cases assert Server.openapi() declares 400 and 404 for /session/{sessionID}/fork, and that forking a missing session returns 404 with NotFoundError in the body; 3/3 pass.
  • tsgo --noEmit clean.
  • route-inventory harness green (no route added/removed).

Context

#936 Effect error-contract line, contract-completion phase (slice A). Upstream anomalyco/opencode declares the equivalent fork not-found failure. Code-only slice, no SDK regen.

POST /session/:sessionID/fork forks an existing session, calling
Session.get(sessionID) first - which raises NotFoundError (mapped to 404
by ErrorMiddleware) when the parent session does not exist. The route's
OpenAPI contract only declared a 200 response, so this 404 (and the
validator's 400) were undocumented: SDK consumers reading the contract
had no way to know fork could fail with not-found.

Add errors(400, 404) to the fork route so the contract matches what the
running server already serves. Runtime behavior is unchanged - this is a
purely additive contract declaration.

Per the line's strategy, the stale hand-maintained packages/sdk/openapi.json
snapshot and generated SDK are NOT regenerated here (they are ~24
operations behind the live code and are batch-resynced separately); this
PR changes only the live served OpenAPI source and adds coverage.

Verify: bun test test/server/session-core-routes.test.ts - new cases
assert Server.openapi() declares 400 and 404 for /session/{sessionID}/fork
and that forking a missing session returns 404 with NotFoundError in the
body; tsgo --noEmit clean; route-inventory harness green.
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Astro-Han, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 51 minutes and 46 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f92dca62-42f1-4d7b-b6e4-74b1e7ed5a4d

📥 Commits

Reviewing files that changed from the base of the PR and between 46d11c9 and 291d579.

📒 Files selected for processing (2)
  • packages/opencode/src/server/instance/session.ts
  • packages/opencode/test/server/session-core-routes.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/i936-session-fork-errors

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Astro-Han Astro-Han added bug Something isn't working harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority upstream Tracked upstream or vendor behavior labels Jun 3, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han
Astro-Han merged commit 53195a3 into dev Jun 3, 2026
34 of 36 checks passed
@Astro-Han
Astro-Han deleted the claude/i936-session-fork-errors branch June 3, 2026 03:14
Astro-Han added a commit that referenced this pull request Jun 3, 2026
…1112)

* chore(sdk): regenerate v2 SDK types for the migrated error contracts

Regenerate packages/sdk/js/src/v2/gen from the current server contract
(`cd packages/sdk/js && bun run build`). This closes the SDK-lag P2 that the
#936 contract slices kept surfacing: the generated v2 SDK error types now match
the live server's declared 4xx responses.

Picked up:
- SessionForkErrors (400, 404) and the fork method's error generic — #1101.
- SessionToolRespondErrors now declares the inline 404/409/422 bodies
  ({ error, details? }) instead of only 400/404 — #1104.
- 409 Conflict (UnknownError) on the nine busy-guarded session routes
  (turnChangeUndo/Redo, turnChangesAggregateUndo/Redo, summarize, deleteMessage,
  shell, revert, unrevert) — #1106.

The regen also absorbs minor pre-existing ordering/description drift from earlier
merges (an EventSessionCompacted union reordering, a VCS diffRaw doc string),
which is just the canonical generator output catching up.

Scope note: this regenerates only the consumed SDK artifact (src/v2/gen). The
checked-in packages/sdk/openapi.json reporting snapshot is intentionally left to
a separate change — it is read only by the route-inventory harness, whose tests
are built to detect (and tolerate) that snapshot's lag, so resyncing it there
would fight that harness by design.

All changes are additive (new error types on existing methods), so SDK consumers
are unaffected. `bun tsc` (run by the SDK build) and the route-inventory harness
tests pass.

Refs #936

* chore(ci): route packages/sdk changes to the harness label

This SDK regen is the first packages/sdk-only PR, which surfaced that the
labeler has no routing glob for packages/sdk. With sync-labels enabled, a
SDK-only PR therefore gets no primary routing label (and any manually added one
is stripped), so the pr-triage label policy ("at least one routing label")
cannot be satisfied.

Route packages/sdk/** to the harness area: the SDK is the opencode server's
generated client, part of the same contract surface as packages/opencode. Lock
the routing in the pr-triage workflow test.

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

Labels

bug Something isn't working harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority upstream Tracked upstream or vendor behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant