Skip to content

Persist per-tab terminal font zoom across app restarts - #8519

Closed
Zingzy wants to merge 3 commits into
manaflow-ai:mainfrom
Zingzy:fix-terminal-font-zoom-restore
Closed

Zingzy wants to merge 3 commits into
manaflow-ai:mainfrom
Zingzy:fix-terminal-font-zoom-restore

Conversation

@Zingzy

@Zingzy Zingzy commented Jul 20, 2026 •

Copy link
Copy Markdown

Summary

  • What changed? SessionTerminalPanelSnapshot gains an optional fontSize field holding the surface's unscaled base font points, mirroring how browser panels persist pageZoom. Capture reads the live surface via cmuxCurrentSurfaceFontSizePoints and normalizes against the global font magnification, falling back to the lineage-seeded points for hibernated or dead surfaces. Restore threads the value through newTerminalSurface into the surface config template, so a restored tab keeps its own zoom instead of inheriting the pane's. Legacy snapshots decode with fontSize nil and keep the previous behavior.
  • Why? Fixes Per-tab terminal font zoom (Cmd+= / Cmd+-) not restored after app restart — SessionTerminalPanelSnapshot has no font-size field #8515: per-tab terminal zoom (Cmd+= / Cmd+-) resets to the default on every relaunch. Zoom already survives a pane split via runtime inheritance, but never a restart, because the terminal snapshot had no font-size field.

Testing

  • Two-commit red/green structure per the regression test policy: the first commit adds only the test, the second adds the fix. The test injects fontSize through JSON so it compiles against pre-fix code and fails on behavior rather than on build.
  • Local run on macOS 26 (Apple Silicon, Xcode 26.6): xcodebuild test -scheme cmux-unit -only-testing:cmuxTests/SessionPersistenceTests. The new test fails at the seeding assertion on the test-only commit and passes on the fix commit. 133 of 142 suite tests pass; the 9 failing Hermes agent resume tests fail identically on a tree without this change (pre-existing local environment failures, not related).
  • Manual verification in a tagged Debug build (reload.sh --tag font-zoom): zoomed several terminal tabs down and up, quit and relaunched, each tab restored at its own zoom and untouched tabs restored at the default. Split-pane zoom inheritance is unchanged.

Demo Video

  • Video URL or attachment: none attached; happy to record one if useful.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed
  • I requested bot reviews after my latest commit (copy/paste block above or equivalent)
  • All code review bot comments are resolved
  • All human review comments are resolved

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Persists per-tab terminal font zoom across app restarts so each tab restores its own font size instead of inheriting the pane’s. Only explicitly zoomed tabs persist size; tabs at the config default keep following config after restart. Fixes #8515.

  • Bug Fixes
    • Added optional fontSize to SessionTerminalPanelSnapshot to store unscaled base points; backward compatible when nil.
    • Capture saves size only when zoom was adjusted at runtime or seeded by a zoomed template, normalizing against global magnification and falling back to lineage points if needed.
    • Restore threads the saved size into the surface config template so the tab keeps its zoom after relaunch.

Written for commit 7bba17a. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Terminal tabs now persist and restore each tab’s own font size and zoom settings across save, restore, and reload.
    • Session restore continues to apply the correct font size even when the terminal surface is not available for runtime probing.
  • Tests
    • Added coverage to verify injected/persisted terminal font size is honored during restore and reflected in subsequent session snapshots.

Zingzy added 2 commits July 20, 2026 20:09
Injects a fontSize value into a terminal panel session snapshot via JSON
and expects the restored workspace to seed the panel's inheritance font
points and carry the value into the next capture. Fails until the
terminal snapshot persists per-tab font size.

Refs manaflow-ai#8515
SessionTerminalPanelSnapshot gains an optional fontSize field holding the
surface's unscaled base font points, mirroring how browser panels persist
pageZoom. Capture reads the live surface via
cmuxCurrentSurfaceFontSizePoints and normalizes it against the global
font magnification, falling back to the lineage-seeded points for
hibernated or dead surfaces. Restore threads the value through
newTerminalSurface into the surface config template, so a restored tab
keeps its own zoom instead of inheriting the pane's. Old snapshots decode
with fontSize nil and keep the previous behavior.

Fixes manaflow-ai#8515
Copilot AI review requested due to automatic review settings July 20, 2026 17:15
@cursor

