fix: correct bridge mode cross-directory dispatch and race conditions#41
Merged
fix: correct bridge mode cross-directory dispatch and race conditions#41
Conversation
- Add x-opencode-directory header to bridge task dispatch with CRLF sanitization - Wrap fire-and-forget SessionPrompt.prompt() in Instance.provide() to preserve ALS context - Use timingSafeEqual for x-bridge-id comparison to prevent timing attacks - Extract shared authHeaders() utility and use it consistently across bridge UI - Add suppressUntil + generation counter to prevent poll race conditions in bridge.tsx - Add activeTaskIDs idempotency set to prevent duplicate task dispatch - Fix process.cwd() vs session.directory for bridge registration in app.tsx - Move bridgeInited = true to success branch to allow retry on failure - Add double-submit guard and fix dialog.close() ordering in dialog-become-friend.tsx
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes bridge-mode correctness issues across the CLI and web app, focusing on cross-directory dispatch, auth header consistency, and race-condition prevention.
Changes:
- CLI: include
x-opencode-directoryon cross-node task dispatch; fix bridge registration directory and init retry behavior. - Server: harden
/bridge/dispatch-taskauth comparison and make friend task dispatch more robust (fire-and-forget + logging + idempotency guard). - Web app: centralize
Authorizationheader creation and prevent bridge poll races from overwriting optimistic state.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/opencode/src/tool/task.ts | Adds directory header (with CRLF sanitization) and improved HTTP error reporting for bridge dispatch. |
| packages/opencode/src/server/routes/bridge.ts | Uses constant-time bridge ID comparison; adds task idempotency set; wraps prompt execution in Instance.provide() and logs failures. |
| packages/opencode/src/cli/cmd/tui/app.tsx | Registers bridge directory from process.cwd() and only marks bridge initialized after successful POST. |
| packages/app/src/utils/auth.ts | Introduces shared authHeaders() helper for Basic Auth. |
| packages/app/src/pages/session/session-info-panel.tsx | Switches to shared authHeaders() for bridge actions (master/leave). |
| packages/app/src/pages/session/dialog-become-friend.tsx | Adds auth headers to join request; adds double-submit guard; closes dialog before invoking success callback. |
| packages/app/src/context/bridge.tsx | Uses shared authHeaders() and adds suppression/generation logic to prevent stale polls overwriting optimistic bridge state. |
Comments suppressed due to low confidence (1)
packages/opencode/src/server/routes/bridge.ts:395
activeTaskIDsis added before several awaited operations that can throw (e.g.,Session.create). If an exception occurs before the fire-and-forgetInstance.provide()branch runs, the handler will error and thetaskIDwill remain inactiveTaskIDs, causing a memory leak and potentially making retries of the sametaskIDreturn the "dup" response without actually processing. Ensure thetaskIDis removed on all failure paths (e.g., try/finally around the handler body, or add to the set only after all awaited setup steps succeed).
activeTaskIDs.add(taskID)
const dir = Instance.directory
const session = await Session.create({
title: description,
permission: [
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+382
to
386
| const incoming = c.req.header("x-bridge-id") ?? "" | ||
| const safe = (a: string, b: string) => | ||
| a.length === b.length && timingSafeEqual(Buffer.from(a, "utf8"), Buffer.from(b, "utf8")) | ||
| if (!Bridge.isFriend() || !bid || !safe(incoming, bid)) return c.json({ error: "Unauthorized" }, 401) | ||
| const { taskID, prompt, description } = c.req.valid("json") |
packages/app/src/context/bridge.tsx
Outdated
Comment on lines
+102
to
+106
| function set(...args: any[]) { | ||
| suppressUntil = Date.now() + SUPPRESS_MS | ||
| // @ts-expect-error — forward all overloads | ||
| setState(...args) | ||
| } |
- Fix describeRoute responses for /dispatch-task: document 401 (not 403) to match runtime behavior - Fix activeTaskIDs leak: wrap Session.create and setup in try/catch to delete taskID on synchronous failure - Fix set() wrapper in bridge.tsx: type as typeof setState instead of any[] for proper type-safety
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix multiple bugs in bridge mode that prevented correct cross-directory task dispatch and introduced race conditions in both the CLI and web app.
CLI fixes
x-opencode-directoryheader (task.ts): The master's fetch to/bridge/dispatch-taskhad no directory header, causing the friend to fall back toprocess.cwd()instead of its registered directory. Added header with CRLF sanitization.bridge.ts):SessionPrompt.prompt()was called fire-and-forget after the HTTP handler returned, tearing down theInstance.provide()AsyncLocalStoragecontext. Wrapped the call inInstance.provide()withasync/await.session.directoryin bridge registration (app.tsx): Both master and friend registered their bridge directory usingsession.directory, frozen at session creation time. Changed toprocess.cwd()captured once atApp()init.bridgeInitedset before POST resolves (app.tsx): Failures were permanent with no retry. MovedbridgeInited = trueinto the.then()success branch only.Web app fixes
Authorizationheader (dialog-become-friend.tsx):/bridge/set-friendfetch silently 401'd on password-protected servers. Added sharedauthHeaders()utility and spread into all bridge fetch calls.bridge.tsx): AfterbecomeMaster()optimistically set bridge state, the background poll could race in with stale server response and overwrite back tonull. AddedsuppressUntiltimestamp and a monotonic generation counter to prevent stale poll results from overwriting optimistic state.Security fixes
task.ts):node.directorywas placed directly into an HTTP header without sanitization.bridge.ts):!==string comparison replaced withtimingSafeEqualfrom Node'scryptomodule.authHeaders()logic: Extracted topackages/app/src/utils/auth.tsand used consistently inbridge.tsx,session-info-panel.tsx, anddialog-become-friend.tsx.Other fixes
activeTaskIDsset inbridge.tsto prevent duplicate task dispatch on network retry.dialog-become-friend.tsxif a submit is already in flight.dialog.close()ordering: Moved beforeprops.onSuccess()so dialog always closes even if the callback throws..catch()now logs viaLog.Default.error.Testing
packages/opencodepackages/app