From 47802e7290504bfbcb76f8d2b4cc307afe616b7d Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 20 Aug 2026 21:45:26 -0500 Subject: [PATCH 1/2] fix: disable unsupported ssh shell-integration features ghostty implements `ssh-env` and `ssh-terminfo` by replacing `ssh` with a wrapper that calls a `ghostty` CLI absent from agterm's bundle. Enabling either option made every `ssh` invocation fail. read the resolved feature mask after recursive config loading, force the two unsupported bits off, and restate every known flag so the other choices survive ghostty's reset-from-defaults assignment semantics. Hosted tests pin the bit mapping and total feature count against the bundled libghostty. Fix #463 --- .claude/rules/libghostty.md | 6 ++ agterm/Ghostty/GhosttyApp.swift | 25 ++++++++ agterm/SettingsModel.swift | 4 ++ .../agtermCore/ShellIntegrationFeatures.swift | 22 +++++++ .../ShellIntegrationFeaturesTests.swift | 41 ++++++++++++ .../ShellIntegrationFeatureBitsTests.swift | 64 +++++++++++++++++++ docs/troubleshooting.md | 2 +- .../agterm/skills/agterm/troubleshooting.md | 7 +- site/docs.html | 11 ++++ 9 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 agtermCore/Sources/agtermCore/ShellIntegrationFeatures.swift create mode 100644 agtermCore/Tests/agtermCoreTests/ShellIntegrationFeaturesTests.swift create mode 100644 agtermTests/ShellIntegrationFeatureBitsTests.swift diff --git a/.claude/rules/libghostty.md b/.claude/rules/libghostty.md index 68216ba8..cd3997c0 100644 --- a/.claude/rules/libghostty.md +++ b/.claude/rules/libghostty.md @@ -223,6 +223,12 @@ paths: `shell-integration-features = no-cursor,no-title`. `no-cursor` prevents prompt DECSCUSR bar resets; `no-title` prevents abbreviated local cwd OSC 2 from overriding sidebar names. User/remote OSC titles still work, and OSC 7 is unaffected. +- `ssh-env` and `ssh-terminfo` are forced OFF after `ghostty_config_load_recursive_files`, so no user + source including a `config-file` include can enable them: their wrappers call a `ghostty` CLI agterm + does not bundle, and enabling either broke `ssh` outright (#463). The override reads the resolved + packed bits back and restates all six flags, because ghostty re-parses the key from its defaults on + every occurrence. Setting either is a silent no-op with no diagnostic: a report that it has no effect + is by design, while a report that it still installs an `ssh` wrapper or breaks `ssh` is a regression. - A one-shot local OSC 2 is cleared by the next prompt. Hold the shell with `printf '\033]2;X\007'; cat` to test; SSH works because it blocks the local prompt cycle. - `liveFocus` is key window and first responder. The key gate is essential because AppKit retains one diff --git a/agterm/Ghostty/GhosttyApp.swift b/agterm/Ghostty/GhosttyApp.swift index 3164f933..38c8bef1 100644 --- a/agterm/Ghostty/GhosttyApp.swift +++ b/agterm/Ghostty/GhosttyApp.swift @@ -517,6 +517,30 @@ final class GhosttyApp { return cfg } + /// Their ssh wrappers require a `ghostty` CLI absent from agterm's bundle. + private static let unsupportedShellFeatures: Set = ["ssh-env", "ssh-terminfo"] + + /// Applies the override after recursive includes so no user config can re-enable these features. + private func forceUnsupportedShellFeaturesOff(_ cfg: ghostty_config_t) { + let key = "shell-integration-features" + var bits: UInt32 = 0 + guard key.withCString({ ghostty_config_get(cfg, &bits, $0, UInt(key.utf8.count)) }) else { + logger.warning("could not read \(key, privacy: .public); leaving ssh shell features as configured") + return + } + let value = ShellIntegrationFeatures.overrideValue(resolvedBits: bits, + disabled: Self.unsupportedShellFeatures) + let tmp = (NSTemporaryDirectory() as NSString).appendingPathComponent("agterm-sif-\(UUID().uuidString).conf") + do { + try "shell-integration-features = \(value)\n".write(toFile: tmp, atomically: true, encoding: .utf8) + } catch { + logger.warning("shell-integration-features override write failed: \(error.localizedDescription, privacy: .public)") + return + } + tmp.withCString { ghostty_config_load_file(cfg, $0) } + try? FileManager.default.removeItem(atPath: tmp) + } + private func loadConfig(_ inputs: ConfigInputs, extraOverlayPath: String? = nil) -> ghostty_config_t? { guard let cfg = ghostty_config_new() else { return nil } @@ -559,6 +583,7 @@ final class GhosttyApp { } ghostty_config_load_recursive_files(cfg) + forceUnsupportedShellFeaturesOff(cfg) ghostty_config_finalize(cfg) let diagCount = ghostty_config_diagnostics_count(cfg) diff --git a/agterm/SettingsModel.swift b/agterm/SettingsModel.swift index 1e9a714f..5b870a95 100644 --- a/agterm/SettingsModel.swift +++ b/agterm/SettingsModel.swift @@ -567,6 +567,10 @@ final class SettingsModel { # # NOTE: agterm's UI-managed keys (font, theme, background opacity/blur, scroll speed) are set # in Settings and always win over this file — set those in Settings, everything else here. + # + # NOT SUPPORTED: the `ssh-env` and `ssh-terminfo` shell-integration features. They work by + # wrapping `ssh` as a call to the `ghostty` CLI absent from agterm's bundle, + # so agterm forces them back off. Your other shell-integration-features flags are kept. """ } diff --git a/agtermCore/Sources/agtermCore/ShellIntegrationFeatures.swift b/agtermCore/Sources/agtermCore/ShellIntegrationFeatures.swift new file mode 100644 index 00000000..a6b1e5d5 --- /dev/null +++ b/agtermCore/Sources/agtermCore/ShellIntegrationFeatures.swift @@ -0,0 +1,22 @@ +import Foundation + +/// Renders an explicit `shell-integration-features` line from libghostty's resolved packed bits, so +/// ghostty stays the only thing that parses config text. +public enum ShellIntegrationFeatures { + /// Flag names in the field order of libghostty's `ShellIntegrationFeatures`, so an index here is that + /// flag's bit position. `ShellIntegrationFeatureBitsTests` pins the positions and the count. + static let ordered = ["cursor", "sudo", "title", "ssh-env", "ssh-terminfo", "path"] + + /// Every flag spelled out from `resolvedBits`, with `disabled` forced off. Naming all of them is the + /// point: ghostty re-parses this key from its defaults, so an omitted flag is reset, not left alone. + public static func overrideValue(resolvedBits: UInt32, disabled: Set) -> String { + ordered.enumerated().map { index, name in + let on = bits(resolvedBits, hasFlagAt: index) && !disabled.contains(name) + return on ? name : "no-\(name)" + }.joined(separator: ",") + } + + static func bits(_ value: UInt32, hasFlagAt index: Int) -> Bool { + value & (1 << UInt32(index)) != 0 + } +} diff --git a/agtermCore/Tests/agtermCoreTests/ShellIntegrationFeaturesTests.swift b/agtermCore/Tests/agtermCoreTests/ShellIntegrationFeaturesTests.swift new file mode 100644 index 00000000..0ac89ec4 --- /dev/null +++ b/agtermCore/Tests/agtermCoreTests/ShellIntegrationFeaturesTests.swift @@ -0,0 +1,41 @@ +import Foundation +import Testing +@testable import agtermCore + +struct ShellIntegrationFeaturesTests { + private let sshFlags: Set = ["ssh-env", "ssh-terminfo"] + + /// cursor | title | path, ghostty's own defaults for this struct. + private let defaultBits: UInt32 = 0b100101 + + @Test func defaultsRenderWithBothSSHFlagsOff() { + #expect(ShellIntegrationFeatures.overrideValue(resolvedBits: defaultBits, disabled: sshFlags) + == "cursor,no-sudo,title,no-ssh-env,no-ssh-terminfo,path") + } + + @Test func aResolvedSSHFlagIsForcedOffWhileEveryOtherFlagSurvives() { + // defaults with no-cursor and ssh-terminfo, the reporter's config in #463 + let bits: UInt32 = 0b110100 + #expect(ShellIntegrationFeatures.overrideValue(resolvedBits: bits, disabled: sshFlags) + == "no-cursor,no-sudo,title,no-ssh-env,no-ssh-terminfo,path") + } + + @Test func everyFlagOnStillLosesOnlyTheSSHPair() { + #expect(ShellIntegrationFeatures.overrideValue(resolvedBits: 0b111111, disabled: sshFlags) + == "cursor,sudo,title,no-ssh-env,no-ssh-terminfo,path") + } + + @Test func everyFlagOffStaysOff() { + #expect(ShellIntegrationFeatures.overrideValue(resolvedBits: 0, disabled: sshFlags) + == "no-cursor,no-sudo,no-title,no-ssh-env,no-ssh-terminfo,no-path") + } + + @Test func nothingDisabledLeavesAResolvedSSHFlagOn() { + #expect(ShellIntegrationFeatures.overrideValue(resolvedBits: defaultBits | (1 << 4), disabled: []) + == "cursor,no-sudo,title,no-ssh-env,ssh-terminfo,path") + } + + @Test func flagOrderMatchesLibghosttysFieldOrder() { + #expect(ShellIntegrationFeatures.ordered == ["cursor", "sudo", "title", "ssh-env", "ssh-terminfo", "path"]) + } +} diff --git a/agtermTests/ShellIntegrationFeatureBitsTests.swift b/agtermTests/ShellIntegrationFeatureBitsTests.swift new file mode 100644 index 00000000..4f2d2182 --- /dev/null +++ b/agtermTests/ShellIntegrationFeatureBitsTests.swift @@ -0,0 +1,64 @@ +import GhosttyKit +import XCTest +@testable import agterm +@testable import agtermCore + +/// Pins the Swift flag mapping and count against the bundled libghostty. +@MainActor +final class ShellIntegrationFeatureBitsTests: XCTestCase { + private func resolvedBits(_ cfg: ghostty_config_t) -> UInt32? { + let key = "shell-integration-features" + var bits: UInt32 = 0 + let got = key.withCString { ghostty_config_get(cfg, &bits, $0, UInt(key.utf8.count)) } + return got ? bits : nil + } + + func testDefaultBitsMatchTheKnownLayout() throws { + let cfg = try XCTUnwrap(ghostty_config_new()) + defer { ghostty_config_free(cfg) } + ghostty_config_finalize(cfg) + + // ghostty's own defaults: cursor, title and path on; sudo and both ssh flags off. + XCTAssertEqual(resolvedBits(cfg), 0b100101) + } + + // catches a trailing upstream field the per-name mapping test cannot see + func testBoolTrueCoversExactlyTheFlagsWeKnowAbout() throws { + let dir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + let file = dir.appendingPathComponent("all.conf") + try "shell-integration-features = true\n".write(to: file, atomically: true, encoding: .utf8) + + let cfg = try XCTUnwrap(ghostty_config_new()) + defer { ghostty_config_free(cfg) } + file.path.withCString { ghostty_config_load_file(cfg, $0) } + ghostty_config_finalize(cfg) + + let known = (UInt32(1) << UInt32(ShellIntegrationFeatures.ordered.count)) - 1 + XCTAssertEqual(resolvedBits(cfg), known, "libghostty has a shell-integration feature ShellIntegrationFeatures.ordered does not") + } + + func testEachFlagOccupiesTheBitItsNameIsMappedTo() throws { + let dir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + // each spelling must move only its own bit against ghostty's defaults + let defaults: UInt32 = 0b100101 + for (index, name) in ShellIntegrationFeatures.ordered.enumerated() { + let bit = UInt32(1) << UInt32(index) + for (value, expected) in [(name, defaults | bit), ("no-\(name)", defaults & ~bit)] { + let file = dir.appendingPathComponent("\(value).conf") + try "shell-integration-features = \(value)\n".write(to: file, atomically: true, encoding: .utf8) + + let cfg = try XCTUnwrap(ghostty_config_new()) + defer { ghostty_config_free(cfg) } + file.path.withCString { ghostty_config_load_file(cfg, $0) } + ghostty_config_finalize(cfg) + + XCTAssertEqual(resolvedBits(cfg), expected, "\(value) does not move bit \(index) alone") + } + } + } +} diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 73b2bc3f..74331163 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -99,7 +99,7 @@ A reload applies most keys to your open terminals right away — colors, theme, - **Layout keys** — `window-padding-x`, `window-padding-y`, and other size-affecting keys — do not re-apply to an open pane. libghostty re-derives a surface's padding only when it is first laid out, so a reload (and even resizing the window) leaves existing panes on their old padding. Open a new session or new window to pick up the change; the panes that were already open need a relaunch. - **Spawn-time keys** — `term` and `shell-integration-features` — are read once when the shell starts, so a reload cannot change them for a shell that is already running. Open a new session, whose shell is spawned fresh, to apply them. -The full ghostty key reference is at . +The full ghostty key reference is at . One pair of values in it does not apply to agterm: the `ssh-env` and `ssh-terminfo` values of `shell-integration-features`. Ghostty implements both by replacing your `ssh` with a wrapper that calls the `ghostty` command-line tool absent from agterm's bundle, so in agterm the wrapper would fail on every connection. agterm forces those two values back off and keeps the rest of your `shell-integration-features` flags, so `ssh` stays the real `ssh`. If you need agterm's terminfo entry on a remote host, install it there once with `infocmp -x xterm-ghostty | ssh 'tic -x -'`. ## Copy/paste and shortcuts on a non-Latin or alternative layout diff --git a/plugins/agterm/skills/agterm/troubleshooting.md b/plugins/agterm/skills/agterm/troubleshooting.md index 33ce9044..fed00b56 100644 --- a/plugins/agterm/skills/agterm/troubleshooting.md +++ b/plugins/agterm/skills/agterm/troubleshooting.md @@ -29,7 +29,12 @@ You are inside agterm (`AGTERM_ENABLED=1`). Use: global Ghostty config is on. agterm's Settings (font/theme/opacity/scroll) still win. Use it for keys the UI does not expose, e.g. `macos-option-as-alt`. Most keys apply to open panes on reload, but layout keys (`window-padding-*`) and spawn-time keys (`term`, `shell-integration-features`) only take effect in a new session/window - or after a relaunch. Full reference: https://ghostty.org/docs/config + or after a relaunch. Full reference: https://ghostty.org/docs/config. Two values in it do NOT apply: + `ssh-env` and `ssh-terminfo` for `shell-integration-features`. Ghostty implements them by replacing + `ssh` with a wrapper calling a `ghostty` CLI absent from agterm's bundle, so agterm forces both off + after reading the config and keeps every other flag. Setting either is by design a no-op, reports no + diagnostic, and is NOT a bug. For remote terminfo, install the entry manually with + `infocmp -x xterm-ghostty | ssh 'tic -x -'`. - **Logs** (unified logging, subsystem `com.umputun.agterm`): ```bash log show --predicate 'subsystem == "com.umputun.agterm"' --info --last 30m diff --git a/site/docs.html b/site/docs.html index dc5a5c3e..a18c2652 100644 --- a/site/docs.html +++ b/site/docs.html @@ -2510,6 +2510,17 @@ agterm manages from Settings (font, theme, opacity, blur, scroll speed) still win. A common use: macos-option-as-alt = true. The full key reference is at ghostty.org/docs/config. + Two values in it do not apply here: + ssh-env and + ssh-terminfo for + shell-integration-features. + Ghostty implements both by replacing your + ssh with a wrapper that calls + the ghostty command-line tool + absent from agterm's bundle, so the wrapper would fail on every connection. agterm forces those two + back off and keeps the rest of your flags, leaving + ssh the real + ssh.

