feat(remote-bridge): standalone Go chat companion bridge (1/2)#1275
Merged
Conversation
Astro-Han
added
enhancement
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces the complete ChangesRemote-bridge Go service
Sequence Diagram(s)sequenceDiagram
participant CLI as pawwork-remote-bridge
participant Gateway as gateway.App.Run
participant EventStream as pawwork.Client.StreamEvents
participant Engine as bridge.Engine
participant Sidecar as PawWork Sidecar
participant Platform as registered platforms
CLI->>Gateway: Run(ctx, config)
Gateway->>EventStream: goroutine StreamEvents(ctx, replayHandler)
EventStream-->>Sidecar: GET /global/event with Last-Event-ID
EventStream-->>Gateway: handleStreamReady() → unblock hydrate
Gateway->>Engine: hydrate sessions (ListSessions)
Gateway->>Engine: hydrate permissions (ListPermissions)
Gateway->>Engine: hydrate questions (ListQuestions)
par Start all registered platforms
Gateway->>Platform: Start(ctx, messageHandler)
end
loop until ctx cancel or fatal error
Sidecar-->>EventStream: SSE events (text, permissions, questions, sessions)
EventStream->>Engine: DispatchEvent / HandleAssistantText / HandlePermission / HandleQuestion / HandleSession
Platform-->>Engine: inbound message → HandleMessage
Engine-->>Platform: Reply/Send (permission/question prompt, answer, assistant text)
Engine-->>Sidecar: SendPrompt / ReplyPermission / SubmitQuestion
end
Gateway->>Platform: stopPlatforms()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
github-actions
Bot
added
ui
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/app.tsx, packages/app/src/desktop-api-contract.ts, packages/app/src/desktop-api.ts, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/app/src/pages/settings/remote-source.test.ts, packages/app/src/pages/settings/remote.tsx, packages/app/src/pages/settings/settings-shell.tsx, packages/desktop-electron/src/main/index.ts, packages/desktop-electron/src/main/ipc.ts, packages/desktop-electron/src/main/remote-access-ipc-source.test.ts, packages/desktop-electron/src/main/remote-bridge.test.ts, packages/desktop-electron/src/main/remote-bridge.ts, packages/desktop-electron/src/preload/index.ts, packages/desktop-electron/src/preload/types.ts)).
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.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a remote access bridge feature to connect chat platforms (such as Slack, Feishu, and Discord) to desktop PawWork sessions. It introduces a Go-based remote bridge binary, integrates a controller and IPC handlers in the Electron main process, and adds a dedicated 'Remote access' settings page in the frontend. The code review feedback highlights several key improvement opportunities: in the Go bridge, only known commands starting with / should be intercepted to prevent blocking normal prompts like file paths; in the frontend, SolidJS state management can be simplified by utilizing the built-in mutate function from createResource; and in the Go client, error handling during permission and question listing should be made more resilient by logging warnings and continuing rather than aborting on a single directory failure.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/desktop-electron/src/main/index.ts (1)
698-703:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAwait remote bridge shutdown on quit/relaunch paths.
remoteAccess.stop()is async, butkillSidecar()drops the promise. All of the quit/install/relaunch/signal handlers call this helper and then continue exiting immediately, so the Electron process can terminate before the bridge handles SIGTERM. That can leave remote access running after the desktop app closes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/desktop-electron/src/main/index.ts` around lines 698 - 703, killSidecar currently calls remoteAccess.stop() without awaiting it, allowing the process to exit before the bridge shuts down; make killSidecar async, await remoteAccess.stop(), and still stop the server (await server.stop(true) if it returns a promise) and null it; update all callers (quit/install/relaunch/signal handlers) to await killSidecar() so the Electron process waits for shutdown, and wrap awaits in a try/catch (or timeout) to avoid hanging if shutdown fails.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
281-284: ⚡ Quick winRun the Go race suite here too.
Line 283 only runs
go test ./..., so CI still misses the race coverage for the new remote-bridge concurrency paths. Add a separatego test -race ./...step here, or make this step include it, so the workflow matches the PR’s stated Go unit/race verification scope.🧪 Suggested CI update
- name: Test remote bridge if: ${{ inputs.phase != 'finalize' }} run: go test ./... working-directory: packages/remote-bridge + + - name: Race test remote bridge + if: ${{ inputs.phase != 'finalize' }} + run: go test -race ./... + working-directory: packages/remote-bridge🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 281 - 284, The CI step named "Test remote bridge" currently runs "go test ./..." in the packages/remote-bridge working-directory but omits the race detector; update this step to also execute the race-enabled tests (either replace the run command with a sequence that runs both "go test ./..." and "go test -race ./..." or add a new step that runs "go test -race ./..." with the same name/working-directory) so the remote-bridge concurrency paths are covered by the Go race suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/src/pages/settings/remote.tsx`:
- Around line 107-114: The start() handler persists the config with enabled:
true but doesn't update the local enabled() signal, so the UI can still show
disabled; after a successful remoteAccessStart (where next is returned) or
whenever start forces enabled: true, set the local enabled() signal to true
(e.g. call enabled(true)) and ensure the form state/mutate(next) reflects that
change before calling refetch() so the checkbox stays in sync with the running
bridge.
In `@packages/app/src/pages/settings/settings-shell.tsx`:
- Around line 19-21: The remote-access tab is rendered even when the Electron
preload bridge is absent; update TAB_VALUES (and any tab-rendering logic in this
file, e.g., the tab list at lines ~27-35 and the RemotePage rendering at
~201-203) to only include "remoteAccess" when the preload bridge exists (check
window.api?.remoteAccess or a central isDesktop/isBridgeAvailable flag).
Specifically, change the static TAB_VALUES array to be built conditionally
(e.g., baseTabs plus remoteAccess if window.api?.remoteAccess) and guard
rendering of RemotePage so that if the bridge is missing you either hide the tab
or render an explicit unsupported-state panel instead of relying on
optional-chained no-ops; reference SettingsTab, TAB_VALUES, and RemotePage when
applying the changes.
In `@packages/desktop-electron/src/main/index.ts`:
- Around line 206-214: serverReady.promise is being resolved before the
sidecar's health.wait completes, allowing autoStartRemoteAccess (and IPC callers
that rely on serverReady.promise) to call remoteAccess.start against a
still-booting sidecar; update the logic so the "safe to talk to PawWork" gate
waits for health.wait to succeed. Concretely: either move the resolution of
serverReady.promise inside initialize() to occur only after health.wait()
completes, or change autoStartRemoteAccess and the IPC callers to await
health.wait() (or a new serverHealthy promise) before calling
remoteAccess.start/IPC methods; reference the functions serverReady.promise,
initialize(), health.wait(), autoStartRemoteAccess(), and remoteAccess.start()
when making the change. Ensure errors from health.wait propagate to the existing
catch so startup failures are logged consistently.
In `@packages/desktop-electron/src/main/remote-bridge.ts`:
- Around line 221-226: In readUserConfig, don't blanket-catch all errors; only
return defaultConfig when the readFile/JSON.parse failure is due to a missing
file (ENOENT) and rethrow or propagate other errors (malformed JSON,
permission/I/O). Update the catch to inspect the thrown error (from
readFile/JSON.parse), if err.code === 'ENOENT' return defaultConfig otherwise
throw the error so callers can surface the failure; keep usage of
normalizeConfig and defaultConfig intact and ensure TypeScript error typing/null
checks as needed.
In `@packages/remote-bridge/internal/bridge/engine.go`:
- Around line 563-567: setCurrent(...) updates the pointer store but doesn't
populate the in-memory delivery pointer e.active, causing
HandleAssistantText/replyToActive to fall back or drop messages; after
successfully calling e.setCurrent(sessionID, ...) in both the `/new` and
`/sessions N` branches, call e.setActive(sessionID, platform, msg.ReplyCtx) so
the in-memory delivery is seeded immediately; update the success paths around
setCurrent in engine.go (the blocks that currently return "Started a new PawWork
session." and the similar block at the other location) to invoke
e.setActive(...) right after the successful setCurrent call.
In `@packages/remote-bridge/internal/bridge/session_pointers.go`:
- Around line 47-56: RemoteKeyForSession currently scans p.sessions and returns
the first remoteKey whose stored session shares the same root, which is
nondeterministic and can restore replies to the wrong chat; fix by maintaining
an explicit, locked inverse map (e.g., rootToRemoteKey) inside
MemorySessionPointers that is updated whenever session mappings change (the same
places that call Set/Unset/Bind), then change RemoteKeyForSession to look up the
remote key directly from rootToRemoteKey (use p.rootLocked as needed) and, when
detecting conflicting bindings for the same root, either reject the second
binding at bind time or surface/log an error so engine.restoreDelivery cannot
pick an arbitrary map iteration result.
In `@packages/remote-bridge/internal/gateway/gateway.go`:
- Around line 133-170: The bootstrap currently calls hydrate() before the SSE
subscription, so events that occur between hydrate() and the first StreamEvents
connection are missed; fix by starting the StreamEvents goroutine (the one that
runs handler := replayRefreshHandler{...} and calls a.client.StreamEvents) and
block on the streamReady channel before calling a.hydrate; alternatively, ensure
replayRefreshHandler signals streamReady as soon as the first
subscription/connection is established (even if reconnecting is false) so that
hydrate() runs only after the SSE subscription is live; update references to
hydrate, StreamEvents, replayRefreshHandler, streamReady and streamReadyOnce to
implement this ordering change.
- Around line 191-194: The current App.messageHandler swallows errors from
a.engine.HandleMessage; change it to capture the returned error and handle it
(e.g., log the failure with context and the error, emit a metric, and/or enqueue
a retry). Concretely, in function App.messageHandler wrap the call to
a.engine.HandleMessage(ctx, platform, msg) in an if err := ...; err != nil { ...
} block, invoke the service logger (e.g., a.logger.Errorf or similar) including
platform and message identifiers and err, and optionally increment a failure
metric or requeue the message for retry so inbound messages are not silently
dropped.
In `@packages/remote-bridge/internal/pawwork/client.go`:
- Around line 184-186: The loop currently swallows all errors from
c.doJSONWithDirectory while hydrating pending interactions; change it to only
continue on transient errors (network timeouts, temporary 5xx responses) and
return or propagate non-transient/fatal errors (401, 403, 404, etc.) so restart
recovery can't silently drop data. Modify the calls to c.doJSONWithDirectory in
the pending-interaction hydration loop (the block using http.MethodGet
"/permission" and the similar block at the other occurrence) to inspect the
returned error or response status, implement a small helper (e.g.,
isTransientError) that checks for transient network errors or 5xx status codes,
and only continue on those cases — otherwise return the error up the call stack
immediately.
In `@packages/remote-bridge/internal/pawwork/events.go`:
- Around line 105-120: The code calls HandleStreamReady on handler whenever
res.StatusCode is 2xx without validating the response is actually an SSE stream;
change the flow in the function that currently calls
StreamReadyHandler.HandleStreamReady and parseSSE to first validate the response
Content-Type (res.Header.Get("Content-Type")) contains "text/event-stream" (or
"text/event-stream;") and only then invoke HandleStreamReady and parseSSE; if
the content-type is absent or not an SSE type, return an HTTPStatusError (or a
new descriptive error) similar to the existing non-2xx branch so callers don’t
treat non-SSE bodies as ready. Ensure you reference handler, StreamReadyHandler,
HandleStreamReady, parseSSE, and res in the updated logic.
---
Outside diff comments:
In `@packages/desktop-electron/src/main/index.ts`:
- Around line 698-703: killSidecar currently calls remoteAccess.stop() without
awaiting it, allowing the process to exit before the bridge shuts down; make
killSidecar async, await remoteAccess.stop(), and still stop the server (await
server.stop(true) if it returns a promise) and null it; update all callers
(quit/install/relaunch/signal handlers) to await killSidecar() so the Electron
process waits for shutdown, and wrap awaits in a try/catch (or timeout) to avoid
hanging if shutdown fails.
---
Nitpick comments:
In @.github/workflows/build.yml:
- Around line 281-284: The CI step named "Test remote bridge" currently runs "go
test ./..." in the packages/remote-bridge working-directory but omits the race
detector; update this step to also execute the race-enabled tests (either
replace the run command with a sequence that runs both "go test ./..." and "go
test -race ./..." or add a new step that runs "go test -race ./..." with the
same name/working-directory) so the remote-bridge concurrency paths are covered
by the Go race suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e3b0bb1-0d28-4559-918e-d63b95ea9b96
⛔ Files ignored due to path filters (1)
packages/remote-bridge/go.sumis excluded by!**/*.sum
📒 Files selected for processing (37)
.github/workflows/build.yml.github/workflows/desktop-smoke.ymlpackages/app/e2e/snap/settings-shell.snap.tspackages/app/src/app.tsxpackages/app/src/desktop-api-contract.tspackages/app/src/desktop-api.tspackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/settings/remote-behavior.test.tspackages/app/src/pages/settings/remote-source.test.tspackages/app/src/pages/settings/remote.tsxpackages/app/src/pages/settings/settings-shell.tsxpackages/desktop-electron/scripts/build-remote-bridge.tspackages/desktop-electron/scripts/desktop-smoke-workflow-contract.test.tspackages/desktop-electron/scripts/prebuild.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/remote-access-ipc-source.test.tspackages/desktop-electron/src/main/remote-bridge.test.tspackages/desktop-electron/src/main/remote-bridge.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/opencode/src/server/event-replay.test.tspackages/opencode/src/server/event-replay.tspackages/remote-bridge/cmd/pawwork-remote-bridge/main.gopackages/remote-bridge/go.modpackages/remote-bridge/internal/bridge/engine.gopackages/remote-bridge/internal/bridge/engine_test.gopackages/remote-bridge/internal/bridge/session_pointers.gopackages/remote-bridge/internal/gateway/gateway.gopackages/remote-bridge/internal/gateway/gateway_test.gopackages/remote-bridge/internal/pawwork/client.gopackages/remote-bridge/internal/pawwork/client_test.gopackages/remote-bridge/internal/pawwork/events.gopackages/remote-bridge/internal/pawwork/events_test.gopackages/remote-bridge/internal/platforms/platforms.gopackages/remote-bridge/internal/platforms/platforms_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/remote-bridge/internal/bridge/session_pointers.go (1)
48-51:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject parent cycles before committing parent links.
SetParentcan currently creates1 -> s2 -> s1; after that,rootSessionreturns the starting session for each node, so duplicate-root checks can be bypassed and two remote conversations can bind into the same cyclic parent component.Proposed fix
func (p *MemorySessionPointers) SetParent(sessionID string, parentID string) error { if sessionID == "" || parentID == "" { return nil } p.mu.Lock() defer p.mu.Unlock() - if hasAnyRootConflict(p.sessions, withParent(p.parents, sessionID, parentID)) { + nextParents := withParent(p.parents, sessionID, parentID) + if hasParentCycle(nextParents, sessionID) { + return fmt.Errorf("session parent relationship would create a cycle") + } + if hasAnyRootConflict(p.sessions, nextParents) { return fmt.Errorf("session root is already bound to another remote conversation") } - p.parents[sessionID] = parentID + p.parents = nextParents return nil } func (p *FileSessionPointers) SetParent(sessionID string, parentID string) error { if sessionID == "" || parentID == "" { return nil } p.mu.Lock() defer p.mu.Unlock() - if hasAnyRootConflict(p.sessions, withParent(p.parents, sessionID, parentID)) { + nextParents := withParent(p.parents, sessionID, parentID) + if hasParentCycle(nextParents, sessionID) { + return fmt.Errorf("session parent relationship would create a cycle") + } + if hasAnyRootConflict(p.sessions, nextParents) { return fmt.Errorf("session root is already bound to another remote conversation") } - p.parents[sessionID] = parentID + p.parents = nextParents return p.saveLocked() } + +func hasParentCycle(parents map[string]string, sessionID string) bool { + seen := map[string]bool{} + current := sessionID + for current != "" { + if seen[current] { + return true + } + seen[current] = true + current = parents[current] + } + return false +}Also applies to: 166-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remote-bridge/internal/bridge/session_pointers.go` around lines 48 - 51, The SetParent function in packages/remote-bridge/internal/bridge/session_pointers.go does not validate that adding a parent link would not create a cycle before committing the change to p.parents[sessionID] = parentID. Add cycle detection logic after the hasAnyRootConflict check but before assigning the parent link to ensure that parentID is not already a descendant of sessionID (which would create a cycle). This check needs to be applied at both occurrences of the parent assignment: the primary location around lines 48-51 in the SetParent function and the secondary location around lines 166-170, ensuring parent cycles are rejected before they are stored in the parents map.packages/app/src/pages/settings/remote-behavior.test.ts (1)
167-170:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen the post-stop status assertion.
assert(statusCalls > 0)is already satisfied by initial page load, so this won’t catch regressions in stop-path status refresh behavior.Suggested patch
+const callsBeforeStop = statusCalls root.querySelector('[data-action="settings-remote-stop"]').click() await waitFor(() => stopped === 1, "stop should call desktop API") -assert(statusCalls > 0, "settings should refresh bridge status through desktop API") +assert(statusCalls > callsBeforeStop, "stop flow should trigger a fresh status read")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/pages/settings/remote-behavior.test.ts` around lines 167 - 170, The assertion `assert(statusCalls > 0)` after the stop action click is insufficient because the statusCalls counter is already incremented by the initial page load, so this assertion will pass regardless of whether the stop-path actually triggers a status refresh. Replace this assertion with a stronger check that captures the statusCalls value before clicking the stop button, then asserts that statusCalls has increased after the stop action is clicked and the waitFor condition completes. This ensures the test catches regressions where the stop-path fails to refresh the bridge status.
♻️ Duplicate comments (1)
packages/app/src/pages/settings/remote.tsx (1)
185-185:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a safe fallback when mapping status to translation keys.
If backend state ever includes an unexpected value,
STATUS_LABELS[state()]becomesundefined, andlanguage.t(...)gets an invalid key.Suggested patch
- const statusLabel = createMemo(() => language.t(STATUS_LABELS[state()])) + const statusLabel = createMemo(() => { + const key = STATUS_LABELS[state() as keyof typeof STATUS_LABELS] ?? STATUS_LABELS.idle + return language.t(key) + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/pages/settings/remote.tsx` at line 185, The statusLabel createMemo accesses STATUS_LABELS using state() as a key without validation, which results in passing undefined to language.t() if state() contains an unexpected value. Add a fallback mechanism to the statusLabel createMemo that checks if STATUS_LABELS[state()] exists and provides a sensible default translation key (such as a fallback status key or a default "unknown" key) when the state value is not found in the STATUS_LABELS mapping. This ensures language.t() always receives a valid key regardless of what state() returns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/opencode/test/github/build-workflow.test.ts`:
- Around line 197-203: Add a missing assertion to validate the if condition on
the remoteBridgeTestStep. After the existing assertions on remoteBridgeTestStep
(which validate run and working-directory), add an expectation that
remoteBridgeTestStep?.if equals "${{ inputs.phase != 'finalize' }}" to ensure
consistency with the race test step and prevent regressions if this condition is
removed from the actual workflow.
---
Outside diff comments:
In `@packages/app/src/pages/settings/remote-behavior.test.ts`:
- Around line 167-170: The assertion `assert(statusCalls > 0)` after the stop
action click is insufficient because the statusCalls counter is already
incremented by the initial page load, so this assertion will pass regardless of
whether the stop-path actually triggers a status refresh. Replace this assertion
with a stronger check that captures the statusCalls value before clicking the
stop button, then asserts that statusCalls has increased after the stop action
is clicked and the waitFor condition completes. This ensures the test catches
regressions where the stop-path fails to refresh the bridge status.
In `@packages/remote-bridge/internal/bridge/session_pointers.go`:
- Around line 48-51: The SetParent function in
packages/remote-bridge/internal/bridge/session_pointers.go does not validate
that adding a parent link would not create a cycle before committing the change
to p.parents[sessionID] = parentID. Add cycle detection logic after the
hasAnyRootConflict check but before assigning the parent link to ensure that
parentID is not already a descendant of sessionID (which would create a cycle).
This check needs to be applied at both occurrences of the parent assignment: the
primary location around lines 48-51 in the SetParent function and the secondary
location around lines 166-170, ensuring parent cycles are rejected before they
are stored in the parents map.
---
Duplicate comments:
In `@packages/app/src/pages/settings/remote.tsx`:
- Line 185: The statusLabel createMemo accesses STATUS_LABELS using state() as a
key without validation, which results in passing undefined to language.t() if
state() contains an unexpected value. Add a fallback mechanism to the
statusLabel createMemo that checks if STATUS_LABELS[state()] exists and provides
a sensible default translation key (such as a fallback status key or a default
"unknown" key) when the state value is not found in the STATUS_LABELS mapping.
This ensures language.t() always receives a valid key regardless of what state()
returns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d86ae67-9407-4e42-9a7a-63ebcd69f5cf
📒 Files selected for processing (22)
.github/workflows/build.yml.github/workflows/desktop-smoke.ymlpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/settings/remote-behavior.test.tspackages/app/src/pages/settings/remote-platform-icons.tspackages/app/src/pages/settings/remote-platforms.tsxpackages/app/src/pages/settings/remote.tsxpackages/app/src/pages/settings/settings-shell.tsxpackages/desktop-electron/scripts/predev.tspackages/desktop-electron/src/main/remote-bridge.test.tspackages/desktop-electron/src/main/remote-bridge.tspackages/opencode/test/github/build-workflow.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.tspackages/remote-bridge/cmd/pawwork-remote-bridge/main.gopackages/remote-bridge/internal/bridge/engine.gopackages/remote-bridge/internal/bridge/session_pointers.gopackages/remote-bridge/internal/bridge/session_pointers_test.gopackages/remote-bridge/internal/gateway/gateway.gopackages/remote-bridge/internal/pawwork/client.gopackages/remote-bridge/internal/pawwork/client_test.gopackages/remote-bridge/internal/pawwork/events.go
✅ Files skipped from review due to trivial changes (1)
- packages/app/src/pages/settings/remote-platform-icons.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- .github/workflows/build.yml
- packages/remote-bridge/internal/pawwork/client.go
- packages/remote-bridge/internal/gateway/gateway.go
- .github/workflows/desktop-smoke.yml
- packages/remote-bridge/internal/pawwork/client_test.go
- packages/desktop-electron/src/main/remote-bridge.test.ts
- packages/desktop-electron/src/main/remote-bridge.ts
- packages/remote-bridge/internal/bridge/engine.go
- packages/remote-bridge/internal/pawwork/events.go
- packages/app/src/pages/settings/settings-shell.tsx
Self-contained Go module (own go.mod, no app/electron deps) wrapping github.com/chenhg5/cc-connect platform adapters: bridge engine, gateway, pawwork client + event stream, session pointers, platform registry. ~51% tests. Split out of #1275 as PR 1/2 for reviewability.
Astro-Han
force-pushed
the
codex/i1188-remote-gateway
branch
from
8e4e6dd to
61ad3b5
Compare
Astro-Han
changed the title
github-actions
Bot
removed
app
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/remote-bridge/internal/pawwork/events_test.go (2)
315-406: ⚡ Quick winAdd positive test case for
HandleStreamReadyinvocation.The test suite includes
TestClientRejectsNonSSEEventStreamBeforeReady, which verifies thatHandleStreamReadyis NOT called when the content-type is incorrect. However, there's no positive test verifying thatHandleStreamReadyIS called when:
- The content-type is
text/event-stream, and- The handler implements
StreamReadyHandler.Consider adding a test case that uses
streamReadyEventHandlerwith a valid SSE response and asserts thathandler.ready == 1afterStreamEventscompletes successfully.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remote-bridge/internal/pawwork/events_test.go` around lines 315 - 406, Add a new positive test case to verify that HandleStreamReady is called when the server returns a valid SSE response. Create a test function that sets up an httptest server returning content-type text/event-stream with valid SSE data (similar to TestClientStreamsGlobalEvents), uses a streamReadyEventHandler instance, calls New(server.URL).StreamEvents(t.Context(), handler), and asserts that handler.ready equals 1 after successful completion, contrasting with the existing TestClientRejectsNonSSEEventStreamBeforeReady which verifies the handler.ready remains 0 when content-type is invalid.
80-313: Add test coverage forserver.connectedevent.The DispatchEvent tests cover most event types, but the
server.connectedevent type is implemented inevents.go(line 141-146) to triggerHandleReplayRefresh. A test case for this event routing is missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remote-bridge/internal/pawwork/events_test.go` around lines 80 - 313, Add a new test function following the existing pattern in events_test.go to cover the server.connected event routing. Create a test that calls DispatchEvent with a JSON payload containing the server.connected event type and verify that it triggers the HandleReplayRefresh callback on the fakeEventHandler. This test should follow the same structure as the other TestDispatchEvent* functions, checking that the handler properly processes the connected event.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/remote-bridge/internal/pawwork/events_test.go`:
- Around line 315-406: Add a new positive test case to verify that
HandleStreamReady is called when the server returns a valid SSE response. Create
a test function that sets up an httptest server returning content-type
text/event-stream with valid SSE data (similar to
TestClientStreamsGlobalEvents), uses a streamReadyEventHandler instance, calls
New(server.URL).StreamEvents(t.Context(), handler), and asserts that
handler.ready equals 1 after successful completion, contrasting with the
existing TestClientRejectsNonSSEEventStreamBeforeReady which verifies the
handler.ready remains 0 when content-type is invalid.
- Around line 80-313: Add a new test function following the existing pattern in
events_test.go to cover the server.connected event routing. Create a test that
calls DispatchEvent with a JSON payload containing the server.connected event
type and verify that it triggers the HandleReplayRefresh callback on the
fakeEventHandler. This test should follow the same structure as the other
TestDispatchEvent* functions, checking that the handler properly processes the
connected event.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 533f5ee9-80dc-4122-be07-a5186165912a
⛔ Files ignored due to path filters (1)
packages/remote-bridge/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
.github/workflows/build.ymlpackages/remote-bridge/cmd/pawwork-remote-bridge/main.gopackages/remote-bridge/go.modpackages/remote-bridge/internal/bridge/engine.gopackages/remote-bridge/internal/bridge/engine_test.gopackages/remote-bridge/internal/bridge/session_pointers.gopackages/remote-bridge/internal/bridge/session_pointers_test.gopackages/remote-bridge/internal/gateway/gateway.gopackages/remote-bridge/internal/gateway/gateway_test.gopackages/remote-bridge/internal/pawwork/client.gopackages/remote-bridge/internal/pawwork/client_test.gopackages/remote-bridge/internal/pawwork/events.gopackages/remote-bridge/internal/pawwork/events_test.gopackages/remote-bridge/internal/platforms/platforms.gopackages/remote-bridge/internal/platforms/platforms_test.go
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/remote-bridge/internal/platforms/platforms_test.go
- packages/remote-bridge/internal/platforms/platforms.go
- packages/remote-bridge/internal/bridge/session_pointers_test.go
- packages/remote-bridge/go.mod
- .github/workflows/build.yml
- packages/remote-bridge/cmd/pawwork-remote-bridge/main.go
- packages/remote-bridge/internal/pawwork/events.go
- packages/remote-bridge/internal/bridge/session_pointers.go
- packages/remote-bridge/internal/gateway/gateway.go
- packages/remote-bridge/internal/bridge/engine_test.go
- packages/remote-bridge/internal/gateway/gateway_test.go
- packages/remote-bridge/internal/pawwork/client_test.go
- packages/remote-bridge/internal/pawwork/client.go
- packages/remote-bridge/internal/bridge/engine.go
Behavior-preserving simplifications surfaced by two independent reviews. Full test suite + race detector green; per-package coverage unchanged. - session_pointers: merge Memory/File dual impl into one store with an optional path (saveLocked is a no-op when path is empty) - engine: collapse permissionOrder/questionOrder/blockerOrder into a single blockerOrder; derive oldest-of-kind by scanning it directly - engine: drop the unreachable "Unknown command" branch; handleCommand now returns (handled, err) and isRemoteCommand is gone - engine: inline the single-field Prompt wrapper to a plain string arg - engine: slices.DeleteFunc for blocker filtering; inline single-use Handle*Resolved forwards and pendingBlocker alias locals - gateway: defer stopPlatforms once instead of repeating it on every return - client: drop the ListQuestions JSON marshal round-trip - events: one flush closure for the parseSSE dispatch path Net -187 lines.
Review P3. The question prompt only said "Reply with a number or answer text", but the parser requires one line per question (multi-question) and comma-separated numbers (multi-select) — users couldn't know the format. - questionReplyHint: prompt now states the exact format per question shape (single / multi-select / multi-question), with an example - accept full-width "，" and ideographic "、" commas in multi-select replies so replies typed on a Chinese keyboard parse like ASCII ones - tests for the per-shape hint text and the comma variants Single-question prompt text unchanged.
Review P2. The client used http.DefaultClient (no timeout) and JSON calls rode only the outer ctx, so a stalled PawWork sidecar could hang startup and hydration indefinitely. - doJSONWithDirectory (the single JSON funnel) wraps ctx with a 30s timeout - the SSE stream builds its own request and stays intentionally unbounded - test: a slow server that never responds makes ListSessions return promptly
Review P1. parseSSE advances the global SSE cursor even when a chat-platform delivery fails, so a transient blip silently drops the assistant message. The cursor is global and shared, so holding it (the reviewer's proposed replay) would wedge every session's stream on any permanent failure. First principle: the cursor tracks ingestion, not delivery. Keep it advancing; make delivery best-effort-good by retrying at the sink. - deliverWithRetry: assistant text gets bounded retries (3) with short backoff, then gives up (the caller logs it) — the cursor is never held - scoped to assistant text only: permissions/questions are already reconciled by the gateway's hydrate on every reconnect, and a test pins their delivery as single-shot, so they must not be retried here - test: transient failure recovers; sustained failure stops after N attempts The global-cursor wedge risk was confirmed by an independent review.
github-actions
Bot
added
the
harness
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opencode/test/github/build-workflow.test.ts (1)
200-205: ⚡ Quick winConsider validating the setup-go step for completeness.
The test validates setup-node (line 172) but doesn't validate the setup-go step, which is equally critical for the remote-bridge Go tests to run. Adding validation would:
- Ensure
go-version-filepoints topackages/remote-bridge/go.mod- Ensure
cache-dependency-pathpoints topackages/remote-bridge/go.sum- Prevent regressions in Go toolchain configuration
- Maintain consistency with other setup step validations in this test
📋 Suggested additions
After line 105, add:
const runtimeImportGuardStep = steps.find((step) => step.name === "Check desktop runtime imports") +const setupGoStep = steps.find( + (step) => step.uses === "actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff", +) const remoteBridgeTestStep = steps.find((step) => step.name === "Test remote bridge")After line 199, add:
expect(runtimeImportGuardStep?.["working-directory"]).toBe("packages/desktop-electron") +expect(setupGoStep?.with).toEqual({ + "go-version-file": "packages/remote-bridge/go.mod", + "cache-dependency-path": "packages/remote-bridge/go.sum", +}) expect(remoteBridgeTestStep?.if).toBe("${{ inputs.phase != 'finalize' }}")Optionally, add step ordering assertions after line 205:
expect(remoteBridgeRaceStep?.["working-directory"]).toBe("packages/remote-bridge") +expect(steps.indexOf(remoteBridgeTestStep!)).toBeGreaterThan(steps.indexOf(setupGoStep!)) +expect(steps.indexOf(remoteBridgeRaceStep!)).toBeGreaterThan(steps.indexOf(setupGoStep!)) expect(steps.indexOf(runtimeImportGuardStep!)).toBeGreaterThan(steps.indexOf(buildElectronAppStep!))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/test/github/build-workflow.test.ts` around lines 200 - 205, The test is missing validation for the setup-go step even though it validates setup-node, which is inconsistent. Add validation assertions for the setup-go step to verify that go-version-file is set to packages/remote-bridge/go.mod and cache-dependency-path is set to packages/remote-bridge/go.sum. This should be added alongside the existing setup-node validations to ensure the Go toolchain is properly configured for the remote-bridge tests, consistent with how remoteBridgeTestStep and remoteBridgeRaceStep are currently validated. Optionally add step ordering assertions to ensure setup-go runs before the test steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/remote-bridge/internal/pawwork/client.go`:
- Around line 267-278: The canSkipHydrationDirectoryError function currently
skips all context.DeadlineExceeded errors indiscriminately, including when the
caller's own context has already expired, which causes hydration to incorrectly
return partial results after shutdown or upstream timeouts. Modify
canSkipHydrationDirectoryError to accept the caller's context as a parameter and
add a check that returns false if the caller's context is already done (by
calling ctx.Err()). This way, the function will distinguish between deadline
exceeded errors from the hydration operation itself versus timeouts caused by
the caller's context being expired, ensuring that caller-initiated cancellations
are not treated as skippable errors.
---
Nitpick comments:
In `@packages/opencode/test/github/build-workflow.test.ts`:
- Around line 200-205: The test is missing validation for the setup-go step even
though it validates setup-node, which is inconsistent. Add validation assertions
for the setup-go step to verify that go-version-file is set to
packages/remote-bridge/go.mod and cache-dependency-path is set to
packages/remote-bridge/go.sum. This should be added alongside the existing
setup-node validations to ensure the Go toolchain is properly configured for the
remote-bridge tests, consistent with how remoteBridgeTestStep and
remoteBridgeRaceStep are currently validated. Optionally add step ordering
assertions to ensure setup-go runs before the test steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b6f999bd-b0cd-45cc-9f04-6e8c7d6027bf
📒 Files selected for processing (8)
packages/opencode/test/github/build-workflow.test.tspackages/remote-bridge/internal/bridge/engine.gopackages/remote-bridge/internal/bridge/engine_test.gopackages/remote-bridge/internal/gateway/gateway_test.gopackages/remote-bridge/internal/pawwork/client.gopackages/remote-bridge/internal/pawwork/client_test.gopackages/remote-bridge/internal/pawwork/events.gopackages/remote-bridge/internal/pawwork/events_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/remote-bridge/internal/pawwork/client_test.go
- packages/remote-bridge/internal/gateway/gateway_test.go
- packages/remote-bridge/internal/bridge/engine_test.go
- packages/remote-bridge/internal/pawwork/events_test.go
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/remote-bridge/internal/bridge/engine.go (1)
802-808:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPotential key collision if channel or user IDs contain colons.
strings.Join(parts, ":")can produce identical keys for distinct entities when any component contains a colon. For example, platform "p" with channel "a:b" and user "c" collides with channel "a" and user "b:c". Colliding keys would cause unrelated users to share the same session pointer.Consider using a non-printable separator (as
questionKeydoes with\x00) or URL-encoding each component before joining.Suggested fix using NUL separator
func remoteKey(platform core.Platform, msg *core.Message) string { if strings.TrimSpace(msg.SessionKey) != "" { return msg.SessionKey } - parts := []string{platform.Name(), msg.ChannelID, msg.UserID} - return strings.Join(parts, ":") + return platform.Name() + "\x00" + msg.ChannelID + "\x00" + msg.UserID }Note: This requires updating
restoreDeliveryline 321 to usestrings.Cut(remoteKey, "\x00").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/remote-bridge/internal/bridge/engine.go` around lines 802 - 808, The remoteKey function uses a colon separator in strings.Join which can cause key collisions if any of the component parts (platform name, channel ID, or user ID) contain colons themselves. Replace the colon separator with a non-printable separator like the NUL character \x00 (as already used in the questionKey function). You must also update the restoreDelivery function to parse the remoteKey using strings.Cut with the matching \x00 separator instead of its current parsing approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/remote-bridge/internal/bridge/engine.go`:
- Around line 802-808: The remoteKey function uses a colon separator in
strings.Join which can cause key collisions if any of the component parts
(platform name, channel ID, or user ID) contain colons themselves. Replace the
colon separator with a non-printable separator like the NUL character \x00 (as
already used in the questionKey function). You must also update the
restoreDelivery function to parse the remoteKey using strings.Cut with the
matching \x00 separator instead of its current parsing approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f10e6d9-07aa-409b-89a5-73ab13a9212f
📒 Files selected for processing (4)
packages/remote-bridge/internal/bridge/engine.gopackages/remote-bridge/internal/bridge/engine_test.gopackages/remote-bridge/internal/pawwork/client.gopackages/remote-bridge/internal/pawwork/client_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/remote-bridge/internal/pawwork/client_test.go
- packages/remote-bridge/internal/pawwork/client.go
- packages/remote-bridge/internal/bridge/engine_test.go
After answering the current blocker, handlePendingReply auto-surfaces the next queued prompt. A persistently failing delivery dropped that prompt and discarded the error, leaving no log signal until the next hydrate. Propagate surfaceActiveBlocker's error up to HandleMessage so the gateway's existing warn logging catches it.
When the next queued prompt failed to deliver, the blocker was dropped and only recovered at the next reconnect hydrate. Keep it undelivered instead: it stays unanswerable so it cannot intercept an ordinary message, and the next inbound message retries surfacing it now that the user is reachable. This deletes the drop/loop in surfaceActiveBlocker (the delivered flag already prevents interception).
…ging the cursor A permission/question/session event that failed to decode was logged and skipped, recovered only at the next reconnect hydrate — an unbounded blind window for a safety-critical confirmation on a healthy long-lived stream. Mark such decode failures repairable; parseSSE advances the cursor past the bad frame first (so a reconnect can never replay it and wedge the global stream), then hydrates immediately to reconcile from the REST list endpoints. Assistant text (no REST backstop) and resolved events (hydrate cannot clear them) stay non-repairable.
Replace the single wildcard-Feishu case with a table covering the hasRemoteAudience contract: specific audience accepted, wildcard/empty/blank rejected, Feishu/Lark group_only both ways, allow_chat ignored for other platforms. Also make the run-loop tests' request counters atomic — they were written in the httptest handler goroutine and read in the test body, a pre-existing data race that -race intermittently catches.
A permission.asked without id/sessionID, a session.created without info.id, and a question tool event without sessionID/messageID/callID decoded fine but were silently skipped, so their state waited for the next reconnect hydrate. Treat these missing-field cases as repairable like a decode failure: advance the cursor past the frame, then hydrate immediately. Rename the marker type to repairableEventError since it now covers validation, not just decoding.
A bad SetParent that points a session's ancestry back at itself would form a cycle. rootSession then returns the starting node instead of a true root, letting the cycled session bypass the duplicate-root guard and bind the wrong remote conversation on restore/delivery. Walk the prospective parent chain before mutating; reject and leave state unchanged when it already reaches the child.
hydrate fetched ListSessions(ctx, 0), which the client treats as unbounded and pulls the full session history on every startup and reconnect. Users with many sessions pay a slow warm-up. Parent links are restored from the persisted pointer store and live event replay, so the scan only needs the most recently active sessions. Cap it at hydrateSessionLimit (100).
The picker cached the listed sessions and /sessions N reused that cache whenever it was non-empty. A delayed pick then switched to a session from a stale snapshot instead of the current recent list. Drop the picker cache entirely and re-fetch the bounded session list in switchSession, so N always resolves against what is live now. This also removes per-conversation in-memory state that was never cleared.
saveLocked always wrote a fixed <path>.tmp before renaming. Two bridge processes sharing one state path then race on that single temp: one renames it away before the other's rename, failing the second write with ENOENT (clobbering cursor/session updates). Write each snapshot through os.CreateTemp in the same directory and rename it into place, with a deferred cleanup that is a no-op once the rename succeeds. The in-process mutex is unchanged.
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
1 participant
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
Standalone Go chat companion bridge (
packages/remote-bridge/) — PR 1 of 2, split out of the original combined remote-access branch for reviewability. This PR adds only the self-contained Go subsystem; the desktop integration + settings UI follow in PR 2 (built on this branch).New module — own
go.mod, depends only ongithub.com/chenhg5/cc-connect, zero imports back into the app/Electron:internal/bridge— engine + persisted session pointers (message ↔ session ↔ agent); bounded sink-retry for user-visible deliveriesinternal/gateway— wires platforms + engine + PawWork client; audience gating; hydrate-on-reconnectinternal/pawwork— HTTP client + SSE event stream/replay to the PawWork serverinternal/platforms— platform registry (cc-connect adapters)cmd/pawwork-remote-bridge— binary entrypoint.github/workflows/build.yml): setup-go +go test ./...+ race tests onbridge/gateway/pawwork, with a meta-test inpackages/opencodepinning those steps~4.9k insertions, 0 deletions, all under
packages/remote-bridge/plus the CI step. Purely additive — touches no existing app runtime code, so it ships dormant until PR 2 wires it in.Why
Issue #1188 (mobile companion): let users monitor and interact with running agents from chat platforms (Slack/Discord/QQ/Feishu) on their phone. Landing the backend bridge in isolation lets it be reviewed module-by-module without the desktop/UI surface.
Related Issue
Part of #1188 (PR 1 of 2; the desktop + UI half in PR 2 completes and closes it).
Human Review Status
Pending
Review Focus
internal/pawwork+internal/bridge): the global SSE cursor tracks ingestion and always advances. User-visible deliveries (assistant text, permission/question prompts) get a bounded sink-retry; permission/question are additionally reconciled by gateway hydrate on reconnect. The shared cursor is never held on a delivery failure — doing so would wedge every session's stream.internal/pawwork/client.go): fatal stream errors (401/403/404, non-SSE content-type) stop the reconnect loop; only genuinely transient hydration errors (408/429/5xx, timeouts) are skipped — JSON/protocol errors surface rather than silently dropping pending interactions.internal/gateway): who is allowed to drive a session from chat.Risk Notes
go.mod:github.com/chenhg5/cc-connect(chat-platform adapters). No change to the app's dependency tree.SessionPointersStore) via plainpath/filepath; cross-platform.How To Verify
Screenshots or Recordings
None — no visible UI in this PR.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Release Notes
New Features
pawwork-remote-bridgecommand with JSON config loading,-list-platformsoutput, and signal-aware execution.Tests
Chores
finalize.