feat: editor pane stat-polling auto-sync#259
Merged
mattleaverton merged 58 commits intomainfrom Mar 31, 2026
Merged
Conversation
…hints When called with unrecognized params (e.g. new-tab with url), the MCP tool now returns an error listing valid parameters. A specific hint suggests open-browser when url is passed to new-tab. Help text updated to clarify the distinction between new-tab and open-browser, with a new playbook for opening URLs.
- Remove localStorage persistence for tool strip expanded state - ToolStrip now uses local useState initialized from showTools prop - ToolBlocks inherit initial expanded state from showTools - Remove autoExpandAbove/completedToolOffset props (no longer needed) - All toggle state is session-only, resets on page refresh
Add implementation plan for making terminal URLs clickable (opening in browser panes) with right-click context menu support for open in pane, open in tab, open in browser, and copy URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewed all source files referenced by the plan against the actual codebase and corrected several issues: - Fix ILinkHandler hover/leave signatures (need range param for OSC 8) - Add explicit wrapperRef JSX attachment instructions for outer div - Fix test file reference to canonical path (components/context-menu/) - Add useMemo dependency array update note for ContextMenuProvider - Clarify link provider registration order for priority - Note cleanup needs for both hoveredUrl map and data attribute - Add context-menu-utils.test.ts to new files list - Document picker vs direct browser pane design decision Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Corrects three factual errors in the implementation plan: 1. xterm.js link provider priority is first-registered=highest (not last) 2. menu-defs.test.ts does not exist yet (was incorrectly labeled "update existing") 3. Removes context-menu-constants.ts from modified files list (no changes needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
58 new tests across 8 files plus updates to 2 existing test files, aligned to the implementation plan's TDD phases. Covers hover state tracking, URL detection, left-click behavior, context menu integration, multi-pane integration, and browser-use E2E smoke testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 1 of clickable terminal URLs: module-level map for tracking hovered URLs per pane, and findUrls utility for detecting http/https URLs in terminal output text. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…link provider - Left-click on OSC 8 links and detected URLs now opens a browser pane (split right) instead of window.open - Warning modal confirm also opens browser pane instead of window.open - Add hover/leave callbacks to OSC 8 linkHandler for tracking hovered URL - Register URL link provider (after file path provider) to detect plain http/https URLs in terminal output - Track hovered URL in module-level map and data-hovered-url DOM attribute - Clear hover state on terminal dispose and tab hide - Update existing link warning and keyboard tests for new behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add hoveredUrl optional field to terminal ContextTarget - Parse hoveredUrl from dataset in parseContextTarget - Add URL-specific menu items (Open in pane/tab/browser, Copy URL) to terminal context menu when hovering a URL - Add openUrlInPane, openUrlInTab, openUrlInBrowser, copyUrl actions to ContextMenuProvider - Add tests for context-menu-utils and menu-defs URL behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- terminal-url-link-click: verifies URL click in nested pane opens browser pane on the correct branch (both plain URL and OSC 8 paths) - terminal-url-context-menu: verifies URL context menu items appear when hovering a URL, are absent without hover, and "Open URL in pane" creates a browser pane split Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verifies that when a terminal tab becomes hidden, the hovered URL module state and DOM attribute are cleared. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
With the new URL link provider registered after the file path provider, the existing test mock was capturing the URL provider instead of the file path provider. Fixed by only storing the first registered provider. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove unnecessary escape in url-utils regex character class - Capture wrapperRef.current in local variable before cleanup to satisfy react-hooks/exhaustive-deps rule Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ks, docs update - URL detection now preserves balanced parentheses (e.g. Wikipedia URLs) while still stripping unbalanced trailing parens - OSC 8 linkHandler.activate validates http/https scheme before opening browser panes; non-http schemes fall back to window.open - Added clickable URLs feature to docs/index.html feature list Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevents non-left-click from triggering link activation on OSC 8 links, file path links, and URL links. Adds tests for right-click and middle-click. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align .opencode/.gitignore with upstream best practices: add plans/, package-lock.json, and .freshell-mcp-state.json. Remove the transient MCP state file from tracking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes: stale auto-save timer after Reload (critical), Keep Mine corrupting clean/dirty state, native file-picker panes excluded from polling, existing tests updated for stat polling, removed non-verifying mtime tracking test.
F1: fetch mock shape - all mock responses must include .text() method F2: Silent-reload race guard - snapshot before async read, re-check after F3: Keep Mine cancels auto-save timer, does NOT update lastSavedContent F4: Reload cancels auto-save timer
The fresheyes review correctly identified that the pending auto-save timer callback could race with conflict resolution because Cancel the timer BEFORE updating refs to ensure the stale timer cannot't fire after refs are updated.
FE1: Cancel auto-save timer on conflict detection FE2: Re-check buffer cleanliness after async read FE3: All test mocks must include .text() FE4: Seed lastKnownMtime with 'initial' sentinel FE5: handleKeepLocal must not update lastSavedContent FE6: Re-trigger auto-save after Keep Mine FE7: New tests must share test harness FE8: Add native file-picker exclusion test
Returns { exists, size, modifiedAt } without reading file content.
Uses existing validatePath middleware for sandbox enforcement.
Used by the editor pane for change detection via stat-polling.
Update fetch mock to return stable stat responses and filter assertions by URL so upcoming stat-polling does not break existing auto-save test expectations.
- server: add GET /api/files/stat endpoint for lightweight file metadata - client: polls stat every 3s for open file panes - excludes native file-picker panes from polling - silent reload if buffer clean, conflict banner if dirty - FE1: cancel auto-save before setting conflictState - FE2: re-check cleanliness after async read - FE3: all mocks include .text() method - FE5: Keep Mine preserves dirty buffer - FE6: Keep Mine reschedules auto-save - FE8: native file-picker exclusion test - fix integration test infinite timer cascade from setInterval
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd1c90005d
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
382
to
+384
| pendingContent.current = response.content | ||
| lastSavedContent.current = response.content | ||
| lastKnownMtime.current = response.modifiedAt || null |
There was a problem hiding this comment.
When a disk-conflict is active, selecting a new path through handlePathSelect updates the editor content but leaves conflictState untouched. In that state, the banner/actions still point to the previous file’s snapshot, so clicking “Reload” can inject old-file content into the newly selected file and later saves can overwrite the wrong path; polling also remains disabled because poll returns early while conflictState is set. Clear conflict state after a successful file load (or on file-path change).
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| pollIntervalRef.current = setInterval(poll, 3000) |
There was a problem hiding this comment.
The polling loop is scheduled with setInterval around an async poll function, so a slow /api/files/read request can overlap with the next tick. If responses arrive out of order, an older read can still apply response.content/modifiedAt and overwrite newer state, causing temporary rollback or incorrect conflict decisions. Use an in-flight guard (or recursive setTimeout) so only one poll request chain runs at a time.
Useful? React with 👍 / 👎.
Local dev tool config with hardcoded absolute paths — should not be tracked in the repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mattleaverton
approved these changes
Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /api/files/statendpoint returning{exists, size, modifiedAt}for lightweight file metadata pollingshowOpenFilePicker()) are excluded from polling.text()mocks, handleKeepLocal re-triggers auto-saveTest Plan
Files Changed
server/files-router.ts— stat endpointsrc/components/panes/EditorPane.tsx— stat-polling, mtime tracking, conflict bannertest/unit/server/files-router.test.ts— stat endpoint teststest/unit/client/components/panes/EditorPane.autosave.test.tsx— stat-polling teststest/integration/client/editor-pane.test.tsx— conflict banner integration testdocs/plans/2026-03-29-editor-auto-sync.md— implementation plan