cursor Bot commented Jul 20, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Zingzy

Zingzy commented Jul 20, 2026

Copy link
Copy Markdown
Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 20, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@Zingzy I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 12 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown

@Zingzy, I’ll review the changes for #8519, including persistence, restore behavior, backward compatibility, and regression-test coverage.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Terminal session snapshots now persist optional per-tab font size. Workspace capture records runtime or fallback font points, and session restoration applies persisted values when recreating terminal surfaces. A regression test verifies restoration and subsequent snapshot capture.

Changes

Terminal Font Persistence

Layer / File(s) Summary
Snapshot font size capture
Sources/SessionPersistence.swift, Sources/Workspace.swift, Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift
SessionTerminalPanelSnapshot stores optional fontSize, while workspace snapshotting captures runtime or fallback terminal font points. TerminalSurface.configTemplate is publicly readable.
Restored terminal font application
Sources/Workspace.swift, cmuxTests/SessionPersistenceTests.swift
Terminal creation forwards persisted font points into local configuration, and tests verify restoration and subsequent snapshot capture.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SessionSnapshot
  participant Workspace
  participant TerminalSurface
  SessionSnapshot->>Workspace: restore terminal fontSize
  Workspace->>TerminalSurface: create with restoredFontBasePoints
  TerminalSurface-->>Workspace: restored terminal surface
  Workspace->>SessionSnapshot: capture current fontSize
Loading

Suggested reviewers: azooz2003-bit, austinywang, lawrencecchen

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #8515 by persisting terminal font size, restoring it on relaunch, and preserving split-pane zoom inheritance.
Out of Scope Changes check ✅ Passed No clearly unrelated changes stand out; the visibility tweak and API threading support the terminal zoom persistence work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed Workspace changes stay on @MainActor; the new snapshot field is a Sendable Float?, and TerminalSurface.configTemplate exposes immutable Sendable config only.
Cmux Swift Blocking Runtime ✅ Passed Full PR diff only adds fontSize plumbing; no new blocking waits, sleeps, syncs, polling, or locks appear in production code.
Cmux Browser Automation Off-Main ✅ Passed Diff only touches terminal session persistence/restore files; no browser.* routing, WebKit/AppKit worker-lane changes, or policy tests were added/changed.
Cmux Expensive Synchronous Load ✅ Passed Diff only threads terminal font size through session snapshot/restore; no new or moved agent-history load, JSONL scan, or RestorableAgentSessionIndex.load() call appears.
Cmux Cache Substitution Correctness ✅ Passed Live font size is still read first; the cached template is only a fallback with a documented cold/dead-surface rationale.
Cmux No Hacky Sleeps ✅ Passed Only Swift source/test files changed; the diff contains no sleeps, timers, polling, or delay hacks.
Cmux Algorithmic Complexity ✅ Passed Only O(1) font-size capture/restore plumbing was added per terminal panel; no new scalable rescans, repeated sorts, or batch-lookup loops were introduced.
Cmux Swift Concurrency ✅ Passed Diff only adds synchronous persistence/UI plumbing; no new DispatchQueue, Task.detached, Combine state, or completion-handler APIs beyond existing AppKit/XCTest boundaries.
Cmux Swift @Concurrent ✅ Passed Touched Swift changes are synchronous state plumbing only; no new @concurrent/nonisolated async code or UI-hop issues were introduced.
Cmux Swift Package Boundaries ✅ Passed The change is app-lifecycle persistence/restore wiring, with only a package-property visibility tweak; no reusable domain module was stranded in the app target.
Cmux Swiftpm Lockfiles ✅ Passed PR changes only Swift source/tests; no Package.resolved, Package.swift, .gitignore, workflow, or Xcode project files were modified.
Cmux Swift Logging ✅ Passed No added or changed print/debugPrint/dump/NSLog/Logger in the diff; changes are session/font-size plumbing and a visibility tweak.
Cmux User-Facing Error Privacy ✅ Passed Changed code only persists/restores terminal zoom and updates tests/public API; no user-facing errors, alerts, or raw internal details were added.
Cmux Full Internationalization ✅ Passed PR only changes persistence logic, a public Swift property, comments, and tests; no user-facing text or locale assets were added or altered.
Cmux Swiftui State Layout ✅ Passed No new SwiftUI state/layout pattern appears; the diff is session-persistence logic plus a TerminalSurface visibility tweak, with no GeometryReader, lazy-row store refs, or render-time mutation.
Cmux Architecture Rethink ✅ Passed Small value-propagation fix: snapshot owns persisted fontSize, Workspace threads it directly, and no new timing/observer/side-channel ownership is introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Diff only changes terminal font-size persistence and configTemplate visibility; no standalone window/controller code or identifier registration changed.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source/test files; no logs, caches, temp dirs, build output, or other artifact paths were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No new test/debug seam was added: the diff only threads fontSize through production restore/capture, and configTemplate is exposed for Workspace’s production use.
Cmux No Ambient Global State ✅ Passed The diff adds only stored properties and instance-method parameters for font restoration; no new top-level funcs, mutable globals, or singletons appear.
Title check ✅ Passed The title clearly summarizes the main change: persisting per-tab terminal font zoom across restarts.
Description check ✅ Passed The description follows the template with summary, testing, demo video note, review trigger, and checklist filled in.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR persists per-tab terminal font zoom across app restarts by adding an optional fontSize: Float? field to SessionTerminalPanelSnapshot, capturing it only for explicitly-zoomed tabs, and threading it through newTerminalSurface so a restored tab opens at its own zoom instead of the pane's inherited default.

  • SessionPersistence.swift gains the new optional fontSize field; legacy snapshots decode with nil and keep previous behavior.
  • Workspace.swift adds capture logic that reads ghostty_surface_font_size_adjusted to distinguish runtime zoom from the config default, falling back to the creation template for dead surfaces; the restore path injects the saved points into inheritedConfig before calling newTerminalSurface.
  • TerminalSurface.configTemplate is widened to public so the main-target capture code can read the creation template's font size.