A keybind here follows ghostty's From 443186079af54e134d6e5c46abb65e4890ce6d68 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 20 Aug 2026 22:13:48 -0500 Subject: [PATCH 2/2] chore(revmux): track the project review profile `.revmux/profile.md` calibrates every revmux review of this repo: what agterm is, what a real failure looks like here, the blast radius, and which conventions are deliberate rather than defects. Without it each review runs on generic calibration and flags the short-comment policy and the visibility rule as problems. `.revmux/` was ignored wholesale for the task and run archive, and a re-include under an excluded directory is unreachable, so the pattern becomes `.revmux/*` with the profile as the one exception. The archive stays ignored. --- .gitignore | 7 ++++-- .revmux/profile.md | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 .revmux/profile.md diff --git a/.gitignore b/.gitignore index 638413d3..301cb817 100644 --- a/.gitignore +++ b/.gitignore @@ -25,8 +25,11 @@ windows/ __pycache__/ .ruff_cache/ -# revmux review scaffolding: caller-written task context plus its run archive, local to a review -.revmux/ +# revmux review scaffolding: caller-written task context plus its run archive, local to a review. +# profile.md is the exception — it calibrates every review of this repo, so it is tracked. Excluding +# the directory itself would make that re-include unreachable, hence the glob. +.revmux/* +!.revmux/profile.md # track the path-scoped Claude rules (the global ~/.gitignore_global ignores .claude/) !.claude/ diff --git a/.revmux/profile.md b/.revmux/profile.md new file mode 100644 index 00000000..6c54351b --- /dev/null +++ b/.revmux/profile.md @@ -0,0 +1,59 @@ +# agterm + +## What it is + +A native macOS terminal, SwiftUI over libghostty, with a workspace-to-session sidebar and a full +control API driven by the bundled `agtermctl` over a local unix socket. Shipped as a signed and +notarized `.app` on Homebrew, plus a static site at agterm.com. One maintainer, no on-call. + +Two modules. `agtermCore` is a host-free SwiftPM package under Swift 6 complete concurrency checking +with no Xcode, GhosttyKit, AppKit, Metal or CoreGraphics dependency, holding the model, persistence, +parsing, validation, routing, response shaping and static catalogs. The app target is the side-effect +adapter: the SwiftUI shell, the AppKit sidebar, and the C boundary to libghostty. + +## What a real failure looks like here + +A user's terminal session becomes unusable or is lost: a pane that stops painting, a session that +restores without its shell environment, a spawned command that never runs, a control command that +lands on the wrong session, a crash at the libghostty C boundary. Persisted window and session state +being corrupted is the closest thing to data loss. + +libghostty is called through a C boundary from an `@unchecked Sendable` callbacks type, not a +main-actor one, so use-after-free and cross-thread access there are real and have shipped before. + +## Blast radius + +One user per install, recoverable by relaunch. No server, no customer data, nothing irreversible. +A bad release reaches everyone on Homebrew until the next one, which raises the bar for anything in +the launch, restore or surface-lifecycle paths specifically. + +## Reporting bar + +Severity follows user-visible consequence: critical for data loss or a broken primary path, major for +wrong results or a broken secondary path, minor otherwise. Documentation inaccuracies are never +critical or major. + +macOS is the only platform, so POSIX portability is not a finding on its own, and a shellcheck SC3xxx +on a shipped script is a lint gate rather than a runtime defect. + +## Deliberate conventions, not defects + +- Comments and docs are kept short on purpose. Only non-obvious constraints, rejected alternatives, or + why the obvious implementation fails. Narrating code, restating a fact owned elsewhere, or a doc + comment longer than the body it documents are all defects in the other direction. +- Test comments are rare and one line. No arrange/act/assert labels, no restating an assertion. +- Private by default. Exported only for an out-of-package caller. +- Interfaces are defined on the consumer side; the app target accepts them and returns concrete types. +- SwiftLint runs strict with zero findings required: 200-column lines, 1000-line files, 800-line types, + raised to 2000 for tests. Disabled and tuned rules are deliberate. +- `agterm/Resources/ghostty`, `agterm/Resources/terminfo` and `GhosttyKit.xcframework` are gitignored + build artifacts staged from upstream ghostty at a pinned revision, not project source. +- `cookbook/` recipes are third-party work the project publishes but does not own. + +## What the project keeps in sync + +A new user action is incomplete until the control protocol, the dispatcher, `agtermctl` and the +protocol tests all carry it, and a state-setting command must expose its result on the control tree. +`site/docs.html` is the canonical user guide, `site/commands.html` the canonical command reference, +and the bundled agent skill under `plugins/agterm/skills/agterm/` is the source for installed copies. +A change to the control API, the keymap or the model that updates only some of those is a real finding.