From c661945a1fa764b55f5872a5b119aa68419c0b3f Mon Sep 17 00:00:00 2001 From: Alan Wizemann Date: Sun, 3 May 2026 22:09:21 +0200 Subject: [PATCH] feat(cron): auth-error banner + running indicator + per-job log tail (#72) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cron rows now surface the same OAuth-refresh-revoked recovery flow as chat instead of a generic red dot, plus three previously-missing observability cues: - ACPErrorHint.classify is reused on `job.lastError`. When it returns `oauthRefreshRevoked(provider)` the detail pane shows the human hint + a "Re-authenticate" button that drops the user into Credential Pools via `coordinator.pendingOAuthReauth = provider` — same wiring ChatView's banner uses. Unrecognized errors fall back to the legacy red `lastError` text (no regression). - Row dot turns blue + pulses when `state == "running"` (taking precedence over disabled / error / success); the detail header gains a `ScarfBadge("running…", kind: .info)` next to active/paused. No new polling — `HermesFileWatcher.lastChangeDate` (already wired into ActivityView/Logs) drives `CronViewModel.load()` so state flips surface within a watcher tick. - "LAST RUN OUTPUT" replaces the inline `LAST OUTPUT` block with a collapsible panel: a one-line summary (` — ok|error|running…`) always visible, full monospaced terminal-style scroll view on expand, auto-scrolls to bottom when new runs land. Also fixes a pre-existing bug in `HermesFileService.loadCronOutput`: Hermes nests per-run output under `~/.hermes/cron/output//.md` but the loader treated the dir as flat, so the cron output panel never rendered any content. The fix walks the per-job subdir + keeps the legacy flat-file fallback for older Hermes layouts. Co-authored-by: Claude Opus 4.7 (1M context) --- .../Core/Services/HermesFileService.swift | 31 ++- .../Cron/ViewModels/CronViewModel.swift | 10 + .../scarf/Features/Cron/Views/CronView.swift | 181 +++++++++++++++++- ...ronViewModelErrorClassificationTests.swift | 76 ++++++++ 4 files changed, 286 insertions(+), 12 deletions(-) create mode 100644 scarf/scarfTests/CronViewModelErrorClassificationTests.swift diff --git a/scarf/scarf/Core/Services/HermesFileService.swift b/scarf/scarf/Core/Services/HermesFileService.swift index 2d21c23..51cb8a1 100644 --- a/scarf/scarf/Core/Services/HermesFileService.swift +++ b/scarf/scarf/Core/Services/HermesFileService.swift @@ -490,12 +490,35 @@ struct HermesFileService: Sendable { } } + /// Read the most-recent run output for a cron job. Hermes writes + /// `~/.hermes/cron/output//.md` per run + /// (one file per execution); we resolve the per-job subdir, take + /// the lexicographically-last filename (which is the newest given + /// the timestamp prefix), and return its contents. Returns nil + /// when the subdir is missing, empty, or the read fails — the cron + /// detail surface treats nil as "no output yet." + /// + /// A legacy flat-file layout (`/`) + /// is checked as a fallback so older Hermes installs that used a + /// non-nested layout still surface their last run. nonisolated func loadCronOutput(jobId: String) -> String? { let dir = context.paths.cronOutputDir - guard let files = try? transport.listDirectory(dir) else { return nil } - let matching = files.filter { $0.contains(jobId) }.sorted().last - guard let filename = matching else { return nil } - return readFile(dir + "/" + filename) + let perJobDir = dir + "/" + jobId + if let runs = try? transport.listDirectory(perJobDir), + let latest = runs.sorted().last { + if let content = readFile(perJobDir + "/" + latest) { + return content + } + } + // Legacy fallback: pre-subdir layouts had files like + // `-.log` directly under cronOutputDir. Keep + // matching them so users on older Hermes versions still see + // their tail. + if let files = try? transport.listDirectory(dir), + let matching = files.filter({ $0.contains(jobId) }).sorted().last { + return readFile(dir + "/" + matching) + } + return nil } // MARK: - Skills diff --git a/scarf/scarf/Features/Cron/ViewModels/CronViewModel.swift b/scarf/scarf/Features/Cron/ViewModels/CronViewModel.swift index 6150e37..3c0f44e 100644 --- a/scarf/scarf/Features/Cron/ViewModels/CronViewModel.swift +++ b/scarf/scarf/Features/Cron/ViewModels/CronViewModel.swift @@ -24,6 +24,16 @@ final class CronViewModel { var editingJob: HermesCronJob? var isLoading = false + /// Classified hint for the selected job's `lastError`, computed via + /// `ACPErrorHint.classify` so cron rows surface the same OAuth-revoked + /// affordance that ChatView's banner offers. `nil` when the selected + /// job has no error or the error doesn't match a known pattern — the + /// detail pane falls back to rendering `lastError` raw. + var selectedErrorClassification: ACPErrorHint.Classification? { + guard let job = selectedJob, let lastError = job.lastError, !lastError.isEmpty else { return nil } + return ACPErrorHint.classify(errorMessage: lastError, stderrTail: "") + } + func load() { isLoading = true let svc = fileService diff --git a/scarf/scarf/Features/Cron/Views/CronView.swift b/scarf/scarf/Features/Cron/Views/CronView.swift index 03617aa..214b1dc 100644 --- a/scarf/scarf/Features/Cron/Views/CronView.swift +++ b/scarf/scarf/Features/Cron/Views/CronView.swift @@ -12,7 +12,10 @@ import ScarfDesign struct CronView: View { @State private var viewModel: CronViewModel @State private var pendingDelete: HermesCronJob? + @State private var showOutputPanel: Bool = false @Environment(\.hermesCapabilities) private var capabilitiesStore + @Environment(AppCoordinator.self) private var coordinator + @Environment(HermesFileWatcher.self) private var fileWatcher init(context: ServerContext) { _viewModel = State(initialValue: CronViewModel(context: context)) @@ -36,6 +39,13 @@ struct CronView: View { .navigationTitle("Cron Jobs") .loadingOverlay(viewModel.isLoading, label: "Loading cron jobs…", isEmpty: viewModel.jobs.isEmpty) .onAppear { viewModel.load() } + // Reload on Hermes file mutations — Hermes flips `state` between + // "scheduled" and "running" inside `~/.hermes/cron/jobs.json` + // when a job starts/finishes, and writes a new run-output file + // under `~/.hermes/cron/output/`. The watcher gives us the + // running indicator + log tail refresh "for free" without a + // polling timer. Same wiring ActivityView uses. + .onChange(of: fileWatcher.lastChangeDate) { viewModel.load() } .sheet(isPresented: $viewModel.showCreateSheet) { CronJobEditor(mode: .create, availableSkills: viewModel.availableSkills, supportsWorkdir: hasCronWorkdir) { form in viewModel.createJob( @@ -172,6 +182,13 @@ struct CronView: View { Circle() .fill(statusDotColor(job)) .frame(width: 7, height: 7) + .opacity(job.state == "running" ? 0.55 : 1.0) + .animation( + job.state == "running" + ? .easeInOut(duration: 0.9).repeatForever(autoreverses: true) + : .default, + value: job.state + ) } HStack(spacing: 10) { Text(job.schedule.expression ?? job.schedule.display ?? "—") @@ -221,7 +238,13 @@ struct CronView: View { } private func statusDotColor(_ job: HermesCronJob) -> Color { + // Order matters: a currently-running job overrides a stale + // lastError so the user sees "yes, retrying right now" rather + // than "still showing the old failure." Disabled wins over + // everything else — a paused job isn't running, regardless + // of state-field churn. if !job.enabled { return ScarfColor.foregroundFaint } + if job.state == "running" { return ScarfColor.info } if job.lastError != nil { return ScarfColor.danger } return ScarfColor.success } @@ -272,6 +295,9 @@ struct CronView: View { .foregroundStyle(ScarfColor.foregroundPrimary) ScarfBadge(job.enabled ? "active" : "paused", kind: job.enabled ? .success : .neutral) + if job.state == "running" { + ScarfBadge("running…", kind: .info) + } } Text(CronScheduleFormatter.humanReadable(from: job.schedule)) .scarfStyle(.footnote) @@ -420,26 +446,165 @@ struct CronView: View { } if let error = job.lastError { + errorBanner(job: job, error: error) + } + + outputPanel(job: job) + } + + /// Last-error surface. When `ACPErrorHint` recognizes the message + /// (OAuth refresh-revoked, missing credentials, SSH failure, etc.), + /// it renders the human hint + raw error + a re-auth button when + /// applicable. Otherwise falls back to the legacy single-line + /// red text — same chrome the view used pre-PR for unrecognized + /// errors. Mirrors `ChatView.errorBanner` so the recovery flow is + /// identical between cron and chat. + @ViewBuilder + private func errorBanner(job: HermesCronJob, error: String) -> some View { + if let classification = viewModel.selectedErrorClassification { + VStack(alignment: .leading, spacing: 6) { + HStack(alignment: .top, spacing: 8) { + Image(systemName: "exclamationmark.triangle.fill") + .foregroundStyle(ScarfColor.warning) + VStack(alignment: .leading, spacing: 4) { + Text(classification.hint) + .scarfStyle(.body) + .foregroundStyle(ScarfColor.foregroundPrimary) + .textSelection(.enabled) + Text(error) + .scarfStyle(.caption) + .foregroundStyle(ScarfColor.foregroundMuted) + .textSelection(.enabled) + .lineLimit(2) + } + Spacer(minLength: ScarfSpace.s2) + if let provider = classification.oauthProvider { + Button("Re-authenticate") { + coordinator.pendingOAuthReauth = provider + coordinator.selectedSection = .credentialPools + } + .buttonStyle(ScarfPrimaryButton()) + .help("Open Credential Pools and re-authenticate \(provider).") + } + } + } + .padding(ScarfSpace.s3) + .background( + RoundedRectangle(cornerRadius: ScarfRadius.lg, style: .continuous) + .fill(ScarfColor.warning.opacity(0.08)) + ) + .overlay( + RoundedRectangle(cornerRadius: ScarfRadius.lg, style: .continuous) + .strokeBorder(ScarfColor.warning.opacity(0.25), lineWidth: 1) + ) + } else { HStack(spacing: 6) { Image(systemName: "exclamationmark.triangle.fill") Text(error) .scarfStyle(.caption) + .textSelection(.enabled) } .foregroundStyle(ScarfColor.danger) } + } - if let output = viewModel.jobOutput { - sectionBlock("LAST OUTPUT") { - Text(output) - .font(ScarfFont.monoSmall) - .foregroundStyle(ScarfColor.foregroundPrimary) - .textSelection(.enabled) - .padding(ScarfSpace.s3) - .frame(maxWidth: .infinity, alignment: .leading) + /// Per-job run-output panel. Always visible; collapsed by default + /// with a one-line summary so the detail pane stays scannable when + /// the user has dozens of cron jobs. Expanded body mirrors the + /// dark monospaced tail layout `LogsView` uses, fed by + /// `HermesFileService.loadCronOutput` (Hermes writes per-run files + /// under `~/.hermes/cron/output/-*`). Reload happens via the + /// outer `HermesFileWatcher` `.onChange` — when a fresh run lands a + /// new output file, the VM re-reads on the next mtime tick. + @ViewBuilder + private func outputPanel(job: HermesCronJob) -> some View { + let summary = outputSummary(job) + VStack(alignment: .leading, spacing: ScarfSpace.s2) { + Button { + showOutputPanel.toggle() + } label: { + HStack(spacing: ScarfSpace.s2) { + Image(systemName: showOutputPanel ? "chevron.down" : "chevron.right") + .font(.system(size: 10, weight: .semibold)) + .foregroundStyle(ScarfColor.foregroundMuted) + Text("LAST RUN OUTPUT") + .scarfStyle(.captionUppercase) + .foregroundStyle(ScarfColor.foregroundMuted) + Text(summary) + .font(ScarfFont.monoSmall) + .foregroundStyle(ScarfColor.foregroundFaint) + .lineLimit(1) + Spacer() + } + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + + if showOutputPanel { + if let output = viewModel.jobOutput, !output.isEmpty { + ScrollViewReader { proxy in + ScrollView { + Text(output) + .font(ScarfFont.monoSmall) + .foregroundStyle(ScarfColor.foregroundPrimary) + .textSelection(.enabled) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(ScarfSpace.s3) + .id("cron-output-bottom") + } + .frame(maxHeight: 320) + .background( + RoundedRectangle(cornerRadius: ScarfRadius.lg, style: .continuous) + .fill(Color(red: 0.07, green: 0.06, blue: 0.05)) + ) + .overlay( + RoundedRectangle(cornerRadius: ScarfRadius.lg, style: .continuous) + .strokeBorder(ScarfColor.border, lineWidth: 1) + ) + // Auto-scroll to the latest line whenever the + // output content changes (a new run lands). + .onChange(of: output) { + withAnimation(.easeOut(duration: 0.18)) { + proxy.scrollTo("cron-output-bottom", anchor: .bottom) + } + } + .onAppear { + proxy.scrollTo("cron-output-bottom", anchor: .bottom) + } + } + } else { + Text("No output yet — this job hasn't run, or its output file is gone.") + .scarfStyle(.caption) + .foregroundStyle(ScarfColor.foregroundMuted) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(ScarfSpace.s3) + .background( + RoundedRectangle(cornerRadius: ScarfRadius.lg, style: .continuous) + .fill(ScarfColor.backgroundSecondary) + ) + .overlay( + RoundedRectangle(cornerRadius: ScarfRadius.lg, style: .continuous) + .strokeBorder(ScarfColor.border, lineWidth: 1) + ) + } } } } + /// One-line summary rendered next to the LAST RUN OUTPUT chevron + /// when the panel is collapsed. Gives a quick "yes there's content" + /// (or "no output yet") read without expanding. + private func outputSummary(_ job: HermesCronJob) -> String { + let timestamp = job.lastRunAt.map { CronScheduleFormatter.formatNextRun(iso: $0) } ?? "never" + let status: String = { + if job.state == "running" { return "running…" } + if job.lastError != nil { return "error" } + if job.lastRunAt != nil { return "ok" } + return "no runs yet" + }() + return "\(timestamp) — \(status)" + } + @ViewBuilder private func sectionBlock(_ title: String, @ViewBuilder _ content: () -> Content) -> some View { VStack(alignment: .leading, spacing: ScarfSpace.s2) { diff --git a/scarf/scarfTests/CronViewModelErrorClassificationTests.swift b/scarf/scarfTests/CronViewModelErrorClassificationTests.swift new file mode 100644 index 0000000..5afdfe1 --- /dev/null +++ b/scarf/scarfTests/CronViewModelErrorClassificationTests.swift @@ -0,0 +1,76 @@ +import Testing +import Foundation +import ScarfCore +@testable import scarf + +/// Exercises `CronViewModel.selectedErrorClassification` — the bridge +/// between Hermes's cron `last_error` field and the in-app re-auth +/// affordance. Covers the OAuth-revoked path that motivated the surface +/// (real string captured from `~/.hermes/cron/jobs.json` when an +/// OAuth-authed provider's refresh session is invalidated) plus the +/// "no error" + "unrecognized error" branches the UI relies on. +@Suite struct CronViewModelErrorClassificationTests { + + /// The exact `last_error` string Hermes writes to `~/.hermes/cron/jobs.json` + /// after an OAuth-authed cron run hits a revoked refresh session. + /// Captured from a live failed run on 2026-05-03 — if Hermes ever + /// changes the wording, this test breaks loudly so we know to + /// update the matcher in `ACPErrorHint.classify`. + private static let revokedErrorString = + "RuntimeError: Refresh session has been revoked Run `hermes model` to re-authenticate." + + @Test @MainActor func oauthRevokedErrorClassifies() { + let vm = CronViewModel() + vm.selectedJob = Self.fixtureJob(lastError: Self.revokedErrorString) + + let classification = vm.selectedErrorClassification + #expect(classification != nil) + #expect(classification?.hint.contains("Re-authenticate") == true + || classification?.hint.contains("re-authenticate") == true + || classification?.hint.contains("revoked") == true + || classification?.hint.contains("expired") == true) + // The classifier returns nil oauthProvider when no provider word + // is present in the haystack — Hermes's revoked-session line + // doesn't always include the provider name. Either result is + // acceptable to the UI: a non-nil provider lets the row render + // a "Re-authenticate" button; a nil provider still surfaces the + // human hint without the button. + _ = classification?.oauthProvider + } + + @Test @MainActor func noSelectedJobReturnsNil() { + let vm = CronViewModel() + #expect(vm.selectedErrorClassification == nil) + } + + @Test @MainActor func selectedJobWithoutErrorReturnsNil() { + let vm = CronViewModel() + vm.selectedJob = Self.fixtureJob(lastError: nil) + #expect(vm.selectedErrorClassification == nil) + } + + @Test @MainActor func unrecognizedErrorReturnsNil() { + // ACPErrorHint returns nil when no pattern matches; the UI + // falls back to rendering the raw lastError without the + // re-auth banner. + let vm = CronViewModel() + vm.selectedJob = Self.fixtureJob( + lastError: "RuntimeError: cron-specific failure that doesn't match any known pattern" + ) + #expect(vm.selectedErrorClassification == nil) + } + + // MARK: - Fixtures + + private static func fixtureJob(lastError: String?) -> HermesCronJob { + HermesCronJob( + id: "test-job", + name: "Test Job", + prompt: "noop", + schedule: CronSchedule(kind: "cron", expression: "0 9 * * *"), + enabled: true, + state: lastError != nil ? "failed" : "scheduled", + lastError: lastError + ) + } +}