Confidence Score: 4/5

Safe to merge after addressing the zoom-reset fallback logic in the capture closure.

The capture closure's return creationFontBasePoints fallback fires for both dead surfaces (correct) and live surfaces where the user has reset zoom to default via Cmd+0 (incorrect). In that second case, the stale template value is re-persisted, so a user's explicit zoom reset is silently undone on the next restart. Everything else — the new snapshot field, backward-compatible nil decoding, the restore threading through newTerminalSurface, and the configTemplate visibility widening — is correct and well-tested.

Sources/Workspace.swift — specifically the capturedFontBasePoints closure around lines 574–584 where the dead-surface and live-but-not-adjusted branches are collapsed into one fallback.

Important Files Changed

Filename Overview
Sources/Workspace.swift Adds font-size capture/restore threading through newTerminalSurface; the fallback closure conflates a dead surface with a live-but-reset surface, causing reset zoom to re-persist across restarts
Sources/SessionPersistence.swift Adds optional fontSize: Float? to SessionTerminalPanelSnapshot and its memberwise init; backward-compatible nil decoding for legacy snapshots; no issues
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift Widens configTemplate from internal to public so production Workspace.swift code can read the creation template's font size; no test seam, legitimate API expansion
cmuxTests/SessionPersistenceTests.swift New @mainactor test injects fontSize via JSON mutation and validates seeding and round-trip capture; accesses internal state via @testable import (correct pattern); test-only, no production seam

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant W as Workspace
    participant TS as TerminalSurface
    participant G as Ghostty (C API)
    participant SP as SessionPersistence

    Note over W: Capture (sessionSnapshot)
    W->>TS: configTemplate?.fontSize (creationFontBasePoints)
    W->>G: ghostty_surface_font_size_adjusted(surface)
    alt "surface live & adjusted"
        W->>G: cmuxCurrentSurfaceFontSizePoints(surface)
        G-->>W: runtimePoints
        W->>W: baseFontSize(fromRuntimePoints:percent:)
        W->>SP: "snapshot.fontSize = normalizedPoints"
    else surface dead or not adjusted
        W->>SP: "snapshot.fontSize = creationFontBasePoints (may be nil)"
    end

    Note over W: Restore (restoreSessionSnapshot)
    SP->>W: snapshot.terminal?.fontSize
    W->>W: "restoredFontBasePoints > 0?"
    alt has persisted zoom
        W->>W: "inheritedConfig.fontSize = restoredFontBasePoints"
    end
    W->>TS: newTerminalSurface(inheritedConfig)
    W->>W: seedTerminalInheritanceFontPoints(panelId, inheritedConfig)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant W as Workspace
    participant TS as TerminalSurface
    participant G as Ghostty (C API)
    participant SP as SessionPersistence

    Note over W: Capture (sessionSnapshot)
    W->>TS: configTemplate?.fontSize (creationFontBasePoints)
    W->>G: ghostty_surface_font_size_adjusted(surface)
    alt "surface live & adjusted"
        W->>G: cmuxCurrentSurfaceFontSizePoints(surface)
        G-->>W: runtimePoints
        W->>W: baseFontSize(fromRuntimePoints:percent:)
        W->>SP: "snapshot.fontSize = normalizedPoints"
    else surface dead or not adjusted
        W->>SP: "snapshot.fontSize = creationFontBasePoints (may be nil)"
    end

    Note over W: Restore (restoreSessionSnapshot)
    SP->>W: snapshot.terminal?.fontSize
    W->>W: "restoredFontBasePoints > 0?"
    alt has persisted zoom
        W->>W: "inheritedConfig.fontSize = restoredFontBasePoints"
    end
    W->>TS: newTerminalSurface(inheritedConfig)
    W->>W: seedTerminalInheritanceFontPoints(panelId, inheritedConfig)
