Design pr 1

[eonist]

CodeAnt-AI Description

Redesign the popover to show live stats, clearer progress, and more history

What Changed

• The popover now shows live CPU, memory, and disk usage in the header, with quick access to Settings, sign in, and Close
• Action history shows more runs at once, supports loading 10 more items, and keeps child jobs visible under their parent action
• Run and job rows now use pie-style progress indicators and clearer status labels, including partial progress for active work
• Job, step, and settings views keep a fixed size while open to avoid popover jumps during updates
• Local runner setup now blocks install paths outside the user’s home folder, and local runner scanning is limited to known install locations

Impact

✅ More action history visible at once
✅ Clearer CI progress at a glance
✅ Fewer popover jumps while data refreshes

🔄 Retrigger CodeAnt AI Review

Details

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

[github-actions]

🐙 Octopus posted 6 inline findings. See the pinned Octopus summary comment for the full review.

[github-actions]

🟠 makeActiveJob passes pre-parsed String to JobStep but JobStep expects String — ISO parsing silently dropped

In the refactored makeActiveJob, step startedAt/completedAt are passed directly as the raw String? values from JobPayload (step.startedAt, step.completedAt) instead of being parsed through the ISO8601DateFormatter. JobStep.startedAt/completedAt are typed as Date? (visible in ActiveJob.swift), so the compiler would reject a String?. However, if JobStep was also changed to store String? in this PR, then PieProgressView and elapsed-time calculations that depend on Date arithmetic will silently receive nil or incorrect values whenever ISO parsing would have been needed. Verify that JobStep's stored type matches what is passed here — if JobStep stores Date?, the compiler would catch this, but if it was also changed to String? the date-dependent UI features break silently.

Suggested change

	createdAt: payload.createdAt.flatMap { iso.date(from: $0) },
	startedAt: step.startedAt.flatMap { iso.date(from: $0) },
	completedAt: step.completedAt.flatMap { iso.date(from: $0) }

AI Fix Prompt

