[melodic-automation-lanes]

Closes #6109

Summary

Opened by the work-resolve-issue lane. The run's own summary:

The watcher now exits after N consecutive empty waits so an orphaned watcher no longer keeps the server and token alive. The view bridge splits a watcher-only token (wait and lease release) from the page token (actions only). U+2028 and U+2029 are escaped after JSON encoding in both send paths. The README documents the single-user limit and the header-check limit.

Not verified

• scripts/run-test-file.sh on lib/session-bridge/test_session_bridge.py and test_view_bridge.py was not run: the sandbox denied it, and the new and existing tests are unexecuted in this run. Left to the PR's CI.
• Changes are uncommitted in the working tree, per the runner's instruction; the runner makes the patch.
• Changelog fragments were written by hand because new-changelog-fragment needs a branch and the checkout is detached; check-changelog-fragments accepts them.

Fix

• lib/session-bridge/watch.sh: exits 4 after WAIT_EMPTY (default 40) consecutive empty waits with a re-arm message; reads WATCH_TOKEN from the env file, else TOKEN
• lib/session-bridge/session_bridge.py: watch_token on LoopbackWatcher (falls back to the page token), watch_token_ok gating /api/wait and POST /api/lease, WATCH_TOKEN in the env file and watchToken in the session json, release_lease and ChannelRelay use it, dumps_line escapes U+2028/U+2029 in send(),
• lib/session-bridge/view_bridge.py: ViewHub sets its own watch_token that /api/token never serves
• lib/session-bridge/README.md: single-user workstation limit, header checks do not stop a non-browser local client, token scopes, exit 4, U+2028/U+2029 escaping
• lib/session-bridge/test_session_bridge.py: tests for the exit-4 idle exit, delivery before the limit, and U+2028/U+2029 escaping in the wait answer and the channel server
• lib/session-bridge/test_view_bridge.py: tests that the page token is refused on wait and lease release, the watcher token is accepted there and refused on actions, and /api/token never returns the watcher token
• plugins/{planning,review,work-items}/view-bridge/, plugins/planning/{surface,shared}/: regenerated copies via sync-shared-copies
• .changes/{planning,review,work-items}/fix-view-bridge-hardening-*.md: patch fragments

Verification

The runner ran each check from the base copy against 7646d5f:

• scripts/check-changelog-fragments.sh --check: passed

The run reports:

• sync-shared-copies: exit 0, regenerated 13 plugin copies
• check-changelog-fragments: all 6 fragments valid
• check-affected: 11 passed, 5 skipped, rest CI-only
• fresh-context subagent review of the diff: no confirmed problems
• The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40112490239

Left out: the items under Not verified.

Related

#6109

[github-actions]

claude-review reviewed 12ddb24f78732f4bcc6e07f611e8533768274968: 2 finding(s), 0 as inline comments.

Code review: PR #9806 (session-bridge hardening, #6109)

The change meets the issue's acceptance criteria on the page. I found two documentation gaps, neither blocking. I did not run the tests, and the PR body says they were not run either, so the new tests are unexecuted as far as I know.

Checks I made against the diff and the files

• Watcher exit: watch.sh exits 4 after WAIT_EMPTY (default 40) consecutive empty waits and prints the re-arm command. The counter lives only in the empty-wait branch, so any event, error or lease exit takes its own path. Tests cover the idle exit and delivery before the limit.
• Token split: /api/wait (long_poll) and POST /api/lease use watch_token_ok; every other POST uses the page token. ViewHub sets its own watch_token, and /api/token serves only self.token. GET /api/lease was already unauthenticated, so it is unchanged. Tests cover the page token refused on wait and release, the watcher token refused on actions, and /api/token never returning the watcher token.
• Fallback: without a hub watch_token, watch_token falls back to the page token, and WATCH_TOKEN is omitted from the env file. watch.sh, ChannelRelay.session and release_lease all fall back to TOKEN, so other LoopbackWatcher consumers keep working.
• U+2028 and U+2029: I confirmed with a character search that dumps_line in lib/session-bridge/session_bridge.py holds the real code points, not spaces. It is used in send(), ChannelServer.send and tool_events.
• Copies: the regenerated plugin copies I read (session_bridge.py and watch.sh under planning/shared, planning/surface and planning/view-bridge) match the lib/ source hunks. I read the diff only to line 998 of 1695, so I did not read the rest of the copies.
• Changelog fragments: all three are bump: patch, which fits a fix. There are no manifest changes, so no manifest <field> lines apply.

Findings (documentation, advisory)