Loading

Reviews (6): Last reviewed commit: "Persist terminal font zoom only for expl..." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a fontSize field to SessionTerminalPanelSnapshot so that per-tab terminal zoom (set via Cmd+= / Cmd+-) survives app restarts, mirroring how browser panel pageZoom is already persisted. The capture path reads the live surface's runtime font size and normalizes it against the global magnification factor; hibernated or dead surfaces fall back to the existing terminalInheritanceFontPointsByPanelId dictionary. Restore threads restoredFontBasePoints through two newTerminalSurface overloads into the surface config template, seeding the inheritance dictionary so subsequent snapshots (without a live surface) also round-trip correctly.

  • SessionPersistence.swift: Adds fontSize: Float? to SessionTerminalPanelSnapshot; nil preserves legacy behavior.
  • Workspace.swift: Capture computes base font points from the live surface or the lineage dictionary; restore overrides inheritedConfig.fontSize before the panel is created and relies on seedTerminalInheritanceFontPoints to propagate it.
  • SessionPersistenceTests.swift: A round-trip test injects fontSize via JSON (compile-compatible with pre-fix snapshots), asserts the lineage dictionary is seeded on restore, and verifies re-capture preserves the value without a live surface.

Confidence Score: 4/5

Safe to merge; the fix is narrowly scoped to persistence serialization and the tab-creation path, and legacy snapshots decode cleanly with nil.

The round-trip for explicitly-zoomed tabs works correctly, the test covers both the seeding and re-capture steps, and the existing inheritance dictionary already uses the same normalization formula. The one design gap is that absolute base points are captured for every tab, including unzoomed ones, which prevents a subsequent font-size config change from propagating to already-snapshotted tabs on restart.

Sources/Workspace.swift — the capture closure at line 568 unconditionally stores base points for all tabs; worth considering whether nil should be stored when the font size equals the pane's inherited default.

Important Files Changed

