Skip to content

Commit 0a60281

Browse files
committed
Fix command shim status edge cases
1 parent 4610365 commit 0a60281

14 files changed

Lines changed: 598 additions & 63 deletions

Sources/App/Services/BrokerStatusController.swift

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,11 @@ struct LocalBrokerStatusController: BrokerStatusControlling {
369369
_ = runProcess(executable: "/bin/launchctl", arguments: ["bootout", "gui/\(getuid())", launchAgent.path])
370370
}
371371

372-
removeManagedShims(in: shimDirectory, appDestination: appDestination)
373-
try? fileManager.removeItem(at: shimDirectory)
372+
removeManagedShims(
373+
in: shimDirectory,
374+
expectedShimBinary: binDirectory.appendingPathComponent("agentic-secrets-shim")
375+
)
376+
removeDirectoryIfEmpty(shimDirectory)
374377

375378
for executable in Self.installExecutables {
376379
try? fileManager.removeItem(at: binDirectory.appendingPathComponent(executable))
@@ -394,18 +397,22 @@ struct LocalBrokerStatusController: BrokerStatusControlling {
394397
removeEmptyDirectories(from: URL(fileURLWithPath: plan.prefixPath, isDirectory: true))
395398
}
396399

397-
private func removeManagedShims(in shimDirectory: URL, appDestination: URL) {
400+
private func removeManagedShims(in shimDirectory: URL, expectedShimBinary: URL) {
398401
let fileManager = FileManager.default
399402
guard let entries = try? fileManager.contentsOfDirectory(at: shimDirectory, includingPropertiesForKeys: [.isSymbolicLinkKey]) else { return }
400-
let expectedTargets = Set([
401-
appDestination.appendingPathComponent("Contents/MacOS/agentic-secrets-shim").path,
402-
appDestination.appendingPathComponent("Contents/MacOS/agentic-secrets-shim").standardizedFileURL.path
403-
])
404-
for entry in entries where expectedTargets.contains(entry.resolvingSymlinksInPath().path) {
403+
for entry in entries where CommandShimInstallationInspector.pointsToShimBinary(shimURL: entry, expectedShimBinary: expectedShimBinary) {
405404
try? fileManager.removeItem(at: entry)
406405
}
407406
}
408407

408+
private func removeDirectoryIfEmpty(_ directory: URL) {
409+
let fileManager = FileManager.default
410+
guard let entries = try? fileManager.contentsOfDirectory(atPath: directory.path), entries.isEmpty else {
411+
return
412+
}
413+
try? fileManager.removeItem(at: directory)
414+
}
415+
409416
private func removeManagedAppBundle(at appURL: URL, legacyAppDestination: URL) throws {
410417
let fileManager = FileManager.default
411418
guard fileManager.fileExists(atPath: appURL.path) || (try? fileManager.destinationOfSymbolicLink(atPath: appURL.path)) != nil else {
@@ -536,15 +543,18 @@ struct LocalBrokerStatusController: BrokerStatusControlling {
536543
private func daemonUnavailableReason(_ error: Error) -> DaemonUnavailableReason {
537544
let description = String(describing: error)
538545
let plan = makeInstallPlan()
539-
if !plan.currentAppIsInstalledCopy && FileManager.default.fileExists(atPath: plan.appDestinationPath) {
540-
return .wrongAppCopy
541-
}
542546
if description.contains("No such file or directory") || description.contains("connect") {
543547
return FileManager.default.fileExists(atPath: plan.launchAgentPath) ? .notRunning : .notInstalled
544548
}
545549
if description.contains("unauthorizedPeer") || description.contains("wrongHash") || description.contains("wrongPath") || description.contains("wrongCDHash") {
550+
if !plan.currentAppIsInstalledCopy && FileManager.default.fileExists(atPath: plan.appDestinationPath) {
551+
return .wrongAppCopy
552+
}
546553
return .manifestMismatch
547554
}
555+
if !plan.currentAppIsInstalledCopy && FileManager.default.fileExists(atPath: plan.appDestinationPath) {
556+
return .wrongAppCopy
557+
}
548558
return .unreachable
549559
}
550560

@@ -666,8 +676,15 @@ enum ShellConfigurationCleaner {
666676
guard index + 1 < lines.count else { return nil }
667677
let marker = lines[index].trimmingCharacters(in: .whitespaces)
668678
guard marker == "# Agentic Secrets PATH" || marker == "# AgenticSecrets CLI shims" else { return nil }
669-
guard lines[index + 1].trimmingCharacters(in: .whitespaces) == #"case ":$PATH:" in"# else { return nil }
670-
var cursor = index + 2
679+
let caseIndex: Int
680+
if lines[index + 1].trimmingCharacters(in: .whitespaces).hasPrefix("agentic_secrets_path_dir=") {
681+
caseIndex = index + 2
682+
} else {
683+
caseIndex = index + 1
684+
}
685+
guard caseIndex < lines.count,
686+
lines[caseIndex].trimmingCharacters(in: .whitespaces) == #"case ":$PATH:" in"# else { return nil }
687+
var cursor = caseIndex + 1
671688
while cursor < lines.count {
672689
if lines[cursor].trimmingCharacters(in: .whitespaces) == "esac" {
673690
return cursor

Sources/App/Services/ControlPlaneClient.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ struct IPCControlPlaneClient: ControlPlaneClient {
181181
return nil
182182
}
183183
let prefix = URL(fileURLWithPath: "/" + components[1..<applicationsIndex].joined(separator: "/"), isDirectory: true)
184-
return isLegacyLocalInstallPrefix(prefix) ? prefix : nil
184+
guard prefix.path != "/" else {
185+
return nil
186+
}
187+
return isLegacyLocalInstallPrefix(prefix) || bundle.lastPathComponent == "AgenticSecrets.app" ? prefix : nil
185188
}
186189

187190
static func defaultInstallPrefix() -> URL {

Sources/App/Services/UISmokeRunner.swift

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ enum UISmokeRunner {
2626
try await testEmptyState()
2727
try await testSettingsLayout()
2828
try await testRegisterWizardValidation()
29+
try testCLIShimInstallerEnvironment()
30+
try testCLIShimStatusPresentation()
2931
try testManagementEditorValidation()
3032
try testPasteboardCopy()
3133
try testCommandPolicyDraft()
@@ -200,6 +202,45 @@ enum UISmokeRunner {
200202
]).isEmpty, "whitespace-only secret values are omitted from registration payload")
201203
}
202204

205+
private static func testCLIShimInstallerEnvironment() throws {
206+
let prefix = URL(fileURLWithPath: "/tmp/agentic-secrets-custom-prefix", isDirectory: true)
207+
let environment = CLIShimInstaller.subprocessEnvironment(
208+
base: ["PATH": "/usr/bin"],
209+
installPrefix: prefix
210+
)
211+
try expect(environment["AGENTIC_SECRETS_INSTALL_PREFIX"] == prefix.path, "shim installer subprocess receives the app install prefix")
212+
try expect(environment["AGENTIC_SECRETS_SHIM_DIR"] == prefix.appendingPathComponent("shims", isDirectory: true).path, "shim installer subprocess uses the matching prefix shims directory")
213+
try expect(environment["AGENTIC_SECRETS_SHIM_BINARY"] == prefix.appendingPathComponent("bin/agentic-secrets-shim").path, "shim installer subprocess uses the matching prefix shim helper")
214+
try expect(environment["PATH"] == "/usr/bin", "shim installer subprocess preserves unrelated environment")
215+
}
216+
217+
private static func testCLIShimStatusPresentation() throws {
218+
let installed = CLIShimStatusPresentation(rawValue: "installed")
219+
try expect(installed.label == "Installed", "installed shim status has a human label")
220+
try expect(installed.canInstall, "installed shim can be repaired")
221+
try expect(installed.canRemove, "installed shim can be removed")
222+
223+
let missing = CLIShimStatusPresentation(rawValue: "not installed")
224+
try expect(missing.label == "Not Installed", "missing shim status has a human label")
225+
try expect(missing.canInstall, "missing shim can be installed")
226+
try expect(!missing.canRemove, "missing shim cannot be removed")
227+
228+
let blocked = CLIShimStatusPresentation(rawValue: "blocked")
229+
try expect(blocked.label == "Blocked", "blocked shim status has a human label")
230+
try expect(!blocked.canInstall, "blocked shim install action is disabled until the path is reviewed")
231+
try expect(!blocked.canRemove, "blocked shim remove action is disabled because the path is not verified as managed")
232+
233+
let helperUnavailable = CLIShimStatusPresentation(rawValue: "helper unavailable")
234+
try expect(helperUnavailable.label == "Helper Unavailable", "helper-unavailable shim status has a human label")
235+
try expect(!helperUnavailable.canInstall, "helper-unavailable shim cannot be repaired through per-CLI install")
236+
try expect(!helperUnavailable.canRemove, "helper-unavailable shim cannot be removed as a verified managed shim")
237+
238+
let unknown = CLIShimStatusPresentation(rawValue: "unexpected")
239+
try expect(unknown.label == "Unknown", "unexpected shim status falls back to Unknown")
240+
try expect(!unknown.canInstall, "unknown shim status does not enable install")
241+
try expect(!unknown.canRemove, "unknown shim status does not enable remove")
242+
}
243+
203244
private static func testManagementEditorValidation() throws {
204245
try expect(APISessionProfileEditorDefaults.pathPrefixes == "/v1/", "proxy add flow has a safe default path prefix")
205246
try expect(APISessionProfileEditorDefaults.methods == "GET, POST", "proxy add flow has default HTTP methods")
@@ -397,6 +438,9 @@ enum UISmokeRunner {
397438
store.brokerInstallPlan = plan
398439
store.brokerStatus.message = "Open the installed copy so the authenticated IPC manifest matches the running UI."
399440
try expect(store.bestDaemonAction == .openInstalledApp, "wrong app copy state highlights opening the installed copy")
441+
store.brokerStatus.message = "Local daemon is installed but not running."
442+
try expect(store.bestDaemonAction != .openInstalledApp, "daemon-down state does not over-prioritize opening the installed copy")
443+
store.brokerStatus.message = "Open the installed copy so the authenticated IPC manifest matches the running UI."
400444
try verifyHostingLayout(
401445
LocalStateUnavailableView(store: store),
402446
width: 680,
@@ -734,6 +778,13 @@ enum UISmokeRunner {
734778
*) export PATH="\(binDir):$PATH" ;;
735779
esac
736780
781+
# AgenticSecrets CLI shims
782+
agentic_secrets_path_dir='\(shimDir)'
783+
case ":$PATH:" in
784+
*":$agentic_secrets_path_dir:"*) ;;
785+
*) export PATH="$agentic_secrets_path_dir:$PATH" ;;
786+
esac
787+
737788
# AgenticSecrets CLI shims
738789
case ":$PATH:" in
739790
*":\(shimDir):"*) ;;

Sources/App/Stores/ControlPlaneStore.swift

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,9 @@ final class ControlPlaneStore {
225225
guard let plan = brokerInstallPlan else {
226226
return brokerStatus.canRepair ? .restart : .check
227227
}
228-
if !plan.currentAppIsInstalledCopy && FileManager.default.fileExists(atPath: plan.appDestinationPath) {
228+
if brokerStatus.message.contains("Open the installed copy"),
229+
!plan.currentAppIsInstalledCopy,
230+
FileManager.default.fileExists(atPath: plan.appDestinationPath) {
229231
return .openInstalledApp
230232
}
231233
if plan.canInstall {
@@ -438,11 +440,7 @@ final class ControlPlaneStore {
438440

439441
func refreshAfterActivation() async {
440442
guard !isLoading else { return }
441-
if snapshot == nil {
442-
await refresh()
443-
} else {
444-
await checkDaemon()
445-
}
443+
await refresh()
446444
}
447445

448446
func repairDaemon() async {
@@ -570,8 +568,12 @@ final class ControlPlaneStore {
570568
successMessage = "CLI registered"
571569
errorMessage = nil
572570
}
573-
snapshot = try await client.loadSnapshot()
574-
maintainSelections()
571+
do {
572+
snapshot = try await loadSnapshotWithTransientRetry()
573+
maintainSelections()
574+
} catch {
575+
errorMessage = "\(successMessage ?? "CLI registered"), but the updated local state could not be refreshed. \(localStateLoadError(error))"
576+
}
575577
return true
576578
} catch {
577579
errorMessage = userFacingError(error)
@@ -890,7 +892,10 @@ final class ControlPlaneStore {
890892
errorMessage = nil
891893
try? await Task.sleep(for: .milliseconds(350))
892894
brokerStatus = await brokerController.status()
893-
guard brokerStatus.state == .healthy else { return }
895+
guard brokerStatus.state == .healthy else {
896+
snapshot = nil
897+
return
898+
}
894899
do {
895900
try await loadSnapshotAfterHealthyStatus()
896901
errorMessage = nil

Sources/App/Support/CLIShimInstaller.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ enum CLIShimInstaller {
1313
let process = Process()
1414
process.executableURL = URL(fileURLWithPath: try cliPath())
1515
process.arguments = arguments
16+
process.environment = subprocessEnvironment()
1617

1718
let output = Pipe()
1819
let errors = Pipe()
@@ -48,6 +49,19 @@ enum CLIShimInstaller {
4849
}
4950
throw CLIShimInstallerError.missingCLI
5051
}
52+
53+
static func subprocessEnvironment(
54+
base: [String: String] = ProcessInfo.processInfo.environment,
55+
installPrefix: URL? = IPCControlPlaneClient.installPrefixFromBundle()
56+
) -> [String: String] {
57+
var environment = base
58+
if let installPrefix {
59+
environment["AGENTIC_SECRETS_INSTALL_PREFIX"] = installPrefix.path
60+
environment["AGENTIC_SECRETS_SHIM_DIR"] = installPrefix.appendingPathComponent("shims", isDirectory: true).path
61+
environment["AGENTIC_SECRETS_SHIM_BINARY"] = installPrefix.appendingPathComponent("bin/agentic-secrets-shim").path
62+
}
63+
return environment
64+
}
5165
}
5266

5367
enum CLIShimInstallerError: Error, CustomStringConvertible {

Sources/App/Views/ManagementViews.swift

Lines changed: 101 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,12 @@ private struct CLIRegistrationDetail: View {
142142
var body: some View {
143143
ScrollView {
144144
if let cli = store.selectedCLIRegistration {
145+
let shimStatus = CLIShimStatusPresentation(rawValue: cli.shimStatus)
145146
VStack(alignment: .leading, spacing: 18) {
146147
header(cli.name, subtitle: cli.targetPath)
147148
HStack {
148149
StatusBadge(text: cli.trustStatus, systemImage: "checkmark.shield")
149-
StatusBadge(text: cli.shimStatus, systemImage: "link")
150+
StatusBadge(text: shimStatus.label, systemImage: "link")
150151
}
151152
Form {
152153
Section("Environment bindings") {
@@ -175,23 +176,23 @@ private struct CLIRegistrationDetail: View {
175176
}
176177
}
177178
Section("Shim") {
178-
LabeledContent("Status", value: cli.shimStatus)
179+
LabeledContent("Status", value: shimStatus.label)
179180
HStack {
180181
Button {
181182
Task { await store.installShim(for: cli.name) }
182183
} label: {
183-
Label(cli.shimStatus == "installed" ? "Repair Shim" : "Install Shim", systemImage: "link")
184+
Label(shimStatus.primaryActionTitle, systemImage: shimStatus.primaryActionSystemImage)
184185
}
185186
.buttonStyle(.borderedProminent)
186-
.disabled(!store.canManageBrokerState)
187+
.disabled(!store.canManageBrokerState || !shimStatus.canInstall)
187188
.help("Install or repair the local command shim for this CLI")
188189
Button("Remove Shim", role: .destructive) {
189190
pendingShimRemoval = cli
190191
}
191-
.disabled(!store.canManageBrokerState)
192+
.disabled(!store.canManageBrokerState || !shimStatus.canRemove)
192193
.help("Remove the local command shim for this CLI")
193194
}
194-
Text("A shim routes normal \(cli.name) invocations through Agentic Secrets when the local shims folder is before the native CLI on PATH.")
195+
Text(shimStatus.guidance(commandName: cli.name))
195196
.font(.caption)
196197
.foregroundStyle(.secondary)
197198
}
@@ -253,6 +254,100 @@ private struct CLIRegistrationDetail: View {
253254
}
254255
}
255256

257+
enum CLIShimStatusPresentation: Equatable {
258+
case installed
259+
case notInstalled
260+
case blocked
261+
case helperUnavailable
262+
case unknown(String)
263+
264+
init(rawValue: String) {
265+
switch rawValue {
266+
case "installed":
267+
self = .installed
268+
case "not installed":
269+
self = .notInstalled
270+
case "blocked":
271+
self = .blocked
272+
case "helper unavailable":
273+
self = .helperUnavailable
274+
default:
275+
self = .unknown(rawValue)
276+
}
277+
}
278+
279+
var label: String {
280+
switch self {
281+
case .installed:
282+
"Installed"
283+
case .notInstalled:
284+
"Not Installed"
285+
case .blocked:
286+
"Blocked"
287+
case .helperUnavailable:
288+
"Helper Unavailable"
289+
case .unknown:
290+
"Unknown"
291+
}
292+
}
293+
294+
var canInstall: Bool {
295+
switch self {
296+
case .installed, .notInstalled:
297+
true
298+
case .blocked, .helperUnavailable, .unknown:
299+
false
300+
}
301+
}
302+
303+
var canRemove: Bool {
304+
self == .installed
305+
}
306+
307+
var primaryActionTitle: String {
308+
switch self {
309+
case .installed:
310+
"Repair Shim"
311+
case .notInstalled:
312+
"Install Shim"
313+
case .blocked:
314+
"Shim Blocked"
315+
case .helperUnavailable:
316+
"Repair Daemon"
317+
case .unknown:
318+
"Check Daemon"
319+
}
320+
}
321+
322+
var primaryActionSystemImage: String {
323+
switch self {
324+
case .helperUnavailable:
325+
"wrench.and.screwdriver"
326+
case .unknown:
327+
"questionmark.circle"
328+
case .blocked:
329+
"exclamationmark.triangle"
330+
case .installed, .notInstalled:
331+
"link"
332+
}
333+
}
334+
335+
func guidance(commandName: String) -> String {
336+
switch self {
337+
case .installed:
338+
"Normal \(commandName) invocations use Agentic Secrets when the local shims folder is before the native CLI on PATH."
339+
case .notInstalled:
340+
"Install the local command shim to route normal \(commandName) invocations through Agentic Secrets."
341+
case .blocked:
342+
"A non-Agentic Secrets path already exists where this command shim should be. Review that path before replacing it."
343+
case .helperUnavailable:
344+
"The Agentic Secrets shim helper is missing or does not match the local install manifest. Repair the local daemon install."
345+
case .unknown:
346+
"The local daemon did not report enough install information to verify this command shim. Check the daemon status."
347+
}
348+
}
349+
}
350+
256351
private struct NoCLIRegistrationsView: View {
257352
@Bindable var store: ControlPlaneStore
258353

0 commit comments

Comments
 (0)