Conversation
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughConfiguration and UI updates to the hack daemon and macOS dashboard. Changes include defaulting launchd auto-start behavior, repositioning the status strip in the titlebar, adding expandable session selector panel, implementing session attachment to external terminals, and tracking lifecycle command persistence across the codebase. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dashboard as HomeDashboardView
participant Session as SessionRow
participant Terminal as TerminalIntegration
participant Notifications as NotificationCenter
User->>Dashboard: Click Attach on session
Dashboard->>Session: Show attach menu
User->>Session: Select terminal (Hack/External)
Session->>Dashboard: attachSession(entry, terminal)
alt Hack Terminal
Dashboard->>Notifications: Post hackTerminalOpenRequested
Notifications-->>Terminal: Handle open request
else External Terminal
Dashboard->>Dashboard: Build attach command (tmux/zellij)
Dashboard->>Terminal: Open terminal with command
Terminal-->>User: Display session in terminal
end
Dashboard->>Dashboard: Update preferredExternalTerminal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
## <small>1.10.1 (2026-02-19)</small> * Fix gateway 502 for livenation ([a1bb858](a1bb858)) * Improve dashboard header and groups ([1e58826](1e58826)) * Merge branch 'main' into squircle ([f06f84c](f06f84c)) * Merge pull request #10 from hack-dance/squircle ([4539e92](4539e92)), closes [#10](#10) * fix(runtime): improve local ingress self-healing ([d305e2d](d305e2d)) * fix(tests): stabilize shell mocks across bun test suite ([402ebe2](402ebe2))
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift (1)
137-141:⚠️ Potential issue | 🟡 MinorHeader pill shows
processCountbut lifecycle badge showspersistentCount— inconsistent.Line 140 uses
lifecycleSummary.processCountfor the "persistent" label, but line 812 useslifecycleSummary.persistentCount(which equalspersistentHookCount + processCount). If a project has persistent lifecycle hooks, the header pill will undercount.🐛 Proposed fix
if lifecycleSummary.hasEntries { headerMetricPill( "Startup", - value: "\(lifecycleSummary.startupHookCount) hooks / \(lifecycleSummary.processCount) persistent" + value: "\(lifecycleSummary.startupHookCount) hooks / \(lifecycleSummary.persistentCount) persistent" ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift` around lines 137 - 141, The header pill for "Startup" uses lifecycleSummary.processCount but the lifecycle badge uses lifecycleSummary.persistentCount, causing undercounting; update the headerMetricPill call in ProjectDetailView (the "Startup" label block that currently formats "\(lifecycleSummary.processCount) persistent") to use lifecycleSummary.persistentCount (which is persistentHookCount + processCount) so both the header and the lifecycle badge report the same persistent total.
🧹 Nitpick comments (7)
src/templates.ts (1)
32-35: Consider usingCADDY_DOCKER_POLLING_INTERVALenv var for consistency.The existing
CADDY_INGRESS_NETWORKSis already configured via theenvironment:block. The caddy-docker-proxy docs showCADDY_DOCKER_POLLING_INTERVAL=<duration>as the equivalent env var for the polling interval. Using the env var keeps all runtime configuration in one place and avoids the need to overridecommand:(which replaces the image's default CMD).♻️ Alternative using env var (no command block needed)
- " command:", - " - docker-proxy", - " - --polling-interval", - " - 5s", " ports:", ... " environment:", ` CADDY_INGRESS_NETWORKS: ${DEFAULT_INGRESS_NETWORK}`, + " CADDY_DOCKER_POLLING_INTERVAL: 5s",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/templates.ts` around lines 32 - 35, Replace the explicit command override for docker-proxy with the equivalent environment variable: remove the "command:" array entries (the docker-proxy/--polling-interval/5s lines) and instead add an environment variable CADDY_DOCKER_POLLING_INTERVAL set to the desired duration alongside the existing CADDY_INGRESS_NETWORKS in the environment block so runtime config remains consistent and the image default CMD is preserved.tests/coredns-config.test.ts (1)
27-39: Command block assertions are tied to the static-IP test case, but the block is unconditional.The
command:/docker-proxyblock inrenderGlobalCaddyComposeis emitted regardless ofuseStaticCaddyIp/useStaticCoreDnsIp. Bundling these assertions into the static-IP test provides no coverage for the default render path. Consider adding a dedicated test or a simple standalone assertion:♻️ Suggested additional test for the default render path
test("renderGlobalCaddyCompose always includes docker-proxy command", () => { const text = renderGlobalCaddyCompose(); expect(text).toContain("command:"); expect(text).toContain("- docker-proxy"); expect(text).toContain("- --polling-interval"); expect(text).toContain("- 5s"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/coredns-config.test.ts` around lines 27 - 39, The test currently couples assertions for the unconditional "command:/docker-proxy" block with the static-IP case, leaving the default render path untested; update tests to either extract the docker-proxy assertions into a separate test that calls renderGlobalCaddyCompose() with no args (e.g., a new test "renderGlobalCaddyCompose always includes docker-proxy command") or move those assertions out of the static-IP test so the static-IP test only asserts IP/network lines (references: renderGlobalCaddyCompose, DEFAULT_CADDY_IP, DEFAULT_COREDNS_IP). Ensure the new/adjusted test verifies "command:", "- docker-proxy", "- --polling-interval", and "- 5s" are present.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/GlobalStatusStrip.swift (3)
858-883: Corner radius reset task uses a hardcoded delay that differs from the named constant pattern.
collapseSelectorPaneluses140_000_000nanoseconds directly (line 873) while other delays are defined as named computed properties (e.g.,selectorCloseDelayNanoseconds). For consistency, consider extracting this into a named property.♻️ Proposed fix
+ private var selectorCornerResetDelayNanoseconds: UInt64 { + 140_000_000 + } + private func collapseSelectorPanel() { ... selectorCornerResetTask = Task { - try? await Task.sleep(nanoseconds: 140_000_000) + try? await Task.sleep(nanoseconds: selectorCornerResetDelayNanoseconds) ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/GlobalStatusStrip.swift` around lines 858 - 883, The collapseSelectorPanel function contains a hardcoded sleep delay (140_000_000 ns) for selectorCornerResetTask; replace this literal with a named computed property (e.g., selectorCornerResetDelayNanoseconds or reuse an existing selectorCloseDelayNanoseconds if appropriate) and use that property in the Task.sleep call, update any related comments/tests to reference the new constant, and ensure the property is declared near other timing properties so the timing pattern remains consistent across selectorExpand/collapse logic.
571-583:RelativeDateTimeFormatteris allocated on every computed property access.This computed property creates a new
RelativeDateTimeFormattereach time the view body evaluates. Consider making it astatic letor caching it to avoid repeated allocation.♻️ Proposed fix: hoist formatter to static
+ private static let relativeFormatter: RelativeDateTimeFormatter = { + let f = RelativeDateTimeFormatter() + f.unitsStyle = .abbreviated + return f + }() + private var lastUpdatedText: String? { guard let date = model.lastUpdated else { return nil } - let formatter = RelativeDateTimeFormatter() - formatter.unitsStyle = .abbreviated let now = Date() if date > now { return "Updated just now" } let delta = now.timeIntervalSince(date) if delta < 5 { return "Updated just now" } - return "Updated \(formatter.localizedString(for: date, relativeTo: now))" + return "Updated \(Self.relativeFormatter.localizedString(for: date, relativeTo: now))" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/GlobalStatusStrip.swift` around lines 571 - 583, The computed property lastUpdatedText repeatedly allocates a RelativeDateTimeFormatter on every access; change to use a shared, lazily-initialized formatter (e.g. a static let or cached instance) so the formatter is reused. Locate lastUpdatedText and replace the local "let formatter = RelativeDateTimeFormatter()" with a reference to the shared formatter (ensure its unitsStyle = .abbreviated is set when creating the static), keep the same logic using model.lastUpdated, Date(), and timeIntervalSince(date), and ensure thread-safety by using a static let or a DispatchOnce-style lazy initializer.
15-25: Consider grouping related hover/expansion state into a dedicated struct or observable.Ten
@Stateproperties for a single hover-expand interaction add cognitive load. While functional, extracting them into a smallSelectorExpansionStatestruct (or a lightweight observable) would improve readability and make it easier to reason about the state machine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/GlobalStatusStrip.swift` around lines 15 - 25, Group the closely related hover/expand state (isSelectorHeaderHovered, isSelectorPanelHovered, isSelectorExpanded, isSelectorListVisible, selectorUsesExpandedCorners, hoveredSelectorProjectId, selectorExpandTask, selectorCollapseTask, selectorListRevealTask, selectorCornerResetTask, stripContentWidth) into a single small model (e.g., struct SelectorExpansionState or class SelectorExpansionModel: ObservableObject) and replace the individual `@State` properties in GlobalStatusStrip with one `@StateObject` or `@ObservedObject` instance; update all references and bindings to use the new model’s properties (and expose any Task handlers as methods on the model if needed), ensuring SwiftUI updates work the same way and that initialization and lifecycle (creation, cancelation of tasks) are handled inside the new model.apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/HomeDashboardView.swift (2)
449-456:DashboardSessionEntrysynthesizesHashableover full model objects.The struct holds
ProjectSessionSummaryand optionalProjectSummary, so synthesizedHashablewill hash all their fields. This is fine for typical project counts but worth noting — if these models grow large, consider implementingHashablemanually using just theid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/HomeDashboardView.swift` around lines 449 - 456, DashboardSessionEntry currently derives Hashable for the full session and project models which will hash all fields; change this to a manual, ID-based hash by removing the synthesized Hashable conformance and implementing Hashable (and Equatable) where hash(into:) only feeds session.id and project?.id (or use the existing id property) and equality compares the same IDs; update the type declaration for DashboardSessionEntry and add the hash(into:) and == implementations referencing session.id and project?.id to avoid hashing large model contents.
309-323:attachCommandandshellQuoteare duplicated fromProjectDetailView.Both
attachCommand(for:)(lines 309–316) andshellQuote(_:)(lines 318–323) are identical to their counterparts inProjectDetailView.swift(lines 1898–1905 and 1932–1937). Additionally,SettingsOverlayView.swifthas ashellQuotewith a different escaping strategy ('\''vs'"'"'), which could cause subtle inconsistencies if one is used over the other.Consider extracting these into a shared utility (e.g., on
TerminalIntegrationor a small helper extension) to avoid divergence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/HomeDashboardView.swift` around lines 309 - 323, Duplicate implementations of attachCommand(for:) and shellQuote(_:) exist across HomeDashboardView, ProjectDetailView, and a variant in SettingsOverlayView; extract a single shared implementation (for example as static helpers on TerminalIntegration or a small ShellQuote helper) and replace the duplicated methods with calls to that shared API. Move the tmux/zellij logic from attachCommand(for:) and the robust escaping used in shellQuote(_:) into the new helper, update HomeDashboardView.attachCommand(for:) and ProjectDetailView.attachCommand(for:) to call the shared function, and consolidate SettingsOverlayView to use the same escaping strategy to avoid divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/HomeDashboardView.swift`:
- Around line 370-378: HomeDashboardView currently falls back to
TerminalIntegration.ExternalTerminalApp.hackDesktop while ProjectDetailView
falls back to .terminal, causing inconsistent defaults for the same `@AppStorage`
keys; either align them or document the intentional difference. Fix by locating
the preferredExternalTerminal computed property in HomeDashboardView (using
preferredExternalTerminalRaw and legacyPreferredExternalTerminalRaw) and either
change the final return from .hackDesktop to .terminal to match
ProjectDetailView, or add a clear code comment above the property explaining why
HomeDashboardView intentionally uses .hackDesktop and referencing
ProjectDetailView’s fallback to .terminal so future readers know this divergence
is deliberate.
---
Outside diff comments:
In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ProjectDetailView.swift`:
- Around line 137-141: The header pill for "Startup" uses
lifecycleSummary.processCount but the lifecycle badge uses
lifecycleSummary.persistentCount, causing undercounting; update the
headerMetricPill call in ProjectDetailView (the "Startup" label block that
currently formats "\(lifecycleSummary.processCount) persistent") to use
lifecycleSummary.persistentCount (which is persistentHookCount + processCount)
so both the header and the lifecycle badge report the same persistent total.
---
Nitpick comments:
In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/GlobalStatusStrip.swift`:
- Around line 858-883: The collapseSelectorPanel function contains a hardcoded
sleep delay (140_000_000 ns) for selectorCornerResetTask; replace this literal
with a named computed property (e.g., selectorCornerResetDelayNanoseconds or
reuse an existing selectorCloseDelayNanoseconds if appropriate) and use that
property in the Task.sleep call, update any related comments/tests to reference
the new constant, and ensure the property is declared near other timing
properties so the timing pattern remains consistent across
selectorExpand/collapse logic.
- Around line 571-583: The computed property lastUpdatedText repeatedly
allocates a RelativeDateTimeFormatter on every access; change to use a shared,
lazily-initialized formatter (e.g. a static let or cached instance) so the
formatter is reused. Locate lastUpdatedText and replace the local "let formatter
= RelativeDateTimeFormatter()" with a reference to the shared formatter (ensure
its unitsStyle = .abbreviated is set when creating the static), keep the same
logic using model.lastUpdated, Date(), and timeIntervalSince(date), and ensure
thread-safety by using a static let or a DispatchOnce-style lazy initializer.
- Around line 15-25: Group the closely related hover/expand state
(isSelectorHeaderHovered, isSelectorPanelHovered, isSelectorExpanded,
isSelectorListVisible, selectorUsesExpandedCorners, hoveredSelectorProjectId,
selectorExpandTask, selectorCollapseTask, selectorListRevealTask,
selectorCornerResetTask, stripContentWidth) into a single small model (e.g.,
struct SelectorExpansionState or class SelectorExpansionModel: ObservableObject)
and replace the individual `@State` properties in GlobalStatusStrip with one
`@StateObject` or `@ObservedObject` instance; update all references and bindings to
use the new model’s properties (and expose any Task handlers as methods on the
model if needed), ensuring SwiftUI updates work the same way and that
initialization and lifecycle (creation, cancelation of tasks) are handled inside
the new model.
In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/HomeDashboardView.swift`:
- Around line 449-456: DashboardSessionEntry currently derives Hashable for the
full session and project models which will hash all fields; change this to a
manual, ID-based hash by removing the synthesized Hashable conformance and
implementing Hashable (and Equatable) where hash(into:) only feeds session.id
and project?.id (or use the existing id property) and equality compares the same
IDs; update the type declaration for DashboardSessionEntry and add the
hash(into:) and == implementations referencing session.id and project?.id to
avoid hashing large model contents.
- Around line 309-323: Duplicate implementations of attachCommand(for:) and
shellQuote(_:) exist across HomeDashboardView, ProjectDetailView, and a variant
in SettingsOverlayView; extract a single shared implementation (for example as
static helpers on TerminalIntegration or a small ShellQuote helper) and replace
the duplicated methods with calls to that shared API. Move the tmux/zellij logic
from attachCommand(for:) and the robust escaping used in shellQuote(_:) into the
new helper, update HomeDashboardView.attachCommand(for:) and
ProjectDetailView.attachCommand(for:) to call the shared function, and
consolidate SettingsOverlayView to use the same escaping strategy to avoid
divergence.
In `@src/templates.ts`:
- Around line 32-35: Replace the explicit command override for docker-proxy with
the equivalent environment variable: remove the "command:" array entries (the
docker-proxy/--polling-interval/5s lines) and instead add an environment
variable CADDY_DOCKER_POLLING_INTERVAL set to the desired duration alongside the
existing CADDY_INGRESS_NETWORKS in the environment block so runtime config
remains consistent and the image default CMD is preserved.
In `@tests/coredns-config.test.ts`:
- Around line 27-39: The test currently couples assertions for the unconditional
"command:/docker-proxy" block with the static-IP case, leaving the default
render path untested; update tests to either extract the docker-proxy assertions
into a separate test that calls renderGlobalCaddyCompose() with no args (e.g., a
new test "renderGlobalCaddyCompose always includes docker-proxy command") or
move those assertions out of the static-IP test so the static-IP test only
asserts IP/network lines (references: renderGlobalCaddyCompose,
DEFAULT_CADDY_IP, DEFAULT_COREDNS_IP). Ensure the new/adjusted test verifies
"command:", "- docker-proxy", "- --polling-interval", and "- 5s" are present.
| private var preferredExternalTerminal: TerminalIntegration.ExternalTerminalApp { | ||
| if let explicit = TerminalIntegration.ExternalTerminalApp(rawValue: preferredExternalTerminalRaw) { | ||
| return explicit | ||
| } | ||
| if let legacy = TerminalIntegration.ExternalTerminalApp(rawValue: legacyPreferredExternalTerminalRaw) { | ||
| return legacy | ||
| } | ||
| return .hackDesktop | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the HomeDashboardView file and check the specified lines
find . -name "HomeDashboardView.swift" -type fRepository: hack-dance/hack
Length of output: 155
🏁 Script executed:
# Find ProjectDetailView.swift file
find . -name "ProjectDetailView.swift" -type fRepository: hack-dance/hack
Length of output: 155
🏁 Script executed:
# Search for all preferredExternalTerminal properties and their fallbacks
rg -n 'preferredExternalTerminal' --type=swift -A 10 -B 2Repository: hack-dance/hack
Length of output: 25036
Default terminal inconsistency: .hackDesktop vs .terminal — clarify or align.
HomeDashboardView falls back to .hackDesktop while ProjectDetailView falls back to .terminal (line 1803). Both use the same @AppStorage keys with .terminal initialization, yet diverge in their final fallback. Add a code comment explaining the intentional difference, or align both to use the same fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/HomeDashboardView.swift`
around lines 370 - 378, HomeDashboardView currently falls back to
TerminalIntegration.ExternalTerminalApp.hackDesktop while ProjectDetailView
falls back to .terminal, causing inconsistent defaults for the same `@AppStorage`
keys; either align them or document the intentional difference. Fix by locating
the preferredExternalTerminal computed property in HomeDashboardView (using
preferredExternalTerminalRaw and legacyPreferredExternalTerminalRaw) and either
change the final return from .hackDesktop to .terminal to match
ProjectDetailView, or add a clear code comment above the property explaining why
HomeDashboardView intentionally uses .hackDesktop and referencing
ProjectDetailView’s fallback to .terminal so future readers know this divergence
is deliberate.
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Changes
--no-run-at-loadto disableDocumentation