* fix(gateway): scope per-run message and event reads to the run's data identity
start_run stamps run-event rows with the raw trusted owner id, but
GET /runs/{rid}/messages and /runs/{rid}/events called the event store with
user_id=AUTO, which resolves to the internal caller's make_safe_user_id form.
On the database event store, owners whose id contains characters outside
[A-Za-z0-9_-] (IM channel ids such as "feishu:owner-777") therefore got an
empty list. The regenerate source-run scan had the same mismatch and fell
back or returned 409.
Pass _run_scope_user_id() explicitly, as the thread message routes already
do (#5448), and add user_id to RunEventStore.list_messages_by_run() so every
backend accepts it like list_messages()/list_events(). Browser and API
sessions keep their per-user filter.
* fix(gateway): scope workspace-changes and archive reads to the run's data identity
The same raw-owner mismatch reached two more run-scoped routes. On the SQL
stores, GET /runs/{rid}/workspace-changes reported no changes because its
event lookup used user_id=AUTO, and GET /runs/{rid}/artifacts/archive
answered 404 because both its run-row lookup and its run.delivery event
lookup did.
Pass _run_scope_user_id() to both, and let get_workspace_changes_response()
forward an explicit user_id to the event store.
* docs(changelog): reference #6282 in the run-scope read fix entries
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(workspace-changes): record symlink targets without the verbatim prefix
os.readlink on Windows reports absolute targets in extended-length
form (\?\C:\... or \?\UNC\server\share). The scanner stored that raw
spelling, so workspace-change events showed \?\-prefixed targets that
do not match ordinary Windows paths. Strip the prefix when recording;
POSIX readlink output is unchanged. Skills projection/review readlink
sites are untouched — they have no user-facing contract pinned on the
spelling.
* fix(workspace-changes): gate symlink target normalization to Windows
Review follow-up on #5250:
- Gate _normalize_symlink_target on os.name == "nt". readlink(2) on
POSIX returns the literal string the link was created with, and
backslash is a valid filename byte on Linux, so a target that starts
with the extended-length prefix there must be recorded verbatim. The
docstring's POSIX claim is now provably true.
- Commit the unit checks the PR body previously described as ad-hoc:
drive and UNC prefix stripping, relative and plain POSIX targets,
mid-string prefix left verbatim, and an off-Windows identity case, so
the new branch has real coverage on every platform instead of relying
on a Windows host with symlink privilege.
* test(workspace-changes): force Windows platform in prefix-strip unit tests
The os.name gate added in the previous commit makes
_normalize_symlink_target a verbatim identity off-Windows, so the two
prefix-strip assertions failed on the ubuntu-only unit CI. Force
os.name to "nt" via monkeypatch in both, mirroring the off-Windows
identity test, so every case pins exactly one platform's contract and
the suite is green on every host.
* fix(workspace-changes): strip only extended drive-letter prefixes
Review follow-up on #5250: the catch-all branch also stripped the
extended-length prefix from volume-GUID targets (\?\Volume{...}\...),
leaving a relative-looking path that loses the target's namespace.
Restrict the branch to extended drive-letter paths (letter, colon,
separator) and keep every other \?\ namespace form verbatim; add the
volume-GUID regression plus degenerate-prefix cases.
* fix(mcp): exclude internal temp files from workspace changes
* fix(mcp): address review — shared tmp subdir constant, any-depth docs, nested test
- Export MCP_TMP_SUBDIR from constants.py and import it in both stdio
launch paths (tools.py, task_tool_caller.py) so the "/tmp" suffix is
composed once.
- Document that the .mcp exclusion matches by directory name at any
depth (like .git/node_modules) in README.md and mcp/AGENTS.md —
subagent work dirs below the workspace root get their own .mcp/tmp.
- Pin the any-depth semantic in test_workspace_changes.py with a nested
workspace/project/.mcp assertion.
* docs(mcp): correct any-depth exclusion rationale; re-home tmp pinning comment
The previous commit justified the any-depth `.mcp` exclusion with subagent
work dirs sitting below the workspace root — a mechanism that doesn't
exist: subagents share the parent's thread_id and both stdio launch paths
resolve sandbox_work_dir(thread_id), so `.mcp/tmp` is always pinned at the
workspace root. Reword mcp/AGENTS.md and the test comment to the real
justification (consistency with the other reserved dir names, robustness
against a server creating a relative `.mcp` from another cwd).
Also move the orphaned "pinning the process temp dir" rationale from
tools.py to constants.py next to MCP_TMP_SUBDIR, where both importers see it.
* fix: bound MCP server bring-up timeouts and exclude externalized tool outputs from delivery verification
Two related robustness fixes:
1. MCP server bring-up was unbounded. tool_call_timeout only covered
session.call_tool(); tool discovery (subprocess spawn + initialize +
tools/list) and persistent stdio session initialization could hang
forever, blocking agent construction (and on the Gateway event loop,
the whole process). Add a per-server session_init_timeout
(default DEFAULT_MCP_SESSION_INIT_TIMEOUT = 60s, null disables) that
bounds both discovery and pooled-session initialization. The session
pool's existing cancellation handling tears down a session stuck
mid-creation in its own task.
2. ToolOutputBudgetMiddleware externalizes oversized tool outputs into
outputs/.tool-results/ (configurable tool_output.storage_subdir). The
workspace-change scanner and run delivery verification counted those
files as produced artifacts, so any run that externalized a tool output
without also presenting a real artifact failed with
"Artifact delivery incomplete". Exclude TOOL_RESULTS_DIRNAME via a
shared constant (mirroring BROWSER_FRAMES_DIRNAME) and thread the
configured storage_subdir through snapshot capture so both
workspace-changes events and delivery verification stay clean.
* review: enforce single-segment tool_output.storage_subdir; document discovery-timeout cleanup
Address review feedback:
1. A custom tool_output.storage_subdir with a path separator (e.g.
cache/tool-results) silently no-oped the workspace-scanner exclusion:
os.walk yields one-segment dirnames, so a nested value never matched and
its files were counted as produced artifacts again. ToolOutputConfig now
validates storage_subdir as a single directory name (rejects separators,
.., absolute, empty) with tests, so the exclusion is always sound.
2. The discovery-timeout path now documents why cancellation is safe, mirroring
the session-init note: discovery runs inside the adapter's nested async
context managers, and stdio_client's finally terminates the process tree
(SIGTERM->SIGKILL on POSIX, process-tree on Windows), so a timed-out npx
subprocess and its children are reaped rather than accumulating.
* review: log session-init timeouts and align API response model default with runtime config
Address second-round review feedback:
1. A session-init timeout raised TimeoutError without any log, unlike the
discovery timeout which logs a WARNING. Wrap the bounded get_session in a
try/except that logs the timeout (server name + seconds) and re-raises, so
operators can diagnose tool-call failures caused by hung MCP sessions.
2. McpServerConfigResponse.session_init_timeout defaulted to None while
McpServerConfig defaults to 60s: a server created via PUT /api/mcp/config
without the field was persisted with null (no timeout) while the same
server created in the config file got 60s. Align the response-model default
to DEFAULT_MCP_SESSION_INIT_TIMEOUT so API-created and file-created servers
behave the same; an explicit null still opts out.
* review: narrow the discovery-timeout handler to the bounded wait_for path
The except TimeoutError clause covered both the bounded wait_for branch and
the bare discovery branch. With session_init_timeout opted out (None), a
TimeoutError raised by discovery itself would hit the %.1f format with None:
logging raises TypeError internally, the WARNING is silently dropped, and a
--- Logging error --- traceback goes to stderr.
Narrow the handler to wrap only the wait_for call, where the branch condition
guarantees the timeout value is not None. A discovery-internal TimeoutError on
the opted-out path now falls through to the generic failure handler and is
reported as 'tool discovery failed' with exc_info. Covered by a regression
test that asserts the skip is reported without any broken format.
`_count_diff_lines` skipped every line starting with "+++ "/"--- " to drop
difflib's two file headers. But a deleted hunk-body line whose content
begins with "-- " (e.g. a SQL comment `-- get users`) becomes the diff
line `--- get users`, and an added `++ ...` line becomes `+++ ...`, so
those were dropped too — undercounting the user-visible +N/-M change
summary (WorkspaceChangeSummary, per-file WorkspaceFileChange, and the
run-event string in recorder.py).
`difflib.unified_diff` always emits the two headers first, so skip them
by position (`lines[2:]`) instead. Hunk `@@` lines start with `@` and are
still ignored. Numeric/plain diffs are unaffected.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(browser): add agentic browser control
* fix(frontend): format browser view changes
* fix(browser): keep browser optional and isolate sidecar layout
* fix(browser): address PR review security and IME findings
- Nginx: add a browser-stream WebSocket location before the generic
/api/threads regex so Live upgrades instead of downgrading to HTTP
(both nginx.conf and nginx.local.conf).
- Ownership: require an existing owned thread for the WS stream and REST
navigate, and tear down the browser session on thread deletion so a
later caller cannot reuse a retained page/cookies by guessing the id.
- SSRF: enforce the URL policy at the browser request boundary via a
context-level route guard covering redirects, popups, iframes, and
subresources (skipped for CDP-attached Chrome).
- IME: skip key forwarding while a composition is active so confirming a
CJK candidate with Enter no longer submits the remote page form.
Adds regression tests for the request guard, session teardown on delete,
and the composing-Enter key decision.
* fix(frontend): smooth streaming in long tool threads
* Revert "fix(frontend): smooth streaming in long tool threads"
This reverts commit f0462516eabe77f138d4027ea1c714fb226683cf.
* fix(browser): address review security and lifecycle findings
- Reject cross-origin WebSocket upgrades on the live browser stream
(Origin allow-list reuse of CORS/same-origin helpers) to close a
WS-CSRF hole, and fail closed when the ownership store is absent.
- Warn when a CDP-attached session runs with the SSRF request guard
off, and drop the unreachable CDP screencast teardown dead code.
- Read browser session launch config from a single canonical source
(browser_navigate) so it is deterministic regardless of call order.
- Bound per-thread Chromium accumulation with idle-timeout eviction
and an LRU max-sessions cap.
- Reset the Live reconnect counter on a successful open so the stream
can't permanently stall after the cumulative attempt cap.
* fix(frontend): reduce long tool thread render stalls
Reuse stable historical message groups during streaming, defer heavy Markdown and browser previews, and lazy-decode message images.
* fix(browser): keep live control responsive during continuous input
Why: Manual browser control felt laggy — a physical click ran the remote
Playwright click three times and each non-move input synchronously awaited a
JPEG screenshot, so events queued behind capture (queue wait up to ~237ms).
The first async attempt used a trailing-edge debounce, which froze the visible
page until a wheel/keyboard gesture stopped ("scroll finishes, then it jumps").
What:
- Frontend forwards one `click` per physical click instead of also emitting
`down`/`up`, so the remote page is not clicked twice per gesture.
- Backend detaches live-frame capture from input dispatch: non-move actions
start a rate-limited background refresh loop (leading frame + bounded cadence)
that keeps emitting frames while input continues and never blocks dispatch.
- Add regression tests: input dispatch no longer awaits the screenshot, rapid
inputs coalesce, and continuous input keeps refreshing before it stops.
Scenarios: Verified in the live Browser panel — a single click completes in
~57ms (was blocked behind a 171ms capture), and a 1.14s sustained wheel gesture
renders ~7 frames throughout the scroll instead of one frame after it ends.
* fix(browser): harden worker and session lifecycle
* fix(browser): address latest review feedback
* fix(frontend): preserve optimistic new-chat message
* test(e2e): preserve mocked message run ids
* fix(browser): address capability review feedback
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(workspace-changes): classify a symlink replacing a file distinctly from deleted
scan_workspace_roots() skipped every symlinked path entirely
(host_file.is_symlink() -> continue), so the path was completely absent
from a snapshot instead of being recorded as a metadata-only stub the
way binary/large/sensitive-looking files already are. When an agent run
replaces a tracked file with a symlink (e.g. rm config.txt && ln -s
/some/other/path config.txt), the after-snapshot never contained that
path at all, so compare_snapshots()'s _status() saw after_file=None and
reported plain "deleted" -- silently hiding that the path is still alive
on disk, now as a symlink that can point anywhere on the host, including
outside the workspace root.
Add a "symlink" classification mirroring the existing binary/large/
sensitive pattern: scan_workspace_roots() now records a symlink as a
metadata-only FileSnapshot stub (symlink=True, symlink_target from
os.readlink(), lstat'd without ever following the link) instead of
omitting it. _status() reports "symlink_created" whenever a symlink
newly occupies a path that was not already a symlink (brand new or
replacing a prior file), so the security-relevant fact surfaces
distinctly instead of collapsing into "deleted". A symlink genuinely
removed with nothing replacing it is unchanged: still "deleted".
Verified against a real POSIX symlink (WSL; native Windows symlink
creation needs elevated privilege) driving the unmodified
scan_workspace_roots()/compare_snapshots() functions, and via a
patch-file revert/reapply cycle on this same fix to confirm the added
regression tests fail before and pass after.
* fix(workspace-changes): count symlink_created in the changed-file badge
getChangedFileCount summed only created + modified + deleted, so a run
whose only change was a symlink replacing a file (reported as the new
symlink_created status, not deleted) produced a count of 0 and
WorkspaceChangeBadge hid the badge entirely -- the exact scenario this
PR targets, with the opposite of the intended result.
Add symlink_created to the frontend WorkspaceChangeSummary interface
(already emitted by the backend) and include it in the count. The
per-file StatusIcon/statusLabel "modified" fallthrough is unaffected
and left as a follow-up, per review.
Regression test reverts cleanly to reproduce a count of 0 pre-fix.
* fix(workspace-changes): rank symlink_created in file sort and complete the type contract
sortWorkspaceChanges's statusRank had no entry for the new
symlink_created status, so statusRank[left.status] - statusRank[right.status]
evaluated to NaN for any comparison involving a symlink-created file,
violating Array#sort's ordering contract instead of producing a
deterministic order. The frontend WorkspaceChangeStatus and
DiffUnavailableReason unions, and the WorkspaceFileChange symlink
fields, also stayed narrower than what the backend now emits, so
TypeScript's satisfies Record<...> guard on statusRank could not catch
the gap.
Widen WorkspaceChangeStatus to include "symlink_created" and
DiffUnavailableReason to include "symlink", add the matching
symlink/symlink_target_before/symlink_target_after fields to
WorkspaceFileChange, and give symlink_created a rank alongside
modified in statusRank -- restoring the satisfies guard's ability to
catch a future unranked status. unavailableLabel now has an explicit
"symlink" branch (new symlinkUnavailable i18n string, en-US + zh-CN)
instead of falling through to the generic label.
Also fixes the failing e2e-tests CI check: the existing
workspace-changes.spec.ts mock summary predates the symlink_created
field, so getChangedFileCount computed 1 + 1 + 0 + undefined = NaN and
the badge rendered "Edited NaN files" instead of "Edited 2 files".
Added symlink_created: 0 to the mock to match the real backend
contract.
New sortWorkspaceChanges unit tests revert cleanly against the
unranked statusRank to reproduce the NaN-driven misordering. pnpm test
(626 tests), pnpm check, and pnpm format are clean, and the
previously-failing e2e spec plus the full e2e suite (94 tests) pass.
* feat: add workspace change review
* chore: format workspace change files
* fix: optimize workspace change summaries
* style: refine workspace change badge scale
* fix: restore workspace change user context import
* fix(frontend): gate workspace change badge to assistant messages
Only pass run_id to assistant MessageListItems so the workspace-change
badge can never render under a user's prompt, which carries the same
run_id. Assert single badge render in the E2E flow.
* fix(workspace-changes): address review feedback on diff parsing and badge
- Restrict unified-diff header detection to "+++ "/"--- " (trailing space)
in both backend _count_diff_lines and frontend getWorkspaceChangeLineClass
so content lines beginning with +++/--- are counted/styled correctly
- Gate WorkspaceChangeBadge to ai messages so tool messages folded into an
assistant group don't render a duplicate badge
- Add regression tests for both diff-classification fixes
- Apply prettier formatting to workspace-changes files flagged by CI
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