Fix the following High (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 308-315:

Problem: In the refactored makeActiveJob, step startedAt/completedAt are passed directly as the raw String? values from JobPayload (step.startedAt, step.completedAt) instead of being parsed through the ISO8601DateFormatter. JobStep.startedAt/completedAt are typed as Date? (visible in ActiveJob.swift), so the compiler would reject a String?. However, if JobStep was also changed to store String? in this PR, then PieProgressView and elapsed-time calculations that depend on Date arithmetic will silently receive nil or incorrect values whenever ISO parsing would have been needed. Verify that JobStep's stored type matches what is passed here — if JobStep stores Date?, the compiler would catch this, but if it was also changed to String? the date-dependent UI features break silently.

Suggested fix:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

[github-actions]

🟡 ActionRowView.actionDotColor and actionStatusColor use group.conclusion but ActionGroup has no .conclusion property

actionStatusLabel(for:) and actionStatusColor(for:) both switch on group.conclusion, but the ActionGroup struct visible in the codebase context has no .conclusion property — group status is derived from runs[].conclusion. If this compiles, it implies a .conclusion property was added elsewhere in this PR but is not visible in the diff; if not, this is a compile error. Verify ActionGroup exposes a .conclusion: String? that aggregates run conclusions, otherwise the switch will always fall through to 'default'.

Suggested change

	private struct ActionRowView: View {
	// In ActionGroup.swift, ensure this property exists:
	var conclusion: String? {
	guard groupStatus == .completed else { return nil }
	if runs.allSatisfy({ $0.conclusion == "success" }) { return "success" }
	if runs.contains(where: { $0.conclusion == "failure" }) { return "failure" }
	if runs.allSatisfy({ $0.conclusion == "cancelled" }) { return "cancelled" }
	return nil
	}

AI Fix Prompt

Fix the following Medium (Bug) issue in `Sources/RunnerBar/PopoverMainView.swift` at lines 232-244:

Problem: actionStatusLabel(for:) and actionStatusColor(for:) both switch on group.conclusion, but the ActionGroup struct visible in the codebase context has no .conclusion property — group status is derived from runs[].conclusion. If this compiles, it implies a .conclusion property was added elsewhere in this PR but is not visible in the diff; if not, this is a compile error. Verify ActionGroup exposes a .conclusion: String? that aggregates run conclusions, otherwise the switch will always fall through to 'default'.

Suggested fix:
// In ActionGroup.swift, ensure this property exists:
var conclusion: String? {
    guard groupStatus == .completed else { return nil }
    if runs.allSatisfy({ $0.conclusion == "success" }) { return "success" }
    if runs.contains(where: { $0.conclusion == "failure" }) { return "failure" }
    if runs.allSatisfy({ $0.conclusion == "cancelled" }) { return "cancelled" }
    return nil
}

[github-actions]

🟡 RelativeTimeFormatter: 172_800-second boundary gives '48h ago' instead of '2d ago' for exactly 2 days

The switch uses ..<172_800 (48 hours) as the boundary for hours vs days. A value of exactly 172_800 seconds falls into the default 'days' case and returns '2d ago', which is correct. But values from 86_400 (24h) to 172_799 (47h59m) return hours — so '25h ago', '36h ago', '47h ago' are all valid outputs. Most relative-time formatters switch to days after 24h, not 48h. This is a product decision, but worth a deliberate comment if 48h is intentional.

Suggested change

	}
	// Change the days boundary to 86_400 (24 h) if day-level granularity
	// is preferred after the first day:
	case ..<86_400: return "\(Int(seconds / 3_600))h ago"
	default: return "\(Int(seconds / 86_400))d ago"

AI Fix Prompt

Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/RelativeTimeFormatter.swift` at lines 27-30:

Problem: The switch uses ..<172_800 (48 hours) as the boundary for hours vs days. A value of exactly 172_800 seconds falls into the default 'days' case and returns '2d ago', which is correct. But values from 86_400 (24h) to 172_799 (47h59m) return hours — so '25h ago', '36h ago', '47h ago' are all valid outputs. Most relative-time formatters switch to days after 24h, not 48h. This is a product decision, but worth a deliberate comment if 48h is intentional.

Suggested fix:
// Change the days boundary to 86_400 (24 h) if day-level granularity
// is preferred after the first day:
case ..<86_400:   return "\(Int(seconds / 3_600))h ago"
default:          return "\(Int(seconds / 86_400))d ago"

[github-actions]

💡 InlineJobRowView hardcodes .yellow for PieProgressView color regardless of job conclusion

InlineJobRowView always passes color: .yellow to PieProgressView, which is correct because inProgressJobs is pre-filtered to status == 'in_progress'. However this is non-obvious to future readers. A brief comment or using a named constant would clarify intent.

Suggested change

	}
	// These rows are pre-filtered to in_progress only (see ActionRowView.inProgressJobs)
	PieProgressView(
	progress: job.progressFraction,
	color: .yellow, // always in_progress at this call site
	size: 7
	)

AI Fix Prompt

Fix the following Nit (Style) issue in `Sources/RunnerBar/PopoverMainView.swift` at lines 338-343:

Problem: InlineJobRowView always passes color: .yellow to PieProgressView, which is correct because inProgressJobs is pre-filtered to status == 'in_progress'. However this is non-obvious to future readers. A brief comment or using a named constant would clarify intent.

Suggested fix:
// These rows are pre-filtered to in_progress only (see ActionRowView.inProgressJobs)
PieProgressView(
    progress: job.progressFraction,
    color: .yellow, // always in_progress at this call site
    size: 7
)

[github-actions]

💡 Spacer() comments removed — load-bearing nature is no longer documented

The PR removes the '⚠️ load-bearing' annotation from Spacer() in ActionDetailView and ActionRowView. These Spacers are still functionally required (they push elapsed/chevron to the trailing edge). Removing the comment is fine under a 'less noise' policy, but the PopoverMainView regression guard explicitly calls out 'RULE 4: Job row HStack Spacer() is LOAD-BEARING', so the two files are now inconsistent in documentation style.

Suggested change

	Spacer()
	Spacer() // pushes trailing controls to right edge

AI Fix Prompt

Fix the following Nit (Style) issue in `Sources/RunnerBar/ActionDetailView.swift` at line 36:

Problem: The PR removes the '⚠️ load-bearing' annotation from Spacer() in ActionDetailView and ActionRowView. These Spacers are still functionally required (they push elapsed/chevron to the trailing edge). Removing the comment is fine under a 'less noise' policy, but the PopoverMainView regression guard explicitly calls out 'RULE 4: Job row HStack Spacer() is LOAD-BEARING', so the two files are now inconsistent in documentation style.

Suggested fix:
Spacer() // pushes trailing controls to right edge

[github-actions]

💡 swiftlint:disable missing_docs removed from ActionGroup.swift but public-facing functions in GitHub.swift lost their doc-comments

The PR removes missing_docs from the ActionGroup.swift swiftlint disable list (good), implying docs will now be enforced there. Simultaneously, GitHub.swift removes several doc-comment blocks from internal functions (fetchUserOrgs, fetchUserRepos, fetchRegistrationToken). With missing_docs: warning for internal symbols, these removals will generate SwiftLint warnings on CI.

Suggested change

	func fetchUserRepos() -> [String] {
	/// Returns the login names of all organisations the authenticated user belongs to.
	/// Calls `GET /user/orgs` with pagination. Returns an empty array on error.
	func fetchUserOrgs() -> [String] {

AI Fix Prompt

Fix the following Nit (Style) issue in `Sources/RunnerBar/GitHub.swift` at lines 214-225:

Problem: The PR removes missing_docs from the ActionGroup.swift swiftlint disable list (good), implying docs will now be enforced there. Simultaneously, GitHub.swift removes several doc-comment blocks from internal functions (fetchUserOrgs, fetchUserRepos, fetchRegistrationToken). With missing_docs: warning for internal symbols, these removals will generate SwiftLint warnings on CI.

Suggested fix:
/// Returns the login names of all organisations the authenticated user belongs to.
/// Calls `GET /user/orgs` with pagination. Returns an empty array on error.
func fetchUserOrgs() -> [String] {

[github-actions]

🐙 Octopus posted 4 inline findings. See the pinned Octopus summary comment for the full review.

[github-actions]

🟠 JobStep date fields silently changed from String? to an unverified type

In makeActiveJob(), the step construction now passes step.startedAt and step.completedAt directly (without the .flatMap { iso.date(from: $0) } conversion that was present for job-level fields). This implies JobStep.startedAt and JobStep.completedAt were changed from String? to Date? (or similar) elsewhere in ActiveJob.swift. If JobPayload's nested step fields are still String? (which is the typical Decodable shape for ISO 8601 timestamps from the GitHub API), these assignments will silently store raw strings in Date fields or fail to compile — one of which is a silent data corruption risk. The diff for JobStep in ActiveJob.swift does not show its stored-property types changing, making this a cross-file consistency concern that warrants explicit confirmation.

Suggested change

	createdAt: payload.createdAt.flatMap { iso.date(from: $0) },
	// Verify JobStep.startedAt / completedAt are Date? before removing the iso.date(from:) conversion.
	// If they remain String?, restore:
	startedAt: step.startedAt.flatMap { iso.date(from: $0) },
	completedAt: step.completedAt.flatMap { iso.date(from: $0) }

AI Fix Prompt

Fix the following High (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 310-315:

Problem: In makeActiveJob(), the step construction now passes step.startedAt and step.completedAt directly (without the .flatMap { iso.date(from: $0) } conversion that was present for job-level fields). This implies JobStep.startedAt and JobStep.completedAt were changed from String? to Date? (or similar) elsewhere in ActiveJob.swift. If JobPayload's nested step fields are still String? (which is the typical Decodable shape for ISO 8601 timestamps from the GitHub API), these assignments will silently store raw strings in Date fields or fail to compile — one of which is a silent data corruption risk. The diff for JobStep in ActiveJob.swift does not show its stored-property types changing, making this a cross-file consistency concern that warrants explicit confirmation.

Suggested fix:
// Verify JobStep.startedAt / completedAt are Date? before removing the iso.date(from:) conversion.
// If they remain String?, restore:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

[github-actions]

🟠 AppDelegate marked @unchecked Sendable without documented invariants

AppDelegate is now marked @unchecked Sendable, but its mutable properties (statusItem, popover, hostingController, savedNavState, popoverIsOpen) are not documented as being accessed exclusively from the main thread, and the class is not annotated @mainactor. The view factory methods are individually @mainactor, but togglePopover() is @mainactor @objc while popoverDidClose() and the RunnerStore.onChange closure (which now dispatches reload() via DispatchQueue.main.async) are not explicitly @mainactor. This is a fragile contract: future contributors can add background-thread access without a compile-time warning. @unchecked Sendable suppresses exactly the diagnostic that would catch this.

Suggested change

	private var savedNavState: NavState?
	// Option A: annotate the class itself
	@MainActor
	final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {
	// removes need for @unchecked Sendable and per-method @MainActor
	}
	// Option B: if full @MainActor is not feasible, add a comment block:
	// THREAD SAFETY: all stored properties are accessed exclusively on the main thread.
	// @unchecked Sendable is safe only under this invariant.

AI Fix Prompt

Fix the following High (Race Condition) issue in `Sources/RunnerBar/AppDelegate.swift` at line 54:

Problem: AppDelegate is now marked @unchecked Sendable, but its mutable properties (statusItem, popover, hostingController, savedNavState, popoverIsOpen) are not documented as being accessed exclusively from the main thread, and the class is not annotated @MainActor. The view factory methods are individually @MainActor, but togglePopover() is @MainActor @objc while popoverDidClose() and the RunnerStore.onChange closure (which now dispatches reload() via DispatchQueue.main.async) are not explicitly @MainActor. This is a fragile contract: future contributors can add background-thread access without a compile-time warning. @unchecked Sendable suppresses exactly the diagnostic that would catch this.

Suggested fix:
// Option A: annotate the class itself
@MainActor
final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {
    // removes need for @unchecked Sendable and per-method @MainActor
}

// Option B: if full @MainActor is not feasible, add a comment block:
// THREAD SAFETY: all stored properties are accessed exclusively on the main thread.
// @unchecked Sendable is safe only under this invariant.

[github-actions]

🟡 RelativeTimeFormatter 172_800s boundary skips the '1 day ago' case

The switch ladder uses 172_800 (48 hours) as the upper bound for the hours bucket, meaning events between 24h and 48h are rendered as '24h ago', '25h ago', etc. instead of '1d ago', '2d ago'. Most users expect '1d ago' for anything over 24 hours. The 48-hour threshold was likely chosen to avoid '1d ago' appearing too soon, but the current boundary means the hours label can display values >= 24 which looks odd ('36h ago'). Consider 86_400 (24h) as the hours/days boundary.

Suggested change

	default: return "\(Int(seconds / 86_400))d ago"
	switch seconds {
	case ..<60: return "just now"
	case ..<3_600: return "\(Int(seconds / 60))m ago"
	case ..<86_400: return "\(Int(seconds / 3_600))h ago"
	default: return "\(Int(seconds / 86_400))d ago"
	}

AI Fix Prompt

Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/RelativeTimeFormatter.swift` at lines 27-29:

Problem: The switch ladder uses 172_800 (48 hours) as the upper bound for the hours bucket, meaning events between 24h and 48h are rendered as '24h ago', '25h ago', etc. instead of '1d ago', '2d ago'. Most users expect '1d ago' for anything over 24 hours. The 48-hour threshold was likely chosen to avoid '1d ago' appearing too soon, but the current boundary means the hours label can display values >= 24 which looks odd ('36h ago'). Consider 86_400 (24h) as the hours/days boundary.

Suggested fix:
switch seconds {
case ..<60:      return "just now"
case ..<3_600:   return "\(Int(seconds / 60))m ago"
case ..<86_400:  return "\(Int(seconds / 3_600))h ago"
default:         return "\(Int(seconds / 86_400))d ago"
}

[github-actions]

💡 enrichGroupJobs() is public but has no doc comment after missing_docs was re-enabled

ActionGroup.swift removed missing_docs from the swiftlint:disable list, meaning doc comments are now enforced. enrichGroupJobs() is func (internal visibility) and the swiftlint config warns on internal. It currently has no /// doc comment, which will trigger a SwiftLint warning under the missing_docs opt-in rule.

Suggested change

	func enrichGroupJobs(_ jobs: [ActiveJob], jobCache: [Int: ActiveJob]) -> [ActiveJob] {
	/// Merges richer step data from `jobCache` into `jobs` where available.
	/// Replaces step arrays for jobs that are in-progress or have no steps yet.
	/// - Parameters:
	/// - jobs: The flat job list to enrich.
	/// - jobCache: The completed-job cache keyed by job ID.
	/// - Returns: A new array with step data backfilled where possible.
	func enrichGroupJobs(_ jobs: [ActiveJob], jobCache: [Int: ActiveJob]) -> [ActiveJob] {

AI Fix Prompt

Fix the following Nit (Style) issue in `Sources/RunnerBar/RunnerStoreState.swift` at lines 161-163:

Problem: ActionGroup.swift removed missing_docs from the swiftlint:disable list, meaning doc comments are now enforced. enrichGroupJobs() is func (internal visibility) and the swiftlint config warns on internal. It currently has no /// doc comment, which will trigger a SwiftLint warning under the missing_docs opt-in rule.

Suggested fix:
/// Merges richer step data from `jobCache` into `jobs` where available.
/// Replaces step arrays for jobs that are in-progress or have no steps yet.
/// - Parameters:
///   - jobs: The flat job list to enrich.
///   - jobCache: The completed-job cache keyed by job ID.
/// - Returns: A new array with step data backfilled where possible.
func enrichGroupJobs(_ jobs: [ActiveJob], jobCache: [Int: ActiveJob]) -> [ActiveJob] {

[github-actions]

🐙 Octopus posted 5 inline findings. See the pinned Octopus summary comment for the full review.

[github-actions]

🟠 AppDelegate marked @unchecked Sendable without enforcing the contract

AppDelegate adopts @unchecked Sendable but its mutable state (statusItem, popover, hostingController, savedNavState, popoverIsOpen) is accessed from @MainActor-annotated methods AND from @objc callbacks that run on the main thread by convention — but the compiler no longer enforces this. The RunnerStore.shared.onChange closure (set in applicationDidFinishLaunching) reads self.popoverIsOpen and calls self.observable.reload() without a DispatchQueue.main.async guard in the closure body itself for the popoverIsOpen read, even though a DispatchQueue.main.async wrapper is added for reload(). If onChange is ever triggered from a background thread in RunnerStore, popoverIsOpen is read without synchronization. Prefer confining AppDelegate to @MainActor explicitly rather than @unchecked Sendable.

Suggested change

	case actionJobDetail(ActiveJob, ActionGroup)
	@MainActor
	final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {

AI Fix Prompt

Fix the following High (Bug) issue in `Sources/RunnerBar/AppDelegate.swift` at line 55:

Problem: AppDelegate adopts `@unchecked Sendable` but its mutable state (`statusItem`, `popover`, `hostingController`, `savedNavState`, `popoverIsOpen`) is accessed from `@MainActor`-annotated methods AND from `@objc` callbacks that run on the main thread by convention — but the compiler no longer enforces this. The `RunnerStore.shared.onChange` closure (set in `applicationDidFinishLaunching`) reads `self.popoverIsOpen` and calls `self.observable.reload()` without a `DispatchQueue.main.async` guard in the closure body itself for the `popoverIsOpen` read, even though a `DispatchQueue.main.async` wrapper is added for `reload()`. If `onChange` is ever triggered from a background thread in `RunnerStore`, `popoverIsOpen` is read without synchronization. Prefer confining AppDelegate to `@MainActor` explicitly rather than `@unchecked Sendable`.

Suggested fix:
@MainActor
final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {

[github-actions]

🟡 RelativeTimeFormatter 172_800s boundary skips 1-day display

The switch ranges are: <60 → "just now", <3_600 → "Xm ago", <172_800 → "Xh ago", else → "Xd ago". The 172_800 threshold is 48 hours, so anything from 25h to 47h returns e.g. "25h ago" or "47h ago" rather than "1d ago" or "2d ago". This is a UX inconsistency — most relative formatters switch to days at 24h. The boundary should be 86_400 (24h) to switch to days after one day.

Suggested change

	}
	case ..<60: return "just now"
	case ..<3_600: return "\(Int(seconds / 60))m ago"
	case ..<86_400: return "\(Int(seconds / 3_600))h ago"
	default: return "\(Int(seconds / 86_400))d ago"

AI Fix Prompt

Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/RelativeTimeFormatter.swift` at lines 27-30:

Problem: The `switch` ranges are: `<60` → "just now", `<3_600` → "Xm ago", `<172_800` → "Xh ago", else → "Xd ago". The `172_800` threshold is 48 hours, so anything from 25h to 47h returns e.g. "25h ago" or "47h ago" rather than "1d ago" or "2d ago". This is a UX inconsistency — most relative formatters switch to days at 24h. The boundary should be `86_400` (24h) to switch to days after one day.

Suggested fix:
case ..<60:      return "just now"
case ..<3_600:   return "\(Int(seconds / 60))m ago"
case ..<86_400:  return "\(Int(seconds / 3_600))h ago"
default:         return "\(Int(seconds / 86_400))d ago"

[github-actions]

🟡 ActionGroup.progressFraction uses groupStatus but ActionGroup.Equatable ignores it

progressFraction delegates to groupStatus which is derived from runs[].status. However, ActionGroup.Equatable only compares id, isDimmed, jobs, and run IDs — not runs[].status or runs[].conclusion. This means if a run transitions from in_progress to completed without a change in job list or run IDs, == returns true and SwiftUI may skip re-rendering the PieProgressView even though progressFraction has changed. Consider including runs.map({ $0.status }) in the equality check.

Suggested change

	&& lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
	static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
	lhs.id == rhs.id
	&& lhs.isDimmed == rhs.isDimmed
	&& lhs.jobs == rhs.jobs
	&& lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
	&& lhs.runs.map({ $0.status }) == rhs.runs.map({ $0.status })
	&& lhs.runs.map({ $0.conclusion }) == rhs.runs.map({ $0.conclusion })
	}

AI Fix Prompt

Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 172-183:

Problem: `progressFraction` delegates to `groupStatus` which is derived from `runs[].status`. However, `ActionGroup.Equatable` only compares `id`, `isDimmed`, `jobs`, and `run IDs` — not `runs[].status` or `runs[].conclusion`. This means if a run transitions from `in_progress` to `completed` without a change in job list or run IDs, `==` returns `true` and SwiftUI may skip re-rendering the `PieProgressView` even though `progressFraction` has changed. Consider including `runs.map({ $0.status })` in the equality check.

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
        && lhs.runs.map({ $0.status }) == rhs.runs.map({ $0.status })
        && lhs.runs.map({ $0.conclusion }) == rhs.runs.map({ $0.conclusion })
}

[github-actions]

🔵 "DO NOT REMOVE THIS COMMENT" pragma-style directives are brittle

The comment block contains multiple sentences of the form "If you are an agent or human, DO NOT REMOVE THIS COMMENT, YOU ARE NOT ALLOWED UNDER ANY CIRCUMSTANCE." This pattern appears three times in the diff (AppDelegate.swift, PopoverMainView.swift twice). While the intent — preventing regression — is good, this style is more emphatic than informative. The technical rationale immediately before these sentences is excellent; the directive itself adds noise and may cause developers to distrust the preceding explanation. Consider replacing the mandate with a test or a CI lint check that verifies the property exists, or at minimum reformulate as "⚠️ Removing this causes regression #375/#376/#377 — see status-bar-app-position-warning.md §4."

Suggested change

	case jobDetail(ActiveJob)
	// ⚠️ Removing `isPopoverOpen` causes position-jump regression — see #375 #376 #377
	// and status-bar-app-position-warning.md §4 (Architecture 1).

AI Fix Prompt

Fix the following Low (Style) issue in `Sources/RunnerBar/AppDelegate.swift` at lines 48-52:

Problem: The comment block contains multiple sentences of the form "If you are an agent or human, DO NOT REMOVE THIS COMMENT, YOU ARE NOT ALLOWED UNDER ANY CIRCUMSTANCE." This pattern appears three times in the diff (AppDelegate.swift, PopoverMainView.swift twice). While the intent — preventing regression — is good, this style is more emphatic than informative. The technical rationale immediately before these sentences is excellent; the directive itself adds noise and may cause developers to distrust the preceding explanation. Consider replacing the mandate with a test or a CI lint check that verifies the property exists, or at minimum reformulate as "⚠️ Removing this causes regression #375/#376/#377 — see status-bar-app-position-warning.md §4."

Suggested fix:
// ⚠️ Removing `isPopoverOpen` causes position-jump regression — see #375 #376 #377
// and status-bar-app-position-warning.md §4 (Architecture 1).

[github-actions]

🔵 ActionRunsResponse and PRRef are now internal (not private) — widens API surface

In the original ActionGroup.swift these types were private. Extracting them to a new file without marking them internal (the default) or private makes them visible to the entire module. RunPayload, HeadCommit, and PRRef are raw API decoding types with no business value outside fetchActionGroups. Marking them private in the new file is not possible since they're module-level, but they can be marked internal explicitly or — better — moved into an enum namespace (e.g. enum GitHub.API {}) to signal their limited purpose. The SwiftLint missing_docs rule is now unenforced for this file because ActionGroup.swift removed missing_docs from the swiftlint:disable list but ActionRunsResponse.swift doesn't have a disable directive, so the rule will fire on these internal types.

Suggested change

	}
	// At minimum, add a file-level disable to silence the enforced missing_docs rule,
	// or add doc comments to each declaration.
	// swiftlint:disable:next missing_docs
	struct RunPayload: Codable {

AI Fix Prompt

Fix the following Low (Architecture) issue in `Sources/RunnerBar/ActionRunsResponse.swift` at lines 6-11:

Problem: In the original `ActionGroup.swift` these types were `private`. Extracting them to a new file without marking them `internal` (the default) or `private` makes them visible to the entire module. `RunPayload`, `HeadCommit`, and `PRRef` are raw API decoding types with no business value outside `fetchActionGroups`. Marking them `private` in the new file is not possible since they're module-level, but they can be marked `internal` explicitly or — better — moved into an `enum` namespace (e.g. `enum GitHub.API {}`) to signal their limited purpose. The SwiftLint `missing_docs` rule is now unenforced for this file because `ActionGroup.swift` removed `missing_docs` from the swiftlint:disable list but `ActionRunsResponse.swift` doesn't have a disable directive, so the rule will fire on these `internal` types.

Suggested fix:
// At minimum, add a file-level disable to silence the enforced missing_docs rule,
// or add doc comments to each declaration.
// swiftlint:disable:next missing_docs
struct RunPayload: Codable {

[codeant-ai]

CodeAnt AI is running Incremental review

[codeant-ai]

CodeAnt AI Incremental review completed.

[codeant-ai]

CodeAnt AI is running Incremental review

[codeant-ai]

CodeAnt AI Incremental review completed.

[codeant-ai]

CodeAnt AI is running Incremental review

[codeant-ai]

CodeAnt AI Incremental review completed.

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.7% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

🐙 Octopus posted 2 inline findings. See the pinned Octopus summary comment for the full review.

[github-actions]

🟡 ActionGroup.Equatable ignores firstJobStartedAt / lastJobCompletedAt — stale elapsed display possible

The Equatable implementation compares id, isDimmed, jobs, and runs.map(.id). It does not include firstJobStartedAt or lastJobCompletedAt. If a group's start/end timestamps change (e.g. the first job starts after the group was initially enqueued) but the job list and run IDs are otherwise the same, SwiftUI's onChange(of: store.actions) will not fire and the elapsed/startedAgo display will not update. The timestamps are derived from job data, so they will typically change together with jobs — but the Equatable contract is weaker than intended.

Suggested change

	static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
	lhs.id == rhs.id
	&& lhs.isDimmed == rhs.isDimmed
	&& lhs.jobs == rhs.jobs
	&& lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
	&& lhs.firstJobStartedAt == rhs.firstJobStartedAt
	&& lhs.lastJobCompletedAt == rhs.lastJobCompletedAt
	}

AI Fix Prompt

Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 180-186:

Problem: The Equatable implementation compares id, isDimmed, jobs, and runs.map(\.id). It does not include firstJobStartedAt or lastJobCompletedAt. If a group's start/end timestamps change (e.g. the first job starts after the group was initially enqueued) but the job list and run IDs are otherwise the same, SwiftUI's onChange(of: store.actions) will not fire and the elapsed/startedAgo display will not update. The timestamps are derived from job data, so they will typically change together with jobs — but the Equatable contract is weaker than intended.

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
        && lhs.firstJobStartedAt == rhs.firstJobStartedAt
        && lhs.lastJobCompletedAt == rhs.lastJobCompletedAt
}

[github-actions]

🔵 RelativeTimeFormatter '3d ago' threshold uses 172_800 (48h) not a true day boundary

The switch case ..<172_800 (48 hours) is used as the boundary between hours and days. The doc comment says '2 hours ago → 2h ago / 3 days ago → 3d ago', implying the intent is a 24-hour (86_400s) boundary. With the current threshold, a run from 47 hours ago shows '47h ago' rather than '1d ago'. This is a minor UX inconsistency — recommend aligning the threshold with the documented examples.

Suggested change

	}
	case ..<86_400: return "\(Int(seconds / 3_600))h ago"
	default: return "\(Int(seconds / 86_400))d ago"

AI Fix Prompt

Fix the following Low (Logic Error) issue in `Sources/RunnerBar/RelativeTimeFormatter.swift` at lines 27-30:

Problem: The switch case ..<172_800 (48 hours) is used as the boundary between hours and days. The doc comment says '2 hours ago → 2h ago / 3 days ago → 3d ago', implying the intent is a 24-hour (86_400s) boundary. With the current threshold, a run from 47 hours ago shows '47h ago' rather than '1d ago'. This is a minor UX inconsistency — recommend aligning the threshold with the documented examples.

Suggested fix:
case ..<86_400:   return "\(Int(seconds / 3_600))h ago"
default:          return "\(Int(seconds / 86_400))d ago"