Filename Overview
Sources/SessionPersistence.swift Adds fontSize: Float? to SessionTerminalPanelSnapshot; clean backward-compatible optional field with a clear doc comment; no issues.
Sources/Workspace.swift Capture reads live surface font size and normalizes it; restore overrides inherited config. Storing absolute base points for ALL tabs means a user's default-font-size config change won't propagate to already-snapshotted tabs after restart.
cmuxTests/SessionPersistenceTests.swift New test uses JSON injection to stay compile-compatible with pre-fix code; reads existing internal property via @testable import (no test seam added to production source); covers both seed and re-capture assertions.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant App as App Quit
    participant WS as Workspace.sessionSnapshot
    participant Surface as Live Surface
    participant Dict as terminalInheritanceFontPointsByPanelId
    participant Snap as SessionTerminalPanelSnapshot

    App->>WS: sessionSnapshot(includeScrollback:)
    WS->>Surface: cmuxCurrentSurfaceFontSizePoints(surface)
    alt live surface available
        Surface-->>WS: runtimePoints
        WS->>WS: baseFontSize(fromRuntimePoints:percent:GlobalFontMagnification.storedPercent)
        WS-->>Snap: "fontSize = baseFontBasePoints"
    else hibernated / dead surface
        WS->>Dict: terminalInheritanceFontPointsByPanelId[panelId]
        Dict-->>WS: seededPoints (or nil)
        WS-->>Snap: "fontSize = seededPoints"
    end

    participant Restore as App Restart
    participant NTS as newTerminalSurface
    participant Seed as seedTerminalInheritanceFontPoints

    Restore->>NTS: "restoredFontBasePoints = snapshot.terminal?.fontSize"
    NTS->>NTS: "inheritedConfig.fontSize = restoredFontBasePoints"
    NTS->>Seed: seedTerminalInheritanceFontPoints(panelId, inheritedConfig)
    Seed->>Dict: "terminalInheritanceFontPointsByPanelId[panelId] = fontSize"
    NTS->>NTS: TerminalPanel(configTemplate: inheritedConfig, …)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant App as App Quit
    participant WS as Workspace.sessionSnapshot
    participant Surface as Live Surface
    participant Dict as terminalInheritanceFontPointsByPanelId
    participant Snap as SessionTerminalPanelSnapshot

    App->>WS: sessionSnapshot(includeScrollback:)
    WS->>Surface: cmuxCurrentSurfaceFontSizePoints(surface)
    alt live surface available
        Surface-->>WS: runtimePoints
        WS->>WS: baseFontSize(fromRuntimePoints:percent:GlobalFontMagnification.storedPercent)
        WS-->>Snap: "fontSize = baseFontBasePoints"
    else hibernated / dead surface
        WS->>Dict: terminalInheritanceFontPointsByPanelId[panelId]
        Dict-->>WS: seededPoints (or nil)
        WS-->>Snap: "fontSize = seededPoints"
    end

    participant Restore as App Restart
    participant NTS as newTerminalSurface
    participant Seed as seedTerminalInheritanceFontPoints

    Restore->>NTS: "restoredFontBasePoints = snapshot.terminal?.fontSize"
    NTS->>NTS: "inheritedConfig.fontSize = restoredFontBasePoints"
    NTS->>Seed: seedTerminalInheritanceFontPoints(panelId, inheritedConfig)
    Seed->>Dict: "terminalInheritanceFontPointsByPanelId[panelId] = fontSize"
    NTS->>NTS: TerminalPanel(configTemplate: inheritedConfig, …)
Loading

Reviews (2): Last reviewed commit: "Persist per-tab terminal font zoom acros..." | Re-trigger Greptile

Comment thread Sources/Workspace.swift
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds an optional fontSize: Float? field to SessionTerminalPanelSnapshot so that per-tab terminal font zoom (Cmd+= / Cmd+-) survives app restarts. Capture reads the live surface via cmuxCurrentSurfaceFontSizePoints, normalizes against GlobalFontMagnification.storedPercent to get the unscaled base points, and falls back to the terminalInheritanceFontPointsByPanelId dict for dead/hibernated surfaces. Restore threads the value through newTerminalSurface into the CmuxSurfaceConfigTemplate before the panel is created.

  • SessionPersistence.swift gains the fontSize: Float? field on SessionTerminalPanelSnapshot; legacy snapshots decode with nil and keep existing behavior.
  • Workspace.swift captures the base font points at snapshot time and applies them at restore time by overriding inheritedConfig.fontSize before panel creation, then seeds terminalInheritanceFontPointsByPanelId via the existing seedTerminalInheritanceFontPoints call.
  • SessionPersistenceTests.swift adds a @MainActor test that injects fontSize via JSON surgery so the test can target the pre-fix binary and verify both the seeding assertion and round-trip recapture.

Confidence Score: 4/5

The change is narrowly scoped to session snapshot capture and restore; legacy snapshots with nil fontSize decode without change and fall through to existing behavior, so there is no rollback risk.

The capture logic correctly prefers the live C-bridge read and falls back to the inheritance dict only for dead/hibernated surfaces. The restore path overrides inheritedConfig.fontSize before panel creation and the existing seedTerminalInheritanceFontPoints call propagates the value so the round-trip works without a live surface. The one nuance worth watching is the dead-surface fallback: if a user manually zoomed a tab and it was deallocated before rememberTerminalConfigInheritanceSource was refreshed, the seeded value in terminalInheritanceFontPointsByPanelId would lag behind the actual zoom — but this is an acknowledged best-effort fallback, and the live-surface path covers the vast majority of quit-time snapshots.