1. plugins/review/skills/explain-change/SKILL.md line 138 is not in this diff. Its watcher step (step 7) covers only exit 3 (another session holds the view, or it was stopped) and exit 2. The watcher now also exits 4 when idle, with the re-arm message on stderr, and this procedure doesn't say what to do. An agent following it has to improvise whether to re-arm or stop. To reproduce: let the watch run for 40 empty waits (about an hour at the default 90 s) and see which instruction step 7 gives. Add one sentence to step 7: exit 4 means the watcher idled out, so re-run wake.sh while the reader is still there. Check the other skills that arm watch in planning and work-items, which I did not read.
2. The lib/session-bridge/README.md protocol section is now inconsistent with the new text. Line 48 still says every POST and /api/wait need the one token, and line 57 lists the env file keys as PID, PORT, TOKEN, NONCE without WATCH_TOKEN. The new paragraph at lines 49-51 and the changes at lines 82-89 correct the substance, but these two lines now contradict it for an app that sets a watcher token.

Spend: none beyond the review lane's own run. The change raises no real charge.

Closing issue #6109: read. The brief's acceptance criteria are met by the changes and tests listed above. The criterion that scripts/run-test-file.sh passes is unverified in this review, as the PR notes state. I did not run it.

[melodic-automation-lanes]

The pr-unblock lane stopped on this pull request and needs a person: the lane could not fix every failing check; its run log names each one and why.

• Reason: ci-fix-escalated
• Resume key: 12ddb24f78732f4bcc6e07f611e8533768274968 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=9806 and activity=fix-ci:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=9806 -f activity=fix-ci

[github-actions]

claude-security-review reviewed 12ddb24f78732f4bcc6e07f611e8533768274968: 0 finding(s), 0 as inline comments.

No security findings.

critical-findings: 0

I read the whole change. The 22 files are the lib/session-bridge source, its tests, README and three changelog fragments, plus regenerated copies of session_bridge.py, view_bridge.py and watch.sh under plugins/{planning,review,work-items}. I read the copies as hunks of the same change and found no difference from the lib/ source.

• Watcher-only token.

  • The wait and lease-release gates now check watch_token_ok (long_poll and /api/lease in lib/session-bridge/session_bridge.py). Both use secrets.compare_digest.
  • ViewHub sets its own watch_token, and /api/token still serves only hub.token.
  • Every other POST route still uses token_ok, so a page that holds the page token cannot read events or release the lease. Both are covered by the new tests.
  • Other apps keep the page token on all three routes, because _watch_token is None and the property falls back to token.
  • The watcher token is written only to the env file and session json, both created with write_private (mode 0600).
  • set_wait_timeout rewrites the env file but keeps every line except WAIT_TIMEOUT=, so WATCH_TOKEN survives.
  • release_lease, ChannelRelay.session and watch.sh all fall back to the page token when no watcher token exists, so they do not break.

• Page token fetch. The README now says the /api/token header checks do not stop a non-browser local client, and that the bridge assumes a single-user workstation. This is a documented limit the owner chose, so it is not a finding.

• U+2028 and U+2029. dumps_line escapes both after json.dumps(..., ensure_ascii=False). It is used in LoopbackHandler.send, ChannelServer.send and tool_events. I could not tell from the diff view whether the .replace() arguments are the literal U+2028 and U+2029 characters, because they render like spaces. grep of lib/session-bridge/session_bridge.py matches the lines, and the tests assert that spaces survive. If those arguments were plain spaces, the tests would fail in CI.

• Idle exit. watch.sh validates WAIT_EMPTY as a positive integer and falls back to 40. The max_empty and empty values never reach a command, and the re-arm message quotes the path with shell_quote. Exit 4 only ends the watcher; it does not widen anything.

• Trusted paths: no changed file matches TRUSTED PATHS:.

• Manifests: no plugin.json or marketplace.json changed, so there are no manifest <field> lines.

• Instruction-surface deletions: none. The README edits add limits and remove no standing rule.

• PR context: it contains no embedded instructions aimed at the review lane.

• Spend: none.

The new and existing session-bridge tests were not run in the author's sandbox. I did not run them either. CI has to run them.

[melodic-automation-lanes]

Unattended merge: native auto-merge leaves this pull request unarmed at head 12ddb24f78732f4bcc6e07f611e8533768274968. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: incremental-gap)

[kyle-sexton]

Hub (GitHub Support): removing the lane-added needs-human. Every check except ci-status is green, and ci-status is red only because of this label. pr-unblock escalated one minute after its own fix landed, a deadlock, not one of the owner's stop reasons (intent unclear, direction changed, high security risk, induced cost). The security review on this head was clean (0 findings). pr-merge's gate still applies before any merge.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-25T17:39:16.012623Z	12ddb24	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[melodic-automation-lanes]

The unattended merge path armed native auto-merge at head 12ddb24f78732f4bcc6e07f611e8533768274968.