Repository navigation
fix: steadier git polling, agent stops and remote reconnects, plus two security fixes - #681
Conversation
Codemux polls git status and diff stats every few seconds. Those reads refreshed the index as a side effect, holding .git/index.lock, so an agent's concurrent `git add` or `git commit` could fail with "Unable to create '.git/index.lock': File exists". Git now runs with GIT_OPTIONAL_LOCKS=0, and the per-file and total line counts come from `git diff-files` and `git diff-index`, which never write the index (porcelain `git diff` refreshes it regardless of that variable). Renames are still detected and an unborn branch still compares against the empty tree, so the numbers match what porcelain reported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The workspace sweep read diff stats and then called git_status only to count its entries, which ran both numstat diffs a second time. It now counts the `git status` entries alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The minute-by-minute fetch wrote FETCH_HEAD, which git does without a lock, so it could clobber a concurrent `git pull`; it could also start auto gc on every tick. It now passes --no-write-fetch-head and --no-auto-gc. A fetch that hits the 10s limit was killed outright, which can leave ref lock files behind and break the next fetch or push. On Unix it now gets SIGTERM and a second to clean up before the kill. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`git worktree add` writes the index together with the files, so their stat data can't be trusted and each status read re-hashes them. Status reads no longer refresh the index in passing, so every worktree Codemux creates (desktop, remote host and automations) now runs `git update-index -q --refresh` once, best effort. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The diff pane rendered every line of `git diff -- <file>` and re-read it
every 5 seconds, so a regenerated lockfile or a minified bundle froze
the UI. A file's diff (working tree, staged, or against the base branch)
now stops at 5,000 lines or 512 KiB, on a line boundary; git is stopped
once the limit is reached. The commands return { diff, truncated } and
the pane says "Diff too large — showing the first N lines".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A wedged CLI never answers the interrupt control request, so Stop hung until the host RPC timed out, the turn kept running, and later steers were refused. Wait at most 10s for the acknowledgement, then end the CLI process the same way a Stop with unread guidance does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…running Pending requests were only failed once the process exited, so a child that closed stdout and lingered left callers waiting for their own timeout (24h for an ACP prompt). After stdout EOF, fail them if the exit is not observed within a second; a prompt exit still gets the watchdog's exit code and stderr. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ACP reports a cancelled tool call with status "cancelled", which both ACP adapters treated as still running, and plan entries with that status showed as pending. End cancelled tool calls as errors and settle cancelled plan steps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex can announce a retry and then go silent, leaving the turn running and the queue blocked forever. After 10 minutes with no progress on the parent thread, ask Codex to interrupt the turn and settle it as failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A repo's SVG or HTML file served from /api/assets ran as a live document on the Codemux origin, so embedded script could use the session cookie to mint a WebSocket ticket. Every asset now carries nosniff, and HTML/XML/SVG responses get a sandboxing Content-Security-Policy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The origin URL was uploaded to the cloud, synced to every device and shown as-is, leaking tokens embedded in remotes like https://oauth2:TOKEN@gitlab.com/... Remote URLs are now stored without http(s) userinfo, query or fragment, and without any ssh password. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Repos cloned through an ~/.ssh/config alias such as github-work were queried as host github-work, so gh found no login and PR features reported not authenticated. Dotless GitHub aliases and github.com-<name> hosts on SSH remotes now resolve to github.com; dotted Enterprise hosts are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A run queued behind a concurrency group or waiting on an environment has no checks yet, so a watch could tell the agent CI passed and let it merge early. When the checks otherwise look green, the watch now reads the head's workflow runs and stays pending while any is unfinished; a failed read is logged and falls back to the checks alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Five quick failures (about 30s offline, since ssh fails instantly with no network) used to stop the tunnel supervisor for good, leaving the workspace dead until it was pushed again. The supervisor now keeps publishing CircuitOpen but retries every 60s, and returns to Connected with fresh counters once the host is reachable. The sidebar pill reads "Connection lost — retrying" to match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The supervisor treats "local socket exists" as ready, but a dead ssh can leave its -L socket file behind. Every retry then reported Connected within ~50ms, reset the backoff, and flickered the status. Remove the leftover file before each ssh spawn so readiness reflects a fresh bind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Quitting or updating left one `ssh -L ... exec sleep` per remote workspace running, plus its sshd session, because the exit handler never touched the tunnel registry and ssh was not killed on drop. Spawn ssh with kill_on_drop, add a registry-wide shutdown that the exit handler runs with a 3s bound, and have a supervisor's shutdown also kill an ssh that is still connecting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…wake A ticket request or socket open sent into a dead network had no deadline, and wake events ignored an attempt already in flight, so a phone that switched networks mid-reconnect could sit on "reconnecting" for minutes. The ticket request now gives up after 8s and the socket open after 15s, falling back into the normal backoff, and an online/visible event restarts an attempt that has been in flight for 3s or more. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The panel subscribed to the active thread's messages, which are replaced on every streamed batch, so the whole deck and its open pane redrew about once per frame while a reply streamed. It now selects only the subagent counts (recomputed when the messages change, compared shallowly), the Subagents pane subscribes to the messages itself, and the tab strip gets stable select/reorder callbacks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The transcript reserves a scrollbar gutter on both edges, and a press on the empty left one also targets the scroller, so it was mistaken for a scrollbar drag and stopped following a streaming reply. Only a press within the scrollbar's reach of the right edge now releases follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The unacknowledged-Stop fallback retired whichever query was current when its timer fired, so a query that replaced the stopped one (a stale-resume rebuild, or a send that crossed the Stop) could be ended and its turn lost. Each Stop now acts only on the query it started with, concurrent Stops share one operation, and a send waits for an in-flight Stop to settle before it is queued. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The capped diff reader left stderr in a pipe it only read after stdout closed. An external diff or textconv driver that writes more than a pipe's worth to stderr blocks before closing stdout, so the read waited forever. Stderr is now drained on a thread, keeping its first 16 KiB for the error message, and git is killed and reaped on every path that leaves output unread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rant Abandoning a stalled ticket request lets connection attempts overlap, but the iroh grant cache took whichever mint answered last and its socket read the cache after dialing. A slow, abandoned mint could land during a newer dial and swap in a stale grant, which the desktop rejects fatally, sending the user back to sign-in. The ticket request's cancellation now reaches the grant request, only the newest un-cancelled mint updates the cache, and each socket keeps the grant it was created with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llation The transport now abandons and redials a socket that doesn't open within its deadline, so hosted startup can dial more than once before its own 30s timeout; the tests now check that every dialed stream is closed and that nothing dials after the timeout. The grant request also receives the ticket request's cancel signal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Alias inference ran before the user's explicit host mappings, so a github-work alias mapped to another provider was still treated as github.com. It also rewrote real dotted hosts such as github.com-work.corp.example, pinning their operations to github.com. Mappings are now checked against the original host first, and only dotless github.com-<label> suffixes count as aliases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… green A timeout or server error while a run was queued let the head read as green, waking the agent, refilling its budget and marking the head told so it was never checked again. A transient failure now holds green back until a later pass reads the runs; access refusals and unsupported responses still fall back to the checks alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A provider that announces a retry every few minutes without progress pushed the deadline out each time, so the turn never failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On Node 22 a Response body read takes more turns than the microtask flush allowed, so two deadline tests saw no socket yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mario-reviewer
left a comment
There was a problem hiding this comment.
Review of 03bc1d6ec089887994a3df4bb0635925a97e8241
1. Preserve real enterprise hosts when resolving SSH aliases
- File:Line:
src-tauri/src/git_provider/detect.rs:287–292 - Severity: Warning
- What's wrong:
let dotless_alias = !host.contains('.') && host.contains("github");treats an actual internal hostname such asgithuborgithub-enterpriseas a public-GitHub SSH alias, silently changing the API destination. - Impact: With
git@github:team/repo.gitpointing to an enterprise instance through corporate DNS or SSH configuration, and no Codemux custom-host override, the base revision preservesgithub; this revision rewrites it togithub.com.github_budget::remote_repositoryshares this resolver, so PR operations lose the enterprise login and targetgithub.com/team/repoinstead, potentially selecting an unrelated repository if that name exists publicly. - Fix: Resolve the effective SSH
HostNameor require an explicit alias-to-host mapping before changing the destination. Keep the original host when its public-GitHub destination has not been established. Add coverage for a dotless enterprise hostname without a custom-host override.
2. Check every workflow-run page before declaring the head green
- File:Line:
src-tauri/src/pr_watch.rs:769–774 - Severity: Warning
- What's wrong:
"repos/{}/actions/runs?head_sha={}&per_page=100"reads only the first page, so an unfinished run on a later page does not hold back green. - Impact: When the same SHA has more than 100 workflow runs and an older run remains queued behind concurrency, the first page can contain only completed runs while that queued run still has no jobs in
statusCheckRollup.parse_runs_pendingignorestotal_count, returns false, andevaluatecan send the green wake, refund attempts, and recordgreen:<sha>before the queued workflow finishes. - Fix: Page through the workflow-run envelope until all runs are checked, stopping early on an unfinished run; alternatively, query unfinished statuses directly. Keep green unconfirmed if the read is incomplete. Add a regression case with a completed first page and an unfinished run on the next page.
Validation: Full first-pass static review of the PR diff and exact-head repository context. No repository code was built, installed, or executed. No prior review findings were present.
CI, reported separately: changes and web-checks (ubuntu-latest) passed. web-checks (windows-latest), rust (ubuntu-latest), and rust (windows-latest) were in progress. iroh-wasm was skipped. Pending CI is not a code finding.
Verdict: REQUEST_CHANGES
🤖 Automated review by Mario (AI). Findings may be wrong; a human makes the final call.
A dotless host like `github` can be a real enterprise host reached through corporate DNS. Ask `ssh -G` for the alias's HostName (cached for five minutes) and keep the original host when ssh names none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A head with more than 100 runs could hide a queued run on a later page. Read pages until one shows an unfinished run or all are read; past ten pages green stays unconfirmed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. Both findings are valid and both are fixed. 1. Enterprise hosts and SSH aliases (153dcfd)
2. Workflow-run paging (d13c4a0)
Verification:
|
mario-reviewer
left a comment
There was a problem hiding this comment.
Re-review of d13c4a000da90d0b4cc2e9cc11c8507aaa7b1652
No new blocking findings. Both findings from the previous review are resolved:
- SSH alias resolution:
src-tauri/src/git_provider/detect.rs:203–218,265–294now consultsssh -Grather than assuming a dotless GitHub-like hostname meansgithub.com. Unresolved or unchanged hostnames remain intact, and explicit custom-host mappings still take precedence. Regression coverage includes dotless enterprise hosts and an alias targeting an enterprise instance. - Workflow-run pagination:
src-tauri/src/pr_watch.rs:230–255,801–809now requests successive pages, stops on an unfinished run, and leaves green unconfirmed when the page limit is reached. Regression coverage includes a queued run on page two, completed pagination, a later-page timeout, and the page limit.
Validation: Reviewed the two commits since 03bc1d6ec089887994a3df4bb0635925a97e8241, their exact-head repository context, and both prior findings. Static review only; no repository code was built, installed, or executed. Regression tests were inspected, not run.
CI, reported separately: changes passed. web-checks (ubuntu-latest), web-checks (windows-latest), rust (ubuntu-latest), and rust (windows-latest) are in progress. iroh-wasm was skipped. Pending CI is not a code defect.
Verdict: APPROVE
🤖 Automated review by Mario (AI). Findings may be wrong; a human makes the final call.
Rust 1.100 builds unit-test executables inside the build directory instead of target/<profile>/deps, so looking two levels up from the test binary no longer finds fake_cursor_acp and the other fixtures. Look in every ancestor directory instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rust 1.100 no longer writes test executables to target/debug/deps, so the Common Controls manifest step failed on a missing directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The Rust CI failures on d13c4a0 were not caused by this branch. CI picked up Rust 1.100.0 (released 2026-11-09); the last Rust run on
|
mario-reviewer
left a comment
There was a problem hiding this comment.
Re-review of d8f94356154f58ec785934344e3ecb67de7528b3
No new blocking findings. The two commits since the previous review look fine:
src-tauri/src/test_isolation.rs:10–16searches executable ancestors for fixture binaries, preserves the missing-fixture fallback, and is used by the updated unit tests. Existing explicit fixture overrides remain intact. The helper is in a test-only module..github/workflows/ci.yml:548–551recursively places Common Controls manifests beside Windows executables after test compilation and before test execution.
Both earlier findings remain resolved: SSH aliases are resolved through SSH configuration without guessing that enterprise hostnames mean public GitHub, and workflow-run checks paginate before confirming green. No unresolved prior blocking findings remain.
Validation: Inspected gh pr diff, both commits since d13c4a000da90d0b4cc2e9cc11c8507aaa7b1652, exact-head source and workflow context, and the prior reviews. Static review only. No repository code was built, installed, or executed; tests were inspected, not run.
CI, reported separately: changes and iroh-wasm passed. web-checks (ubuntu-latest), web-checks (windows-latest), rust (ubuntu-latest), and rust (windows-latest) are in progress. Pending CI is not a code defect.
Verdict: APPROVE
🤖 Automated review by Mario (AI). Findings may be wrong; a human makes the final call.
Problem
Several sources of instability, slowness or security exposure were found while comparing Codemux against known failure modes in agent-driven coding environments:
.git/index.lock. An agent's owngit add/git commitcould fail withindex.lock: File exists. The 60 s background fetch could also break a concurrentgit pull(unlockedFETCH_HEAD) and start auto-gc.cancelledtool calls kept spinning.sshprocesses outlived the app./api/assetsran as a same-origin page, able to mint a WebSocket ticket from the session cookie.originURLs with embedded tokens to the cloud API.git@github-work:…) lost all PR features.Solution
One focused commit per fix:
Git
GIT_OPTIONAL_LOCKS=0on every git command Codemux runs. Status line counts now come from plumbing (diff-files,diff-index --cached), which never writes the index.--no-write-fetch-head --no-auto-gc. On timeout it sends SIGTERM before kill, so git can remove its ref locks.update-index --refresh.Agent runtimes
cancelledsettles tool calls (as errors) and plan steps.Remote
sshgetskill_on_drop, and tunnels are shut down on GUI exit and in headlessserveteardown.Security
/api/assetsalways sendsX-Content-Type-Options: nosniff. HTML, SVG and XML also getContent-Security-Policy: default-src 'none'; style-src 'unsafe-inline'; sandbox. Inline<img>rendering is unaffected.Integrations and UI
github.com-<label>aliases, resolve togithub.com. Explicit host mappings still win, and real dotted hosts (GitHub Enterprise) are untouched.actions/runs?head_sha=call. Queued, waiting or in-progress runs keep it pending. A transient failure of that call holds green back and retries next pass; a permission or "not found" failure falls back to checks only.Screenshots
The diff pane opening a 20,000-line lockfile-style diff (dev mock):
Verification
cargo check: clean.cargo test --no-runcompiles every test target.Focused
cargo test --libruns, all passing:git::tunnel_supervisorjobs::pr_watchjson_rpc_childweb_remote::assetsproject_identitygit_provider/_provider::github_budgetagent_provider::codexagent_provider::custom_acpThe only failures were tests that need prebuilt fixture binaries (
fake_cursor_acp,fake_claude_sidecar); CI builds those.cargo test --features test-fixtures --test integration git_operations: 65 passed.Sidecar:
bun run typecheckclean,bun run test245 passed.npm run check: clean. Affected vitest files: 469 passed (src/remote,src/environments, right panel,MessageList, diff).Each new regression test was confirmed to fail against the old code: transport stalls, right-panel redraw, left-gutter press, stale socket, the stalled Claude Stop, and the repeated-retry stall fixture.
Independent code review over three rounds: 8 findings in total, all fixed; final verdict approve.
Caveats
cfg(unix), and CI covers Windows.--no-write-fetch-headneeds git ≥ 2.29, which Codemux already relies on elsewhere.Implemented by Claude Opus 5.5 via Claude Code; reviewed by GPT 6.1 Sol (extra-high reasoning) via Codex.
🤖 Generated with Claude Code