[johoja12]

The floating log viewer repeatedly reconnects when its SSE stream ends with a startup error. Track whether connected has arrived and close the EventSource on any earlier server error or transport failure. Established streams retain their reconnect behavior.

This is limited to the reconnect fix. It adds no preview/approval button and no CLI-specific behavior.

Updated against master 1.50.0, preserving published history. Resolves all four items from the maintainer review:

• Removed the Codex-only branch and the Allow this file once button.
• Handles any pre-connected error, independent of wording.
• Browser regression covers all six reported startup errors, with no screenshot artifacts or unused remote fixture.
• Resolved the test-suite registration conflict; the browser glob is appended to the exclusion list.

Validation: six real-Chromium regression cases pass without page errors; all five frontend CLI policy tests pass. The normal pre-push static gate passed (typecheck, lint, format, lockfile, public assets, frontend syntax and browser exclusions). Production was untouched.

Fork tracking: johoja12#23.

[Ark0N]

Thanks for this, johoja12, and for the careful Chromium coverage. This PR stops the floating log viewer from reconnecting over and over after a tail-file permission denial, and adds an "Allow this file once" button that opens the file preview in Codex sessions.

The reconnect loop is a real bug: I ran your test against master's panels-ui.js and it fails with two tail requests where your change makes one. A few things need to change before this can merge:

1. src/web/public/panels-ui.js:6017: the mode === 'codex' check makes npm test fail. test/frontend-cli-no-id-branching.test.ts scans panels-ui.js and rejects new CLI-id branches (CLAUDE.md: no code outside stock.ts may branch on a CLI id). The denial, the reconnect loop and openFilePreview all behave the same in every mode, so please remove the condition instead of adding an allowlist entry.

2. The button covers a case that master no longer reaches. Since 7d46a9d, terminal-ui.js:2349-2357 sends any path outside the session's workingDir (and any ~/ path) straight to openFilePreview, which registers it through _registerExternalPreview. It never opens the log viewer (docs/architecture-invariants.md:410).

   • I checked this in Chromium with your fixture path, in both codex and claude sessions: one attachment POST, no tail-file request, no log viewer.
   • Your fork's terminal-ui.js routes text by extension instead, which is where issue [Bug] PTY exit handler does not clean up session listeners or Maps — dead sessions accumulate #23 comes from.
   • On master the button would only appear for paths that look like they are inside the workspace but get refused: a symlinked workspace, a symlink pointing out, or /ws/../x. In those cases openFilePreview takes the workspace route, not the external flow.
   • The test reaches the blocked state only by calling app.openLogViewerWindow(externalPath) directly (test/log-viewer-file-approval.browser.test.ts:83).

   Please drop the button from this PR and keep the reconnect fix. Whether the log viewer should offer a preview fallback at all is a separate design question, and it is still open.

3. src/web/public/panels-ui.js:6009: close on any error that arrives before connected, not on one string. The route ends the response after every createStream failure (file-routes.ts:2706-2709), so these five errors loop the same way:

   • File does not exist
   • File not found or not accessible
   • File too large
   • Maximum 5 concurrent streams per session
   • Failed to spawn tail process

   Track whether connected has arrived and close the EventSource on any earlier error. That covers all six cases and removes the dependency on the exact wording in file-stream-manager.ts:414.

4. Test cleanup.

   • Remove the /tmp/codeman-23-*.png screenshots (lines 99 and 110).
   • Remove the unused remote: { hostId: 'fixture' } on line 79. Every request is stubbed, so no remote path runs.
   • With the button gone, the test can check that reconnecting stops for the permission message and for one other pre-connect error, such as File does not exist.

Related but out of scope here: file-stream-manager.ts validatePath realpaths the file but not workingDir, so in a symlinked workspace every in-workspace tail is refused with this same message. validateSessionFilePath in route-helpers.ts already handles that case. If you want to take that on, it belongs in its own PR.

Once the gate is green and the PR is down to the reconnect fix, it is ready for another review.

[Ark0N]

Hi @johoja12, a quick check-in on this one, since #769 has been keeping you busy. The reconnect loop is a real bug and the fix is worth landing. From my review on 2026-10-26, what's still open on b3a56832:

1. Drop the mode === 'codex' check (it fails test/frontend-cli-no-id-branching.test.ts) and the Allow this file once button. Master routes out-of-workspace paths to the file preview and never opens the log viewer for them, so the button only covers paths it can't help with.
2. Close the EventSource on any error that arrives before connected, instead of matching the permission message. That also stops the loop for File does not exist, File too large, the stream limit and a failed tail spawn.
3. Test cleanup: no /tmp/codeman-23-*.png screenshots, no unused remote fixture, and a check that reconnecting stops for the permission message plus one other pre-connect error.
4. A rebase: it now conflicts in config/test-suites.ts.

If you'd rather I cut it down to the reconnect fix myself, say so and I'll do it on top of your commit.

[johoja12]

The floating log viewer repeatedly reconnects when its SSE stream ends with a startup error. Track whether connected has arrived and close the EventSource on any earlier server error or transport failure. Established streams retain their reconnect behavior.

This is limited to the reconnect fix. It adds no preview/approval button and no CLI-specific behavior.

Updated against master 1.50.0, preserving published history. Resolves all four items from the maintainer review:

• Removed the Codex-only branch and the Allow this file once button.
• Handles any pre-connected error, independent of wording.
• Browser regression covers all six reported startup errors, with no screenshot artifacts or unused remote fixture.
• Resolved the test-suite registration conflict; the browser glob is appended to the exclusion list.

Validation: six real-Chromium regression cases pass without page errors; all five frontend CLI policy tests pass. The normal pre-push static gate passed (typecheck, lint, format, lockfile, public assets, frontend syntax and browser exclusions). Production was untouched.

Fork tracking: johoja12#23.

[johoja12]

GitHub CI is fully green for 6955a64: Typecheck & Lint and the complete Unit & integration suite passed. The six real-Chromium startup-error cases also pass locally without page errors. All requested scope/code/test changes are included; this is ready for another review.

[Ark0N]

Thanks for sticking with this one, johoja12. This PR stops the floating log viewer from reconnecting every few seconds when its tail-file stream ends with a startup error: it closes the EventSource on any error that arrives before connected.

The core fix is right. I ran your browser test against master's panels-ui.js and it fails all six cases with two requests. On your branch it passes with one. CI is green.

One line needs to go: if (!connected) eventSource.close(); in onerror at src/web/public/panels-ui.js:6144. It stops the browser's normal retry for any transport failure before connected. In Chromium, with only the first tail-file request aborted and a good stream after it, master retried and showed the log. This branch made one request and left the window on "Connecting to ..." with status "connection error". Clicking the path again only focuses that window (openLogViewerWindow, lines 6060-6064), so the user has to close it first. The bug doesn't need this line. Every createStream failure is a 200 stream whose error message arrives before the stream ends, which your close at line 6136 already handles, and a non-200 response never reconnects. With line 6144 removed, all six of your test cases still pass.

Two small things in the test:

• test/log-viewer-reconnect.browser.test.ts:54 checks for an "Allow this file once" button that no code renders any more, so it can never fail. It can go.
• The 3.5 s waits are only 0.5 s above Chromium's default 3 s reconnect delay. If you put retry: 200\n before data: in the stubbed body, a reconnect shows up within 200 ms, so a shorter wait gives a stronger check.

I will make these changes when I merge, and I will squash the PR so the first commit with the Codex branch and the button does not land on master. Thanks again for covering all six errors.