The capturedFontBasePoints closure in Sources/Workspace.swift is the subtlest part of the change and deserves a second read to confirm the magnification-normalization math matches what CmuxSurfaceConfigTemplate.baseFontSize(fromRuntimePoints:percent:) expects.

Important Files Changed

Filename Overview
Sources/SessionPersistence.swift Adds optional fontSize: Float? field to SessionTerminalPanelSnapshot with a matching init parameter and nil default; backward-compatible with legacy snapshots.
Sources/Workspace.swift Capture path reads live surface font via C bridge and normalizes against global magnification, with a fallback to the inheritance dict for dead surfaces; restore path overrides the inherited config before panel creation.
cmuxTests/SessionPersistenceTests.swift Adds a well-structured @mainactor round-trip test that injects fontSize via JSON surgery, checks the inheritance-dict seeding assertion, and verifies recapture without a live surface.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant W as Workspace
    participant SP as SessionPersistence
    participant C as C Bridge (cmuxCurrentSurfaceFontSizePoints)
    participant D as terminalInheritanceFontPointsByPanelId

    Note over W,D: Capture (sessionSnapshot)
    W->>C: cmuxCurrentSurfaceFontSizePoints(surface)
    alt live surface available
        C-->>W: runtimePoints (Float)
        W->>W: baseFontSize(fromRuntimePoints:percent:)
        W->>SP: "fontSize = basePoints"
    else dead / hibernated surface
        W->>D: terminalInheritanceFontPointsByPanelId[panelId]
        D-->>W: seeded base points (or nil)
        W->>SP: "fontSize = seededPoints (or nil)"
    end

    Note over W,D: Restore (restoreSessionSnapshot → newTerminalSurface)
    SP-->>W: snapshot.terminal?.fontSize
    alt "fontSize non-nil and > 0"
        W->>W: "template.fontSize = restoredFontBasePoints"
        W->>W: "inheritedConfig = template"
    end
    W->>W: TerminalPanel(configTemplate: inheritedConfig)
    W->>D: seedTerminalInheritanceFontPoints(panelId, inheritedConfig)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant W as Workspace
    participant SP as SessionPersistence
    participant C as C Bridge (cmuxCurrentSurfaceFontSizePoints)
    participant D as terminalInheritanceFontPointsByPanelId

    Note over W,D: Capture (sessionSnapshot)
    W->>C: cmuxCurrentSurfaceFontSizePoints(surface)
    alt live surface available
        C-->>W: runtimePoints (Float)
        W->>W: baseFontSize(fromRuntimePoints:percent:)
        W->>SP: "fontSize = basePoints"
    else dead / hibernated surface
        W->>D: terminalInheritanceFontPointsByPanelId[panelId]
        D-->>W: seeded base points (or nil)
        W->>SP: "fontSize = seededPoints (or nil)"
    end

    Note over W,D: Restore (restoreSessionSnapshot → newTerminalSurface)
    SP-->>W: snapshot.terminal?.fontSize
    alt "fontSize non-nil and > 0"
        W->>W: "template.fontSize = restoredFontBasePoints"
        W->>W: "inheritedConfig = template"
    end
    W->>W: TerminalPanel(configTemplate: inheritedConfig)
    W->>D: seedTerminalInheritanceFontPoints(panelId, inheritedConfig)
Loading

Reviews (3): Last reviewed commit: "Persist per-tab terminal font zoom acros..." | Re-trigger Greptile

Tabs at the config default no longer write a font size into the session
snapshot, so a config font-size change still applies to them on the next
restart. Zoom is persisted when the surface reports a runtime adjustment
(ghostty_surface_font_size_adjusted) or when it was created from a zoomed
template (restored tab, split child of a zoomed pane). The template
fallback matters because config-set font sizes never mark the surface as
adjusted, so flag-only gating would drop a restored zoom on the second
restart.
@Zingzy

Zingzy commented Jul 20, 2026

Copy link
Copy Markdown
Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 20, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@Zingzy I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 12 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown

@Zingzy, I’ll review the changes in #8519, focusing on terminal font-size persistence, restoration behavior, backward compatibility, and test coverage.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Per-tab terminal font zoom (Cmd+= / Cmd+-) not restored after app restart — SessionTerminalPanelSnapshot has no font-size field

2 participants