Add terminal timestamps - #3955
Closed
austinywang wants to merge 39 commits into
Closed
austinywang wants to merge 39 commits into
austinywang wants to merge 39 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a toggleable terminal timestamp gutter: data store, runtime setting with persistence/managed-file support, keyboard shortcut and command wiring, gutter UI integrated into the terminal scroll view, Settings/View menu toggles, localization/schema updates, and tests. ChangesTerminal Timestamps Feature
Sequence DiagramsequenceDiagram
participant User
participant AppMenu as App Menu / Shortcut
participant Settings as SettingsView
participant Surface as GhosttySurfaceScrollView
participant Store as TerminalTimestampStore
participant Gutter as TerminalTimestampGutterView
User->>AppMenu: invoke "Toggle Terminal Timestamps"
AppMenu->>Settings: TerminalTimestampsSettings.toggle()
Settings->>Surface: didChange notification
Surface->>Store: record(scrollbar:at:markVisibleRows: true)
Surface->>Gutter: refreshTimestampGutter()
Gutter->>Store: visibleRows(for: scrollbarState)
Store->>Gutter: TerminalTimestampVisibleRow[]
User->>Surface: scroll / scrollbar update
Surface->>Store: record(scrollbar:at:markVisibleRows: false)
Surface->>Gutter: refreshTimestampGutter()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR adds a toggleable terminal timestamp gutter that stamps per-row output times, wired through Settings, View menu, command palette (⌥⌘⇧E),
Confidence Score: 5/5Safe to merge; all previous review findings are addressed and no new issues were found. All eight previously flagged issues were resolved in prior iterations. The lock-guarded intent stack correctly handles the I/O-thread boundary and sticky keyboard flag across coalesced output events. All 8 new string keys carry translations for all 19 supported locales. The TerminalTimestampStore pruning logic is bounded and correctly avoids hot-path reallocations. No new logic, isolation, or localization defects were found. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant KB as Keyboard Event (Main)
participant NSView as GhosttyNSView (Main)
participant Lock as _scrollbarLock
participant IO as Ghostty I/O Thread
participant Main as Main Thread Flush
participant Store as TerminalTimestampStore
KB->>NSView: keyDown (PageUp/PageDown or copy-mode)
NSView->>Lock: beginScrollbarActionIntent([.keyboardInitiated, .explicitSync])
Note over Lock: _scrollbarActionIntentStack += intent<br/>_pendingScrollbarActionIntent |= intent
IO->>NSView: GHOSTTY_ACTION_SCROLLBAR
NSView->>Lock: consumeScrollbarActionIntent()
Lock-->>NSView: [.keyboardInitiated, .explicitSync]
NSView->>Main: enqueueScrollbarUpdate(scrollbar, intent)
Note over IO: New output line arrives
IO->>NSView: GHOSTTY_ACTION_SCROLLBAR (output)
NSView->>Lock: consumeScrollbarActionIntent()
Note over Lock: _pendingScrollbarActionIntent=[]<br/>but stack still has entry: sticky!
Lock-->>NSView: [.keyboardInitiated, .explicitSync]
NSView->>Main: enqueueScrollbarUpdate(coalesced)
KB->>NSView: keyUp: endScrollbarActionIntent()
Lock->>Lock: pop _scrollbarActionIntentStack
Main->>Main: flushPendingScrollbar()
Main->>Store: record(scrollbar, markVisibleRows: false)
Note over Store: isUserInitiatedScroll=true<br/>no timestamp backfill
Main->>Main: synchronizeScrollView() + refreshTimestampGutter()
Reviews (27): Last reviewed commit: "Add terminal timestamp localizations" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:7808">
P2: Keyboard scroll intent detection is too broad: Ctrl/Option+PageUp/PageDown can leave a stale pending keyboard-scroll flag and misclassify the next passive scrollbar update.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
…-3690-terminal-timestamps
…imestamps # Conflicts:
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9c1a74e. Configure here.
| case "gemini": | ||
| return preserveOptions(args, policy: geminiPolicy) | ||
| case "antigravity": | ||
| return preserveOptions(args, policy: antigravityPolicy) |
There was a problem hiding this comment.
Removed sanitizer policy breaks antigravity agent launch and resume
High Severity
The "antigravity" case and antigravityPolicy were removed from preservedArguments(kind:args:), but CLI/cmux.swift still routes antigravity agents through preservedArguments(kind: "antigravity", ...) for both launch sanitization and session resume. The default case now returns nil, which silently rejects all antigravity agent launches (treated as unsafe) and prevents session resume. This is a silent functional regression for any existing antigravity users.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9c1a74e. Configure here.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
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
3 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
Closes #3690
Validation
Note
Medium Risk
Medium risk because it introduces new CircleCI workflows plus a
pull_request_targetGitHub Action that can approve CI holds based on org/user trust, which impacts CI security posture and execution gating.Overview
Adds a full
.circleci/config.ymlwith macOS unit-test, debug-build (warning budget), and release universal-build jobs, including SwiftPM/Zig caching and a gated workflow that requires an explicit hold step before running on PRs/tags.Introduces a
pull_request_targetGitHub Action (circleci-auto-approve.yml) that runsscripts/circleci_auto_approve.pyto automatically approve the CircleCI hold for PRs from trusted org members/users.Tightens Greptile “no hacky sleeps” rule scoping to repo
web/andscripts/paths and updates the rule text accordingly; also simplifies the perf activation workflow by removing explicit SwiftPM caching/resolution steps.Removes the
antigravityagent launch sanitization policy and its tests, and deletes related Agent icon assets (includingGrok/Antigravityimage sets).Reviewed by Cursor Bugbot for commit 9c1a74e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds an optional left-side timestamp gutter aligned with terminal output rows. Toggle via Settings, View menu, Command Palette, ⌥⌘⇧E, or
cmux.json(terminal.showTimestamps, default off). Closes #3690.New Features
TerminalTimestampStorestamps new/visible rows, skips backfill on keyboard-initiated backscroll, prunes to 20k, retains visible rows across viewport changes and after scrollback rebase/shrink, and updates on scroll/setting changes. Settings search, View menu toggle, Command Palette action, shortcut, docs, tests, and localizations added.Bug Fixes
ghostty.scrollbarUpdateIntentandghostty.scrollbarWasKeyboardInitiated(viaGhosttyScrollbarUpdateIntent) to carry intent, explicitly sync viewport during search navigation, coalesce intent across callbacks, and limit keyboard intent to viewport motions.Written for commit 9c1a74e. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Settings
Localization
Tests & Docs