Repository navigation
Browser: the hub alone ends a remote tab, kept by holder files and working agents (F47 step 1) - #316
immakermatty wants to merge 1 commit into
Conversation
8c96c8c to
0d0cb87
Compare
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty — exact-head QA of 0d0cb87bbd2fddaea09aafa4b04f382064a2fbb2 (F47 step 1). CHANGES_REQUESTED.
-
[P1] Fail closed when reading holders/bindings fails (
src/browser/people/holds.ts:261-266,278-300;hub.ts:1448-1476).readHoldsturns everyreaddirerror into an empty directory and individualreadFileerrors into missing holds. Thus the hub's promised final re-read succeeds with zero holds on EACCES/I/O failure, runs the 8-second release grace, and sendsTarget.closeTargetfor a tab actually held by the Launchpad/T3 or a live agent. Reproduced on this SHA: a holder initially listed the target; afterchmod 000the directory returned EACCES,readHoldsreturned zero holders, and a BrowserHub fixture closed the held target (initial held=1, closed=0->after EACCES held=0, closed=[target]). Only true ENOENT/missing directories should mean empty; other read errors must propagate or preserve the tab, including the final pre-close read. Add a regression test with an unreadable directory / holder file and assert no close until a successful fresh read establishes release. -
[P2] Disconnected close/restore contradict the F47 failure contract (
src/browser/people/service.ts:322-333,hub.ts:425-460;docs/decisions.md:7151). GET tabs returns 503 while disconnected, but if a viewer keeps a tab in memory after connection loss, POST close returns 202 and POST restore returns 200 instead of the specified 503. Reproduced with a retained viewed tab:connected=false, close returned a deadline, restore returned true. Gate both mutations on hub readiness (without losing closes already accepted before the disconnect), and test this path.
Focused verification on the immutable checkout: 55 pass, 0 fail across environment-browser-holds and environment-browser-view; git diff --check clean. The passing tests do not cover the two failure paths above. No merge or push performed.
0d0cb87 to
91125f8
Compare
|
Both fixed in 91125f8; re-review requested on that head.
|
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty — exact-head re-review of 91125f8 (F47 step 1): CHANGES_REQUESTED. The prior unreadable-holder and disconnected close/restore findings are addressed; the two focused test files pass (58 pass, 0 fail), and git diff --check is clean.
[P1] An unclear .pid still releases a working agent tab (src/browser/people/holds.ts:327-330). Number(pidText.trim()) converts an empty or whitespace-only file to 0, and "0"/"-1" are accepted as integer PIDs; processAlive returns false for them. In an isolated fixture with a valid codex-1.target binding, readHolds returned bound=["codex-1"], working=[] for each of "", " ", "0", and "-1", whereas "not a pid" correctly returned a working session. The hub therefore starts the release grace and its final read can close this agent tab despite an indeterminate PID file. Treat empty/invalid/nonpositive PID text as unclear/working, only a well-formed positive PID proven dead as released; add regression cases for empty/whitespace/0/negative values (including the no-close hub path).
The live CI on this head is also red: tests (2/2) fails in tests/tools-team-github.test.ts (profile update expected 200/updated but returned 400/operation-failed); check (ubuntu-24.04) is consequently failed. It may be unrelated to F47, but must be resolved or rerun green before merge. No merge or push performed.
91125f8 to
b840390
Compare
|
Fixed in b840390; re-review requested on that head. New tests:
About the red CI on 91125f8: |
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty — exact-head re-review of b840390 (F47 step 1): CHANGES_REQUESTED for the required CI gate only. The previous P1 is fixed: malformed/empty .pid remains working, and the real-reader hub test covers the no-close path. In an isolated checkout the two focused test files pass (60 pass, 0 fail); git diff --check is clean. The base remains main 342db2f, with no unresolved review threads.
The live tests (2/2) check failed again on this exact head, in the unchanged tests/tools-team-github.test.ts case "a gh login started on a Work Environment stops when a profile change makes it a Team one": /api/update expected 200/updated but got 400/operation-failed (recoveryMayBeRequired: true). The required check (ubuntu-24.04) therefore failed too. I do not attribute this failure to F47, but cannot approve a red required CI gate. Please capture the log in the issue you proposed, investigate/fix or rerun to green on this exact head, then request another exact-head review. No merge or push performed.
… failing busy (#272) (#317) Since #270 the shell setup cache starts a background read right after a profile change, under the Folder's operation lock. A person's next change, or a tools change after a sign-in, then found the lock held and answered 400 operation-failed with recoveryMayBeRequired, although it would have succeeded a moment later. The same race failed CI often enough on 2026-10-11 to block merges (main's rerun and PR #316, three times). withFolderOperationLock now waits, at most 3 s, while the holder is this process's own short read (withFolderReadLock counts its holds per state directory until the lock is released). Another process's holder, or this process's own mutation, is still refused at once, so the kernel lock stays exclusive and a mutation from elsewhere keeps the immediate refusal. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rking agents (F47 step 1) Decision F47 points 4-6 (root decision 0195 points 9-12). The view's hub now decides for every page target, not only the tabs it opened: - held while a holder file in $XDG_RUNTIME_DIR/lazurio-browser/holders lists it or an agent-browser session bound to it has a live daemon (a binding alone no longer counts: agent-browser keeps it past the daemon); - unheld and unviewed tabs end after 8 s when their last hold went (Vrátit), 30 s after their last viewer, 10 minutes when nothing ever viewed or held them; the holds are read again before a tab is closed, and a failed read keeps it; - one blank window of the hub's own stays, because Chrome ends with its last window and the unit restarts it. The hub writes the ids of the live tabs to $XDG_RUNTIME_DIR/lazurio-browser/ tabs.json from a fresh Target.getTargets snapshot, stamped with the boot-clock time it began (boot id and /proc/uptime, never the wall clock), after each discovery, on change and every 30 s while connected, one at a time. keptTabs is the apps' reconciliation rule: an id goes only when a list of the current boot taken after its record lacks it. The hub answers GET/POST /.lazurio/tabs and POST /.lazurio/tabs/<id>/close|restore on loopback only, refusing them through the gateway and to any request with Origin or Sec-Fetch-Site. The view page tells its app lazurio-browser:closed when its tab ends. This changes F39 point 9: an agent's window ends once no agent works in it and nothing holds or views it, and after a restart unheld tabs get 10 minutes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b840390 to
a2cd44d
Compare
|
Rebased onto main 1cd5818, which has #317 (the fix for #272, the profile-update race that failed this PR's CI 3 times and a rerun of main). New head a2cd44d; the step-1 code is unchanged since your review of b840390. Only lock.ts and its tests come from main underneath. CI on a2cd44d is green. The first attempt of |
agentrozjedemeai
left a comment
There was a problem hiding this comment.
@immakermatty — APPROVED on exact head a2cd44d (base 1cd5818). The F47 step-1 patch is unchanged from my b840390 review (matching stable patch-id and identical source/tests); the only delta brings in #317 from main, addressing the earlier unrelated race. Isolated checkout: 60 focused tests passed, 0 failed (bun --smol, max concurrency 1), and git diff --check is clean. The exact-head Platform checks rerun is green, including static checks, both test shards and required Ubuntu aggregate; main CI on 1cd5818 is green on Linux and macOS. The first run had one 60 s timeout in shell-artifact under four workers; the rerun passed. No unresolved review threads or pending competing requests. No merge performed.
|
Thanks, approved on a2cd44d. I'm not merging this yet, on purpose. Matěj decided on 2026-10-11 to release step 1 together with T3 Code's task 501 (HumanAndMachine-ai/mission-control-data#329). Until T3 writes its threads' holder files, a T3 thread's window would end about an hour after its agent's last command whenever nobody views it. Platform release candidates are cut from |
Status: ready for review. The F47 design (#314) is merged as 342db2f; this PR is step 1 on top of
main.Why
F47 step 1 (plan DEV-6656 task 917). Root decision 0195 point 11 says a remote tab ends when the last place holding it lets go and no agent works in it, after a short grace with Vrátit. A close in Přehled or in the Launchpad's strip ends it for everyone. Today the view's hub only closes tabs it opened itself (F39 point 9):
What changes
Holds (
src/browser/people/holds.ts). The hub reads:$XDG_RUNTIME_DIR/lazurio-browser/holders/<strip>.json. The file name must matchstrip, only target ids count, and size limits apply;<session>.pidnames a live process). A binding alone no longer counts, because agent-browser keeps it past the daemon on purpose.Both directories are watched and re-read every 30 s. Reads fail closed (Pablo's review of 0d0cb87). Only a directory or file that does not exist reads as holding nothing. Any other failure (permission, I/O), and a holder or binding file that is not one, throws, so the hub keeps its last reading and no tab ends. A session counts as ended only when its
.pidis gone or names a well-formed process id (^[0-9]+$, ≥ 2) that no longer runs; an empty, blank, zero, signed or otherwise unclear.pidcounts as working.The hub decides for every page target (
hub.ts):The hub's blank window. The unit restarts Chrome when its last window closes. So the hub keeps Chrome's first
about:blankwindow (or opens one), lists it nowhere and never closes it. When someone navigates it, it becomes a normal tab and the hub opens a new blank one.The list of live tabs (Pablo's findings on Docs: F47 designs Nová karta, the holders of a remote tab and the Environment's overview #314). The hub writes
$XDG_RUNTIME_DIR/lazurio-browser/tabs.json:Target.getTargetssnapshot, one at a time;boot_id+/proc/uptime), never the wall clock;keptTabs(stored, list, boot)is the apps' reconciliation rule (steps 2 and 4). An id goes only when a list of the current boot, taken after its record, lacks it. A record from an earlier boot, or one without a time, is older than every list of this boot, and a list of another boot is ignored.readBootClockgives the time.Loopback requests for the Launchpad (
service.ts):GET /.lazurio/tabs(facts per tab: opener, holders, working agents, grace, closing),POST /.lazurio/tabs {url, strip},POST /.lazurio/tabs/<id>/close(everyone, 8 s) and.../restore. Through the gateway (the view's own host) they answer 404. Any request withOriginorSec-Fetch-Sitegets 403, because pages in the Environment's own browser reach the loopback too. While the hub does not know the browser's targets, close and restore answer 503; a close accepted before goes on after the reconnect.The view page posts
lazurio-browser:closedto its app when its tab ends.lazurio browser servecreates the holder directory (0700), watches it and agent-browser's socket directory, and writes the list.F47 gets a "Step 1, implemented" note.
What does not change
tab_gonerecovery already covers a window that ended.Behaviour change to know before release
Until T3 Code writes its holder files (step 4), a T3 Code thread's window stays while its agent works in it or someone views it. Otherwise it ends about an hour after the agent's last command (agent-browser's idle timeout + 8 s). The agent's next command then answers
tab_gone, andlazurio browser windowopens a new one. Before, such windows never closed. I'd release this together with T3 Code's step 4 rather than alone.Verification
bun run checkon this head: 1937 pass, 7 skip, 0 fail, static checks included (biome, tsc, check:public, build/smoke proofs). The watcher test writes until the watch sees a change, because on macOS a watch can miss the instant it starts; the hub's 30 s read covers that in production.classifyRequest);targetCreatedhas not reached the hub is in the list (snapshot);chmod 000, the real reader) lets no tab go until a successful read shows the release;.pidkeeps its tab past every grace (the real reader); a.pidof an ended process then lets it go;tests/environment-browser-holds.test.ts):.pid:""," ","0","-1","1","+5","4.2", an 11-digit id and"x"count as working; a live well-formed id is working, an ended one or a missing file is not;processAlive;readBootClockandparseBootTime;keptTabs: a later list of this boot drops, a later record keeps, an earlier boot's record goes, another boot's list is ignored, the ended-before-record case, and a wall-clock step.Risks
unknown, oragentby their binding.🤖 Generated with Claude Code