feat(fleet): integrate device auth, enrollment, and live runner management - #320
Open
KafuChino123 wants to merge 26 commits into
Open
feat(fleet): integrate device auth, enrollment, and live runner management#320KafuChino123 wants to merge 26 commits into
KafuChino123 wants to merge 26 commits into
Conversation
Add a FleetControlClient Swift package with generated protobuf and gRPC stubs for the fleet agent local control API. Wire the local package into the ArcBox app target so phase 1 desktop integration can import the generated client types.
Wrap the generated fleet control stubs in a high-level client for lifecycle, state watch, and settings RPCs. Map proto responses into desktop-facing models while preserving optional settings update presence semantics, and cover the mapping behavior with tests.
Add the local fleet control client to SwiftUI environment values and app scene injection. Start the fleet control transport outside daemon startup, log through the fleet category, and close it during app termination.
Drive fleet agent state from the local control watch stream. Add lifecycle actions, settings updates, reconnect backoff, and user-readable error handling for the runners dashboard.
Consume the signed-in OIDC session to list Platform workspaces and issue workspace-scoped Fleet enrollment tokens. Wire token issuance into local Fleet Agent enrollment and configure the Platform endpoint for app and CI builds.
Replace RUN-9 sample data and stub actions with FleetViewModel watch snapshots, workspace enrollment, and local drain/resume controls. Expose the runner section in release builds and cover the presentation-state mapping.
Let ArcBoxTests consume FleetControlClient through the hosted ArcBox target, avoiding Xcode’s duplicate dynamic gRPC package graph. Handle runner item-selection deep links explicitly so navigation remains exhaustive.
- coordinate authenticated token handoff with local Agent state - keep Fleet watch and client transport app-scoped - reconcile unknown outcomes and graceful termination
On macOS 26, any state change inside a fixedSize(vertical: true) subtree triggers a window-sizing pass that resizes the window — or the NavigationSplitView content when the window cannot grow — to the screen's visible-frame height, sliding the sidebar under the title bar and pushing the account button off-screen. Drop the modifier from the dynamic error label (it wraps identically without it) and document the pitfall in AGENTS.md.
Replace the OIDC Authorization Code + PKCE flow with the Better Auth device-authorization grant: request a device code, open the approval page in the default browser, and poll the token endpoint per RFC 8628. The polled access token is an opaque Better Auth session token with a sliding server-side expiry, sent as the platform API bearer — the token the platform actually accepts, unblocking fleet enrollment token issuance. Identity now comes from the provider session endpoint instead of OIDC userinfo/ID-token claims, and sign-out revokes the session server-side. The Keychain store self-heals by clearing pre-device-flow token blobs. PKCE, discovery, code exchange, token refresh, and the custom-scheme OAuth callback (including its DeepLinkRouter leg) are gone; build configuration keys are unchanged. The Account pane and runner section show the confirmation code with reopen-browser and cancel affordances while approval is pending.
KafuChino123
force-pushed
the
feat/abxd-99-device-auth
branch
from
July 24, 2026 08:46
c3f4c5a to
97e5cde
Compare
📝 WalkthroughWalkthroughThis change replaces browser-based OIDC authentication with Better Auth device authorization and adds Fleet platform/local-agent clients, enrollment orchestration, runner monitoring, VM image preparation, Fleet settings, application wiring, and associated tests. ChangesFleet authentication and runner integration
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
KafuChino123
marked this pull request as ready for review
July 24, 2026 09:05
Greptile SummaryAdds end-to-end Fleet runner integration to ArcBox Desktop.
Confidence Score: 4/5The PR needs a fix before merging because detached Fleet Agent snapshots bypass the required enrollment-recovery state. Detached snapshots are rendered as enrolled hosts even though reconciliation locks enrollment and records a terminal detached failure, hiding the dedicated start-over flow. Files Needing Attention: ArcBox/ViewModels/RunnersViewModel.swift
|
| Filename | Overview |
|---|---|
| ArcBox/ViewModels/RunnersViewModel.swift | Composes Fleet snapshots and enrollment coordination into runner UI state, but detached snapshots bypass the terminal recovery state. |
| ArcBox/ViewModels/Fleet/FleetEnrollmentCoordinator.swift | Orchestrates enrollment and reconciles Fleet Agent snapshots, including terminal post-handoff failures. |
| Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swift | Maintains device-flow authentication state and Keychain-backed session restoration. |
| Packages/FleetControlClient/Sources/FleetControlClient/FleetControlClient.swift | Adds the local Fleet Agent gRPC interface for state watching and runner management. |
| Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift | Adds authenticated workspace discovery and enrollment-token requests. |
Sequence Diagram
sequenceDiagram
participant User
participant Desktop as ArcBox Desktop
participant Platform as Fleet Platform
participant Agent as Fleet Agent
participant Daemon as arcbox-daemon
User->>Desktop: Sign in with device authorization
Desktop->>Platform: Discover workspaces
Desktop->>Platform: Issue enrollment token
Desktop->>Agent: Enroll local machine
Agent->>Platform: Attach runner
Agent-->>Desktop: Stream runner state
User->>Desktop: Drain, resume, or configure runner
Desktop->>Agent: Fleet control RPC
Agent->>Daemon: Manage runner VM lifecycle
Reviews (2): Last reviewed commit: "fix(auth): isolate device flow from HTTP..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+251
to
+258
| switch snapshot.enrollment { | ||
| case .attaching, .attached, .updating, .detached, .credentialRejected: | ||
| guard normalizedMachineID(snapshot.machineID) != nil else { | ||
| return .failed("Fleet Agent reported an invalid machine identity.") | ||
| } | ||
| return .enrolled( | ||
| RunnerHostViewModel(snapshot: snapshot, agentInfo: agentInfo), | ||
| freshness: hostFreshness(loadState: loadState) |
There was a problem hiding this comment.
When the Fleet Agent reports .credentialRejected with a machine ID, this branch returns .enrolled instead of the coordinator's enrollment-failure state, causing the runner screen to show unusable controls and hide the "Unenroll and Start Over" recovery flow.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
ArcBox/App/AppDelegate.swift (1)
50-68: 🩺 Stability & Availability | 🔵 TrivialVerify the cumulative termination grace period against macOS quit/shutdown behavior.
prepareForTerminationdefaults to a 65s grace andshutdownadds up to 5s (plusdisableDaemon), all under.terminateLater. For a manual Quit this can leave the app appearing unresponsive for over a minute; during a system logout/restart/shutdown the OS terminate watchdog may force-kill the process well before enrollment drain finishes, so the drain/graceful-shutdown guarantee won't hold in that path. Consider a shorter or context-aware grace period (e.g. shorter on logout/shutdown) and confirm the chosen value is acceptable UX.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ArcBox/App/AppDelegate.swift` around lines 50 - 68, Review the termination flow in the AppDelegate Task and make its cumulative grace period compatible with macOS quit and system logout/restart/shutdown watchdog limits. Use a shorter or context-aware timeout for prepareForTermination, shutdown, and disableDaemon as appropriate, while preserving graceful cleanup and warning behavior; ensure manual Quit does not appear unresponsive and system termination does not rely on an unrealistically long enrollment drain.Packages/FleetControlClient/generate.sh (1)
27-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: drop the redundant
echo/command-substitution (SC2005).♻️ Optional
for dir in "${candidates[@]}"; do if [ -d "$dir" ]; then - echo "$(cd "$dir" && pwd)" + (cd "$dir" && pwd) return 0 fi done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/FleetControlClient/generate.sh` around lines 27 - 32, Update the candidate-directory success branch in the loop to return the resolved directory path directly, removing the redundant echo and command substitution while preserving the existing absolute-path resolution and return behavior.Source: Linters/SAST tools
Packages/FleetControlClient/Sources/FleetControlClient/FleetControlClient.swift (1)
182-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCancellation handling differs from
prepareImages.Here you use
catch where Task.isCancelled(note the double space), whereasprepareImages(Line 220) usescatch is CancellationError. Both finish the stream cleanly, but aligning the two makes the intent uniform.♻️ Optional consistency tweak
- } catch where Task.isCancelled { + } catch is CancellationError { continuation.finish() } catch { continuation.finish(throwing: error) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/FleetControlClient/Sources/FleetControlClient/FleetControlClient.swift` around lines 182 - 186, Align the cancellation handling in the stream’s catch block with prepareImages: catch CancellationError explicitly and finish the continuation without throwing. Preserve the existing generic error path that calls continuation.finish(throwing: error), and remove the Task.isCancelled-based condition.ArcBox/Models/RunnerHostCapability.swift (1)
6-17: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: cache the chip name once.
chipNameis machine-constant but is recomputed (twosysctlbynamecalls) on every access — andRunnerHostViewModel.initreads it per watch snapshot. Astatic letcomputes it once and keeps call sites identical.♻️ Optional refactor
- static var chipName: String { + static let chipName: String = { var size = 0 guard sysctlbyname("machdep.cpu.brand_string", nil, &size, nil, 0) == 0, size > 0 else { return "Apple Silicon" } var brand = [CChar](repeating: 0, count: size) guard sysctlbyname("machdep.cpu.brand_string", &brand, &size, nil, 0) == 0 else { return "Apple Silicon" } let bytes = brand.prefix(while: { $0 != 0 }).map { UInt8(bitPattern: $0) } return String(bytes: bytes, encoding: .utf8) ?? "Apple Silicon" - } + }()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ArcBox/Models/RunnerHostCapability.swift` around lines 6 - 17, Change RunnerHostCapability.chipName from a computed static property to a static let initialized with the existing sysctl-based lookup logic, so the chip name is calculated only once while preserving the current fallback behavior and call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ArcBox/Views/Settings/AccountSettingsView.swift`:
- Around line 21-24: Update the view’s .task modifier around
authSession.refreshSession() to key execution to authSession.status, ensuring it
reruns when the status transitions from .restoring to .signedIn. Preserve the
existing refreshSession call and behavior for already signed-in sessions.
In `@Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession`+SignIn.swift:
- Around line 12-24: The signIn flow does not synchronously mark an active
sign-in before awaiting performDeviceSignIn(), allowing duplicate flows. Update
signIn() to set status to .signingIn before creating or awaiting the task, or
guard signInTask == nil, while preserving the existing restoring/signingIn
checks and task cleanup.
In
`@Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift`:
- Around line 1-2: Reorder imports in
Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift
(lines 1-2) so Foundation precedes ArcBoxAuth. In
Packages/FleetPlatformClient/Tests/FleetPlatformClientTests/FleetPlatformClientTests.swift
(lines 1-5), order imports as Foundation, Testing, ArcBoxAuth, with `@testable`
import FleetPlatformClient last.
In `@project.yml`:
- Around line 146-147: Add FleetControlClient and FleetPlatformClient to the
ArcBoxTests dependency declarations in project.yml, alongside its existing
ArcBox and K8sClient packages, so the test target can resolve both direct
imports.
---
Nitpick comments:
In `@ArcBox/App/AppDelegate.swift`:
- Around line 50-68: Review the termination flow in the AppDelegate Task and
make its cumulative grace period compatible with macOS quit and system
logout/restart/shutdown watchdog limits. Use a shorter or context-aware timeout
for prepareForTermination, shutdown, and disableDaemon as appropriate, while
preserving graceful cleanup and warning behavior; ensure manual Quit does not
appear unresponsive and system termination does not rely on an unrealistically
long enrollment drain.
In `@ArcBox/Models/RunnerHostCapability.swift`:
- Around line 6-17: Change RunnerHostCapability.chipName from a computed static
property to a static let initialized with the existing sysctl-based lookup
logic, so the chip name is calculated only once while preserving the current
fallback behavior and call sites.
In `@Packages/FleetControlClient/generate.sh`:
- Around line 27-32: Update the candidate-directory success branch in the loop
to return the resolved directory path directly, removing the redundant echo and
command substitution while preserving the existing absolute-path resolution and
return behavior.
In
`@Packages/FleetControlClient/Sources/FleetControlClient/FleetControlClient.swift`:
- Around line 182-186: Align the cancellation handling in the stream’s catch
block with prepareImages: catch CancellationError explicitly and finish the
continuation without throwing. Preserve the existing generic error path that
calls continuation.finish(throwing: error), and remove the
Task.isCancelled-based condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84f3a2c6-8a8f-4b60-a0d2-199254c33ee4
⛔ Files ignored due to path filters (4)
ArcBox.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedis excluded by!**/Package.resolvedPackages/FleetControlClient/Package.resolvedis excluded by!**/Package.resolvedPackages/FleetControlClient/Sources/FleetControlClient/Generated/arcbox/fleet/control/v1/control.grpc.swiftis excluded by!**/generated/**Packages/FleetControlClient/Sources/FleetControlClient/Generated/arcbox/fleet/control/v1/control.pb.swiftis excluded by!**/generated/**
📒 Files selected for processing (90)
.github/workflows/pr.yml.github/workflows/release.yml.swiftlint.ymlAGENTS.mdArcBox.xcodeproj/project.pbxprojArcBox/App/AppDelegate.swiftArcBox/App/DeepLinkRouter.swiftArcBox/App/EnvironmentValues+Clients.swiftArcBox/ArcBoxApp.swiftArcBox/Components/EmptyStateView.swiftArcBox/Components/StatusBadge.swiftArcBox/Info.plistArcBox/Logging.swiftArcBox/Models/FleetRunnerImageReadiness.swiftArcBox/Models/NavItem.swiftArcBox/Models/RunnerHostCapability.swiftArcBox/Models/RunnerHostStatus.swiftArcBox/Models/RunnerHostViewModel.swiftArcBox/Models/RunnersViewState.swiftArcBox/Services/FleetAgentConnection.swiftArcBox/ViewModels/Fleet/FleetControlServicing.swiftArcBox/ViewModels/Fleet/FleetEnrollmentCoordinator.swiftArcBox/ViewModels/Fleet/FleetViewModel.swiftArcBox/ViewModels/RunnersViewModel.swiftArcBox/Views/ContentView.swiftArcBox/Views/Runners/RunnerEmptyState.swiftArcBox/Views/Runners/RunnerHostStatusBar.swiftArcBox/Views/Runners/RunnerImagePreparationStatusView.swiftArcBox/Views/Runners/RunnerJobRow.swiftArcBox/Views/Runners/RunnerJobsView.swiftArcBox/Views/Runners/RunnersView.swiftArcBox/Views/Settings/AccountSettingsView.swiftArcBox/Views/Settings/FleetSettingsView.swiftArcBox/Views/Settings/SettingsView.swiftArcBox/Views/SidebarAccountButton.swiftArcBoxTests/FleetEnrollmentCoordinatorTests.swiftArcBoxTests/FleetViewModelTests.swiftArcBoxTests/RunnersViewModelTests.swiftLocal.xcconfig.examplePackages/ArcBoxAuth/Sources/ArcBoxAuth/Configuration/AuthClientConfiguration.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/PKCE/PKCE.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/AuthError.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/AuthProviding.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/BetterAuthClient.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/DeviceAuthorization.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/IDTokenClaims.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCAuthorizationURLBuilder.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCClient.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCEndpoints.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCError.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCProviding.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCUserInfo.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/SessionSnapshot.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/TokenResponse.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AccessTokenProviding.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession+SignIn.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthStatus.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/KeychainError.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/KeychainTokenStore.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/StoredSession.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/StoredTokens.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/TokenStoring.swiftPackages/ArcBoxAuth/Sources/ArcBoxAuth/Support/Base64URL.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/AuthClientConfigurationTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/AuthSessionTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/AuthTestSupport.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/BetterAuthClientTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/IDTokenClaimsTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/KeychainTokenStoreTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/OIDCAuthorizationURLBuilderTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/OIDCClientConfigurationTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/OIDCClientTests.swiftPackages/ArcBoxAuth/Tests/ArcBoxAuthTests/PKCETests.swiftPackages/FleetControlClient/PROTO_SOURCEPackages/FleetControlClient/Package.swiftPackages/FleetControlClient/Sources/FleetControlClient/FleetControlClient.swiftPackages/FleetControlClient/Sources/FleetControlClient/FleetControlModels.swiftPackages/FleetControlClient/Sources/FleetControlClient/FleetImageModels.swiftPackages/FleetControlClient/Sources/FleetControlClient/Logging.swiftPackages/FleetControlClient/Sources/FleetControlClient/Module.swiftPackages/FleetControlClient/Tests/FleetControlClientTests/FleetControlClientTests.swiftPackages/FleetControlClient/generate.shPackages/FleetPlatformClient/Package.swiftPackages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swiftPackages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformConfiguration.swiftPackages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformError.swiftPackages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformModels.swiftPackages/FleetPlatformClient/Tests/FleetPlatformClientTests/FleetPlatformClientTests.swiftproject.yml
💤 Files with no reviewable changes (17)
- Packages/ArcBoxAuth/Tests/ArcBoxAuthTests/IDTokenClaimsTests.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCEndpoints.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCUserInfo.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCError.swift
- Packages/ArcBoxAuth/Tests/ArcBoxAuthTests/OIDCClientTests.swift
- Packages/ArcBoxAuth/Tests/ArcBoxAuthTests/OIDCClientConfigurationTests.swift
- Packages/ArcBoxAuth/Tests/ArcBoxAuthTests/OIDCAuthorizationURLBuilderTests.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCAuthorizationURLBuilder.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/PKCE/PKCE.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCProviding.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/IDTokenClaims.swift
- Packages/ArcBoxAuth/Tests/ArcBoxAuthTests/PKCETests.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/OIDCClient.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Protocol/TokenResponse.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Support/Base64URL.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/KeychainError.swift
- Packages/ArcBoxAuth/Sources/ArcBoxAuth/Storage/StoredTokens.swift
Comment on lines
21
to
+24
| .formStyle(.grouped) | ||
| // Sign-in fetches userinfo itself; this covers sessions restored | ||
| // Sign-in verifies the session itself; this covers sessions restored | ||
| // from the Keychain at launch. | ||
| .task { await authSession.loadUserInfo() } | ||
| .task { await authSession.refreshSession() } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1,160p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthStatus.swift
sed -n '90,145p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swiftRepository: arcboxlabs/arcbox-desktop
Length of output: 3580
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '\n== AccountSettingsView ==\n'
sed -n '1,120p' ArcBox/Views/Settings/AccountSettingsView.swift
printf '\n== AuthSession status changes ==\n'
rg -n "status\s*=|forgetSession\(|restore|restor" Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swift Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session -n
printf '\n== AuthSession outline ==\n'
ast-grep outline Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swift --view expandedRepository: arcboxlabs/arcbox-desktop
Length of output: 11250
Re-run session validation after restore completes.
refreshSession() returns early unless authSession.status == .signedIn, so this .task can fire while the view is still .restoring and never run again after the Keychain session is loaded. Key it to authSession.status so validation reruns on the .restoring → .signedIn transition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ArcBox/Views/Settings/AccountSettingsView.swift` around lines 21 - 24, Update
the view’s .task modifier around authSession.refreshSession() to key execution
to authSession.status, ensuring it reruns when the status transitions from
.restoring to .signedIn. Preserve the existing refreshSession call and behavior
for already signed-in sessions.
Comment on lines
+12
to
24
| public func signIn() async { | ||
| guard status != .restoring, status != .signingIn else { return } | ||
| guard !configuration.isPlaceholder else { | ||
| status = .error(AuthError.notConfigured.userMessage) | ||
| return | ||
| } | ||
| status = .signingIn | ||
| await finishAuthorization(callbackURL: url) | ||
| return true | ||
| // App-scoped task so `cancelSignIn()` can abandon the polling loop | ||
| // from anywhere (Cancel button, sign-out, termination). | ||
| let task = Task { await performDeviceSignIn() } | ||
| signInTask = task | ||
| await task.value | ||
| if signInTask == task { signInTask = nil } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the target file with line numbers
sed -n '1,220p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession+SignIn.swift | cat -n
printf '\n--- related symbols ---\n'
rg -n "signInTask|performDeviceSignIn|cancelSignIn|status = \\.signingIn|\\.restoring|signIn\\(" Packages/ArcBoxAuth/Sources/ArcBoxAuth -g '!**/*.generated.*'Repository: arcboxlabs/arcbox-desktop
Length of output: 6229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession+SignIn.swift | cat -n
printf '\n--- related symbols ---\n'
rg -n "signInTask|performDeviceSignIn|cancelSignIn|status = \\.signingIn|\\.restoring|signIn\\(" Packages/ArcBoxAuth/Sources/ArcBoxAuth -g '!**/*.generated.*'Repository: arcboxlabs/arcbox-desktop
Length of output: 6229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swift | cat -nRepository: arcboxlabs/arcbox-desktop
Length of output: 8725
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swift | cat -nRepository: arcboxlabs/arcbox-desktop
Length of output: 8725
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession.swift | cat -nRepository: arcboxlabs/arcbox-desktop
Length of output: 5828
Set status = .signingIn before awaiting the device task.
A second signIn() can run after the first suspends on await task.value but before performDeviceSignIn() updates state, which can start a duplicate device flow and overwrite signInTask. Guard signInTask == nil or set the status synchronously here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/ArcBoxAuth/Sources/ArcBoxAuth/Session/AuthSession`+SignIn.swift
around lines 12 - 24, The signIn flow does not synchronously mark an active
sign-in before awaiting performDeviceSignIn(), allowing duplicate flows. Update
signIn() to set status to .signingIn before creating or awaiting the task, or
guard signInTask == nil, while preserving the existing restoring/signingIn
checks and task cleanup.
Comment on lines
+1
to
+2
| import ArcBoxAuth | ||
| import Foundation |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Import order violates guideline in both the source and test file: Foundation must precede the local ArcBoxAuth package import. As per coding guidelines, "Order imports with Foundation and SwiftUI first, followed by local packages and then third-party dependencies."
Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift#L1-L2: reorder toimport Foundationthenimport ArcBoxAuth.Packages/FleetPlatformClient/Tests/FleetPlatformClientTests/FleetPlatformClientTests.swift#L1-L5: reorder toimport Foundation,import Testing, thenimport ArcBoxAuth, keeping@testable import FleetPlatformClientlast.
📍 Affects 2 files
Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift#L1-L2(this comment)Packages/FleetPlatformClient/Tests/FleetPlatformClientTests/FleetPlatformClientTests.swift#L1-L5
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift`
around lines 1 - 2, Reorder imports in
Packages/FleetPlatformClient/Sources/FleetPlatformClient/FleetPlatformClient.swift
(lines 1-2) so Foundation precedes ArcBoxAuth. In
Packages/FleetPlatformClient/Tests/FleetPlatformClientTests/FleetPlatformClientTests.swift
(lines 1-5), order imports as Foundation, Testing, ArcBoxAuth, with `@testable`
import FleetPlatformClient last.
Source: Coding guidelines
Comment on lines
+146
to
+147
| - package: FleetControlClient | ||
| - package: FleetPlatformClient |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether ArcBoxTests source files import FleetControlClient/FleetPlatformClient directly.
rg -n '^import (FleetControlClient|FleetPlatformClient)' ArcBoxTestsRepository: arcboxlabs/arcbox-desktop
Length of output: 454
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba project.yml | sed -n '136,222p'Repository: arcboxlabs/arcbox-desktop
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path("project.yml").read_text()
for needle in ["FleetControlClient", "FleetPlatformClient", "K8sClient", "ArcBoxTests"]:
print(f"=== {needle} ===")
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f"{i}:{line}")
PYRepository: arcboxlabs/arcbox-desktop
Length of output: 656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
lines = Path("project.yml").read_text().splitlines()
def show(start, end):
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4}: {lines[i]}")
# Show the ArcBoxTests target block and the ArcBox target block.
for i, line in enumerate(lines, 1):
if line.startswith(" ArcBoxTests:"):
print("=== ArcBoxTests ===")
show(i, i+35)
break
for i, line in enumerate(lines, 1):
if line.startswith(" ArcBox:"):
print("\n=== ArcBox ===")
show(i, i+30)
break
PYRepository: arcboxlabs/arcbox-desktop
Length of output: 2400
Add the Fleet packages to ArcBoxTests
ArcBoxTests imports FleetControlClient and FleetPlatformClient directly, but only declares ArcBox and K8sClient as dependencies. Add both Fleet packages here so the test target can compile.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@project.yml` around lines 146 - 147, Add FleetControlClient and
FleetPlatformClient to the ArcBoxTests dependency declarations in project.yml,
alongside its existing ArcBox and K8sClient packages, so the test target can
resolve both direct imports.
Use an ephemeral cookie-free URLSession for Better Auth requests and remove legacy Better Auth cookies persisted by previous builds. Add regressions covering session configuration, response cookie isolation, and scoped legacy cookie cleanup.
| enrollmentContext: EnrollmentContext? | ||
| ) -> RunnersViewState { | ||
| if let snapshot { | ||
| switch snapshot.enrollment { |
There was a problem hiding this comment.
When the Fleet Agent reports .detached with a valid machine ID, this branch returns .enrolled before consulting the coordinator's terminal .failed(.detached) state, causing the runner screen to show the full host controls and hide the prominent "Unenroll and Start Over…" recovery flow even though enrollment remains locked.
Knowledge Base Used: ViewModels: MVVM State Layer
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Architecture boundary
ArcBox Desktop owns authentication, Fleet Platform REST requests, enrollment orchestration, local Fleet Agent gRPC communication, and UI state.
Desktop does not install, create, start, stop, or update the Fleet Agent. The Fleet Agent process remains managed by launchd and its corresponding daemon. Runner VM lifecycle operations remain Fleet Agent → arcbox-daemon responsibilities.
Validation
make verify-arcbox-protobufswift-format lint -r --strict ArcBox/ Packages/swiftlint lint --strict --config .swiftlint.ymlxcodegen generatewith no tracked project-file driftSKIP_RUST_BUILD=1Draft status
Remaining end-to-end validation includes:
Supersedes #281.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation