Add Campfire support - #5813
Add Campfire support#5813
Conversation
Campfire (the collaborative pi-based harness) becomes a first-class
agent:
- cmux hooks campfire install/uninstall writes a native extension to
${CAMPFIRE_CODING_AGENT_DIR:-~/.campfire/agent}/extensions/
cmux-campfire-session.ts; opt out per process with
CMUX_CAMPFIRE_HOOKS_DISABLED=1
- the extension records the HOST role only (CAMPFIRE_SESSION_ROLE);
a joiner is an ephemeral view whose argv carries the invite URL, a
capability token that is never persisted or replayed
- launch capture normalizes the bun-compiled argv (drops the bunfs
virtual entry) and tags kind=campfire so restore runs
campfire --session <id> instead of mis-resuming as plain pi
- the extension subscribes to campfire's in-process observer bridge
(Symbol.for campfire.observer.v1) and surfaces driver-actionable
collaborative moments — a joiner waiting in the lobby, a capability
ask — as cmux notifications
- sanitizer policy preserves --relay/--model config flags and drops
prompts, session selectors, --join-as, and invite URLs; environment
policy replays CAMPFIRE_* config roots, never secrets, and drops the
self-managed PI_PACKAGE_DIR so an upgraded binary is not pinned to a
stale asset cache
- Vault and Task Manager detect campfire processes (compiled binary and
bun dev invocations) with sessions under ~/.campfire/agent/sessions
- docs, en+ja (and 18 more locales) CLI strings, Swift + Python tests,
CI hookup
Verification:
- swift test --package-path Packages/CMUXAgentLaunch (82 tests)
- xcodebuild test -only-testing:cmuxTests/CampfireSupportTests (4 tests)
- CMUX_CLI_BIN=... python3 tests/test_campfire_extension_install.py
- xcodebuild build (full app, tagged derived data)
- ./scripts/check-pbxproj.sh && ./scripts/lint-pbxproj-test-wiring.sh
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@NishantJoshi00 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Campfire as a built-in cmux coding agent: hook definitions, an embedded session extension with install/uninstall commands, notification summarization, launch environment/argument sanitization policies, vault registration and process/session detection, tests, CI wiring, and documentation. It also refactors AgentLaunchEnvironmentPolicy to instance-based methods and updates a Ghostty renderer-realized interop call with matching test stub signature changes. ChangesCampfire integration
Ghostty renderer interop refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CmuxCLI as cmux CLI
participant Extension as Campfire Extension
participant ChildProcess as Hook Child Process
participant SocketServer as cmux Socket
Extension->>Extension: normalize campfire argv, detect host role
Extension->>ChildProcess: spawn "cmux hooks campfire <subcommand>"
ChildProcess->>SocketServer: send session-start/prompt-submit/stop/notification
SocketServer-->>ChildProcess: ack (surface.resume.set / feed.push)
Extension->>Extension: observer bridge forwards join/permission/relay events
Extension->>ChildProcess: spawn "notification" hook with summarized payload
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ 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 |
NishantJoshi00
changed the title
Greptile SummaryThis PR adds Campfire as a first-class cmux agent. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (14): Last reviewed commit: "Merge main into campfire support" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ?? process.environment["PI_CODING_AGENT_SESSION_DIR"] | ||
| ?? configuredSessionDirectory(for: registration) | ||
| ?? ompAgentSessionsRoot(for: process, registration: registration) | ||
| ?? campfireAgentSessionsRoot(for: process, registration: registration) |
There was a problem hiding this comment.
PI_CODING_AGENT_SESSION_DIR pre-empts Campfire's own env vars
PI_CODING_AGENT_SESSION_DIR is checked at line 1070 with no registration-ID guard, so it takes precedence over the entire campfireAgentSessionsRoot lookup that correctly reads CAMPFIRE_CODING_AGENT_SESSION_DIR / CAMPFIRE_CODING_AGENT_DIR. Because Campfire embeds Pi, any user who has both agents installed and has set PI_CODING_AGENT_SESSION_DIR for Pi will silently have their Campfire sessions resolved against the Pi session directory. Vault will then fail to find Campfire sessions, or worse, attempt to restore a Pi session ID via campfire --session <pi-session-id>.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@Packages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchEnvironmentPolicyTests.swift`:
- Around line 46-53: The test keepsPiPackageDirForPiKinds claims to cover both
"pi" and "omp" but only exercises kind: "pi"; update the test to either (A) add
a second assertion calling
AgentLaunchEnvironmentPolicy.selectedEnvironment(from: ["PI_PACKAGE_DIR":
"/nix/store/pi-package"], kind: "omp") and assert selected["PI_PACKAGE_DIR"] ==
"/nix/store/pi-package" for the omp result, or (B) narrow the test name to only
mention "pi" if you intend to test just that path; locate the test function
keepsPiPackageDirForPiKinds and adjust accordingly to ensure both paths are
actually asserted or rename the test to match its single assertion.
In `@tests/test_campfire_extension_install.py`:
- Line 119: The code currently uses truthiness to derive request_id which
rewrites valid falsey JSON-RPC ids (e.g., 0) to "unknown"; change the logic to
check for the presence of the "id" key rather than its truthiness (use
payload["id"] if "id" in payload else "unknown") so that functions handling the
response echo the original id; locate the assignment to request_id in
tests/test_campfire_extension_install.py and replace the truthy check
accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c1c12033-19f6-421e-b907-5d845404f440
📒 Files selected for processing (22)
.github/workflows/ci.ymlCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+CampfireExtension.swiftCLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerPrimaryPolicies.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeArgv.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchEnvironmentPolicyTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeArgvTests.swiftResources/Localizable.xcstringsSources/TaskManagerTypes.swiftSources/VaultAgentProcessScanner.swiftSources/VaultAgentRegistry.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CampfireSupportTests.swiftdocs/agent-hooks.mddocs/feed.mddocs/vault.mdtests/test_campfire_extension_install.pyweb/app/[locale]/docs/session-restore/page.tsx
…ivacy - VaultAgentProcessScanner: gate PI_CODING_AGENT_SESSION_DIR out of the campfire registration so Campfire (which embeds Pi) resolves sessions against CAMPFIRE_CODING_AGENT_SESSION_DIR / CAMPFIRE_CODING_AGENT_DIR instead of being silently pre-empted by a user's Pi session dir. pi/omp behavior unchanged. Adds a regression test. - CMUXCLI+CampfireExtension: drop the raw relay reason from the user-facing notification; emit a generic message per the error-privacy policy. - AgentLaunchEnvironmentPolicyTests: assert both pi and omp keep PI_PACKAGE_DIR (test previously only exercised pi). - test_campfire_extension_install: preserve falsey JSON-RPC ids (0) when echoing responses instead of rewriting them to "unknown". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CLI/cmux.swift`:
- Around line 24505-24508: The case statement for non-nil capability values is
returning the raw value directly without validation, which can expose
provider-specific flags in user-facing alerts. In the `case let value?:` block,
add validation to check if the value is a known/mapped capability. For unknown
or unmapped capabilities, return the localized fallback string (the same one
used in the nil case) instead of exposing the raw provider value. This ensures
only safe, validated capability strings are shown to users while maintaining
proper localization.
In `@Sources/RestorableAgentSession.swift`:
- Around line 451-458: The equality check in the Campfire registration branch
compares the entire customRegistration struct to
CmuxVaultAgentRegistration.builtInCampfire, which is brittle. Replace this
full-struct equality check with a comparison of the id properties instead, so
that the condition checks if customRegistration.id equals
CmuxVaultAgentRegistration.builtInCampfire.id. This approach is more stable and
aligns with how the Antigravity branch handles built-in registration identity
checking.
In `@Sources/VaultAgentProcessScanner.swift`:
- Around line 122-128: The Campfire launch argument normalization is skipped
when a project or global override has the same id as builtInCampfire but
different properties like resumeCommand or sessionDirectory, because the
condition checks for full registration object equality instead of just the id.
Replace the equality check in the if statement that compares `registration ==
CmuxVaultAgentRegistration.builtInCampfire` with a check that compares only the
registration id property against the builtInCampfire registration's id, ensuring
that any Campfire registration with a matching id will trigger the
normalizedCampfireLaunchArguments function call regardless of other property
differences.
- Around line 885-895: The matches method has a logic flaw where primaryMatches
returns true when expectedNames is empty (no primary process names configured),
causing the method to return true before checking alternate criteria. When a
config provides only alternate criteria like alternateArgvContains or
alternateArgvContainsAny with no primary process names, every process
incorrectly matches. Fix the return statement in the matches method to ensure
that when primary criteria are absent (primaryProcessNames is empty), the method
does not return true based on primaryMatches alone, but instead requires that
alternate criteria are satisfied through the alternateMatches call. You may need
to add a condition that checks whether primaryProcessNames has content before
allowing primaryMatches to influence the overall result.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9a6e3c0-143e-4921-966c-e892a45a4d05
📒 Files selected for processing (14)
CLI/CMUXCLI+CampfireExtension.swiftCLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchEnvironmentPolicyTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swiftResources/Localizable.xcstringsSources/AgentForkSupport.swiftSources/RestorableAgentSession.swiftSources/VaultAgentProcessScanner.swiftSources/VaultAgentRegistry.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CampfireHookNotificationTests.swiftcmuxTests/CampfireSupportTests.swifttests/test_campfire_extension_install.py
| case let value?: | ||
| return value | ||
| case nil: | ||
| return String(localized: "agent.campfire.capability.fallback", defaultValue: "do something") |
There was a problem hiding this comment.
Avoid exposing raw capability identifiers in notification copy.
For unknown capabilities, this currently returns upstream raw values directly. That can leak provider/internal flags in a user-facing alert and skips localization fallback. Use the localized fallback for unknown/non-mapped capabilities instead.
Suggested fix
private func campfireCapabilityLabel(_ capability: String?) -> String {
switch capability {
case "queue:add":
return String(localized: "agent.campfire.capability.queueAdd", defaultValue: "queue a prompt")
case "queue:run-now":
return String(localized: "agent.campfire.capability.queueRunNow", defaultValue: "run a prompt now")
case "session:interrupt":
return String(localized: "agent.campfire.capability.sessionInterrupt", defaultValue: "interrupt the agent")
case "shell:exec":
return String(localized: "agent.campfire.capability.shellExec", defaultValue: "run a shell command")
case "tools:contribute":
return String(localized: "agent.campfire.capability.toolsContribute", defaultValue: "add tools or skills")
case "files:list":
return String(localized: "agent.campfire.capability.filesList", defaultValue: "browse files")
- case let value?:
- return value
+ case .some:
+ return String(localized: "agent.campfire.capability.fallback", defaultValue: "do something")
case nil:
return String(localized: "agent.campfire.capability.fallback", defaultValue: "do something")
}
}As per coding guidelines, user-facing alerts must not expose provider-specific flags, and user-facing Swift text must remain localized.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case let value?: | |
| return value | |
| case nil: | |
| return String(localized: "agent.campfire.capability.fallback", defaultValue: "do something") | |
| private func campfireCapabilityLabel(_ capability: String?) -> String { | |
| switch capability { | |
| case "queue:add": | |
| return String(localized: "agent.campfire.capability.queueAdd", defaultValue: "queue a prompt") | |
| case "queue:run-now": | |
| return String(localized: "agent.campfire.capability.queueRunNow", defaultValue: "run a prompt now") | |
| case "session:interrupt": | |
| return String(localized: "agent.campfire.capability.sessionInterrupt", defaultValue: "interrupt the agent") | |
| case "shell:exec": | |
| return String(localized: "agent.campfire.capability.shellExec", defaultValue: "run a shell command") | |
| case "tools:contribute": | |
| return String(localized: "agent.campfire.capability.toolsContribute", defaultValue: "add tools or skills") | |
| case "files:list": | |
| return String(localized: "agent.campfire.capability.filesList", defaultValue: "browse files") | |
| case _?: | |
| return String(localized: "agent.campfire.capability.fallback", defaultValue: "do something") | |
| case nil: | |
| return String(localized: "agent.campfire.capability.fallback", defaultValue: "do something") | |
| } | |
| } |
🤖 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 `@CLI/cmux.swift` around lines 24505 - 24508, The case statement for non-nil
capability values is returning the raw value directly without validation, which
can expose provider-specific flags in user-facing alerts. In the `case let
value?:` block, add validation to check if the value is a known/mapped
capability. For unknown or unmapped capabilities, return the localized fallback
string (the same one used in the nil case) instead of exposing the raw provider
value. This ensures only safe, validated capability strings are shown to users
while maintaining proper localization.
Source: Coding guidelines
| if customRegistration == CmuxVaultAgentRegistration.builtInCampfire { | ||
| return AgentResumeArgv().builtInKind( | ||
| kind: "campfire", | ||
| sessionId: sessionId, | ||
| executablePath: launchCommand?.executablePath, | ||
| arguments: launchCommand?.arguments ?? [] | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use built-in registration identity (id) instead of full-struct equality.
The Campfire branch currently depends on full CmuxVaultAgentRegistration equality, which is brittle and can bypass the built-in Campfire resume path if non-identity fields diverge. Match by id (as done for Antigravity) to keep routing stable.
Suggested patch
- if customRegistration == CmuxVaultAgentRegistration.builtInCampfire {
+ if customRegistration.id == CmuxVaultAgentRegistration.builtInCampfire.id {
return AgentResumeArgv().builtInKind(
kind: "campfire",
sessionId: sessionId,
executablePath: launchCommand?.executablePath,
arguments: launchCommand?.arguments ?? []
)
}🤖 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 `@Sources/RestorableAgentSession.swift` around lines 451 - 458, The equality
check in the Campfire registration branch compares the entire customRegistration
struct to CmuxVaultAgentRegistration.builtInCampfire, which is brittle. Replace
this full-struct equality check with a comparison of the id properties instead,
so that the condition checks if customRegistration.id equals
CmuxVaultAgentRegistration.builtInCampfire.id. This approach is more stable and
aligns with how the Antigravity branch handles built-in registration identity
checking.
| if registration == CmuxVaultAgentRegistration.builtInCampfire { | ||
| arguments = normalizedCampfireLaunchArguments( | ||
| observed.arguments, | ||
| defaultExecutable: registration.defaultExecutable | ||
| ) | ||
| executablePath = arguments.first ?? registration.defaultExecutable | ||
| } |
There was a problem hiding this comment.
Apply Campfire launch normalization by id, not full registration equality.
A project/global override that keeps id == "campfire" but changes resumeCommand or sessionDirectory will no longer equal builtInCampfire, so BunFS/runtime normalization is skipped and replay metadata can retain the wrong argv or drop preserved flags.
Proposed fix
- if registration == CmuxVaultAgentRegistration.builtInCampfire {
+ if registration.id == CmuxVaultAgentRegistration.builtInCampfire.id {
arguments = normalizedCampfireLaunchArguments(
observed.arguments,
defaultExecutable: registration.defaultExecutable
)🤖 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 `@Sources/VaultAgentProcessScanner.swift` around lines 122 - 128, The Campfire
launch argument normalization is skipped when a project or global override has
the same id as builtInCampfire but different properties like resumeCommand or
sessionDirectory, because the condition checks for full registration object
equality instead of just the id. Replace the equality check in the if statement
that compares `registration == CmuxVaultAgentRegistration.builtInCampfire` with
a check that compares only the registration id property against the
builtInCampfire registration's id, ensuring that any Campfire registration with
a matching id will trigger the normalizedCampfireLaunchArguments function call
regardless of other property differences.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/ci.yml:
- Around line 22-25: The checkout action in the guard job is persisting git
credentials in the git config, which creates a security risk for later workflow
steps. Add the `persist-credentials: false` parameter to the with section of the
actions/checkout step to disable credential persistence unless a subsequent step
explicitly requires authenticated git operations.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 909f86dd-da42-4aba-b9ec-468d58d627e9
📒 Files selected for processing (4)
.github/workflows/ci.ymlCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+AgentHookPayload.swiftCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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 @.github/workflows/ci.yml:
- Around line 22-25: The checkout action in the guard job is persisting git
credentials in the git config, which creates a security risk for later workflow
steps. Add the `persist-credentials: false` parameter to the with section of the
actions/checkout step to disable credential persistence unless a subsequent step
explicitly requires authenticated git operations.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 909f86dd-da42-4aba-b9ec-468d58d627e9
📒 Files selected for processing (4)
.github/workflows/ci.ymlCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+AgentHookPayload.swiftCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
🛑 Comments failed to post (1)
.github/workflows/ci.yml (1)
22-25:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisable persisted checkout credentials in the guard job.
This checkout keeps credentials in git config for later steps; harden it by setting
persist-credentials: falseunless a later step explicitly needs authenticated git operations.Suggested patch
- name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 + persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 22-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/ci.yml around lines 22 - 25, The checkout action in the guard job is persisting git credentials in the git config, which creates a security risk for later workflow steps. Add the `persist-credentials: false` parameter to the with section of the actions/checkout step to disable credential persistence unless a subsequent step explicitly requires authenticated git operations.Source: Linters/SAST tools
…re-support # Conflicts: # CLI/cmux.swift # Resources/Localizable.xcstrings # Sources/VaultAgentProcessScanner.swift # cmux.xcodeproj/project.pbxproj
f34c6aa to
84f1ba1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/TaskManagerTypes.swift`:
- Around line 590-597: The Campfire task manager definition in
CmuxTaskManagerCodingAgentDefinition with id "campfire" is missing the
argumentHostBasenames field, which prevents Task Manager from inspecting
arguments when Campfire is launched through ts-node. Add the
argumentHostBasenames field to this definition and include "ts-node" as a value
so that Task Manager can properly match the argumentNeedles when Campfire is
launched with commands like ts-node packages/session/bin/campfire.ts.
In `@Sources/VaultAgentProcessScanner.swift`:
- Around line 171-218: Create a new focused Swift file under Sources/ to house
the Campfire argument parsing helpers. Move the six functions
normalizedCampfireLaunchArguments, campfireScriptArgumentIndex,
campfireArgumentLooksLikeBunfsEntry, campfireArgumentLooksLikeExecutable,
campfireArgumentLooksLikeScript, and campfireArgumentLooksLikeJavaScriptRuntime
from VaultAgentProcessScanner.swift to this new file as static methods of an
appropriately named type (e.g., CampfireArgumentParser). Then update any call
sites in VaultAgentProcessScanner.swift that reference these helpers to use the
new type name as the caller to maintain the same behavior while reducing the
scanner file size.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8c09f6d7-0057-4762-8b83-a9ad26ceff10
📒 Files selected for processing (37)
.github/workflows/ci.ymlCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+AgentHookPayload.swiftCLI/CMUXCLI+CampfireExtension.swiftCLI/cmux.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerPrimaryPolicies.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeArgv.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchEnvironmentPolicyTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeArgvTests.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Renderer.swiftPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Interop/GhosttyRuntimeCInterop.swiftPackages/macOS/CmuxTerminalCore/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminalCore/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hPackages/macOS/CmuxTerminalEngine/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminalEngine/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hPackages/macOS/CmuxTerminalServices/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminalServices/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hResources/Localizable.xcstringsSources/AgentForkSupport.swiftSources/RestorableAgentSession.swiftSources/TaskManagerTypes.swiftSources/VaultAgentProcessScanner.swiftSources/VaultAgentRegistry.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CampfireHookNotificationTests.swiftcmuxTests/CampfireSupportTests.swiftdocs/agent-hooks.mddocs/feed.mddocs/vault.mdtests/test_campfire_extension_install.pyweb/app/[locale]/docs/session-restore/page.tsx
| private static func normalizedCampfireLaunchArguments( | ||
| _ arguments: [String], | ||
| defaultExecutable: String | ||
| ) -> [String] { | ||
| guard !arguments.isEmpty else { return [defaultExecutable] } | ||
| if campfireArgumentLooksLikeExecutable(arguments[0]) { | ||
| if arguments.count > 1, campfireArgumentLooksLikeBunfsEntry(arguments[1]) { | ||
| return [arguments[0]] + Array(arguments.dropFirst(2)) | ||
| } | ||
| return arguments | ||
| } | ||
| if campfireArgumentLooksLikeJavaScriptRuntime(arguments[0]), | ||
| let scriptIndex = campfireScriptArgumentIndex(in: arguments) { | ||
| return [defaultExecutable] + Array(arguments.dropFirst(scriptIndex + 1)) | ||
| } | ||
| return [defaultExecutable] + Array(arguments.dropFirst()) | ||
| } | ||
|
|
||
| private static func campfireScriptArgumentIndex(in arguments: [String]) -> Int? { | ||
| guard arguments.count > 1 else { return nil } | ||
| return arguments.indices.dropFirst().first { campfireArgumentLooksLikeScript(arguments[$0]) } | ||
| } | ||
|
|
||
| private static func campfireArgumentLooksLikeBunfsEntry(_ value: String) -> Bool { | ||
| let normalized = value.replacingOccurrences(of: "\\", with: "/") | ||
| return normalized.contains("$bunfs") | ||
| || normalized.contains("~BUN") | ||
| || normalized.contains("%7EBUN") | ||
| } | ||
|
|
||
| private static func campfireArgumentLooksLikeExecutable(_ value: String) -> Bool { | ||
| URL(fileURLWithPath: value).lastPathComponent.compare( | ||
| "campfire", | ||
| options: [.caseInsensitive, .literal] | ||
| ) == .orderedSame && !campfireArgumentLooksLikeBunfsEntry(value) | ||
| } | ||
|
|
||
| private static func campfireArgumentLooksLikeScript(_ value: String) -> Bool { | ||
| let normalized = value.replacingOccurrences(of: "\\", with: "/").lowercased() | ||
| let base = URL(fileURLWithPath: normalized).lastPathComponent | ||
| return ["campfire.ts", "campfire.js", "campfire"].contains(base) | ||
| && (normalized.contains("/campfire") || normalized.contains("packages/session")) | ||
| } | ||
|
|
||
| private static func campfireArgumentLooksLikeJavaScriptRuntime(_ value: String) -> Bool { | ||
| let base = URL(fileURLWithPath: value).lastPathComponent.lowercased() | ||
| return ["node", "bun", "deno", "tsx", "ts-node"].contains(base) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move the Campfire argv helpers out of this oversized scanner file.
These new Campfire-specific helpers add more responsibility to a 1,568-line production Swift file. Please move them to a focused sibling source file under Sources/ and keep this scanner as orchestration-only.
As per coding guidelines, production Swift files under Sources that exceed 800 lines should be flagged. Based on learnings, do not recommend extracting Foundation-only Swift files into a new SwiftPM package.
🤖 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 `@Sources/VaultAgentProcessScanner.swift` around lines 171 - 218, Create a new
focused Swift file under Sources/ to house the Campfire argument parsing
helpers. Move the six functions normalizedCampfireLaunchArguments,
campfireScriptArgumentIndex, campfireArgumentLooksLikeBunfsEntry,
campfireArgumentLooksLikeExecutable, campfireArgumentLooksLikeScript, and
campfireArgumentLooksLikeJavaScriptRuntime from VaultAgentProcessScanner.swift
to this new file as static methods of an appropriately named type (e.g.,
CampfireArgumentParser). Then update any call sites in
VaultAgentProcessScanner.swift that reference these helpers to use the new type
name as the caller to maintain the same behavior while reducing the scanner file
size.
Sources: Coding guidelines, Learnings
# Conflicts: # Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Interop/GhosttyRuntimeCInterop.swift # Packages/macOS/CmuxTerminalEngine/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c # Packages/macOS/CmuxTerminalEngine/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h # Packages/macOS/CmuxTerminalServices/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c # Packages/macOS/CmuxTerminalServices/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h # Resources/Localizable.xcstrings # cmux.xcodeproj/project.pbxproj
A CmuxVaultAgentDetectRule that specifies only alternate criteria (no primary process names and no argvContains) currently matches every process: the empty primary criteria make primaryMatches return true before the alternate criteria are checked. This test asserts an unrelated `node` process is not classified, and fails without the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lback - VaultAgentProcessScanner: gate the primary match on the presence of primary criteria so an alternate-only detect rule no longer matches every process (fixes the test added in the previous commit). - TaskManagerTypes: add `ts-node` to argumentHostBasenames so Task Manager classifies `ts-node …/campfire.ts` as Campfire, matching the hosts already recognized by Vault detection. - cmux CLI: route unknown/unmapped Campfire capability values to the localized fallback label instead of surfacing the raw identifier in user-facing notification copy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # cmux.xcodeproj/project.pbxproj # docs/agent-hooks.md # docs/feed.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@CLI/CMUXCLI`+CampfireExtension.swift:
- Around line 4-5: The Campfire extension path is duplicated between
CMUXCLI+CampfireExtension and the shared AgentHookDef config, so path
construction can drift. Update campfireExtensionURL() and the related Campfire
extension constants/methods to derive the URL from the existing AgentHookDef
definition (using its resolvedConfigDir and configFile) instead of rebuilding
the same "extensions/cmux-campfire-session.ts" path locally. Keep the unique
symbols campfireExtensionFilename, campfireExtensionURL(), and AgentHookDef as
the single source of truth so both call sites always resolve the same file.
In `@CLI/CMUXCLI`+SessionsListForkStartupInput.swift:
- Around line 116-121: The sessionsListASCIIPrintfCommandSubstitution helper is
doing per-UTF-8-byte String(format:) work while building the octal escape
sequence. Replace the map/join formatting path with a precomputed lookup-table
or buffer-based encoder that writes each byte’s octal escape directly into a
single preallocated string/buffer, and keep the change localized to
sessionsListASCIIPrintfCommandSubstitution so the returned printf substitution
stays identical without the hot-path allocations.
- Around line 81-84: The sessionsListLaunchEnvironmentParts helper is using an
optional collection for environment even though it immediately normalizes nil to
empty, so make the parameter non-optional and treat empty input as the only “no
environment” case. Update sessionsListLaunchEnvironmentParts to accept [String:
String] and adjust its call site to pass a default empty dictionary from
launchCommand?.environment, keeping the existing behavior while removing the
discouraged_optional_collection warning.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f253d064-46cc-48cc-8d52-df8a8261d645
📒 Files selected for processing (6)
.github/workflows/ci.ymlCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+AgentHookPayload.swiftCLI/CMUXCLI+CampfireExtension.swiftCLI/CMUXCLI+SessionsListForkStartupInput.swiftCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 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 `@CLI/CMUXCLI`+CampfireExtension.swift:
- Around line 4-5: The Campfire extension path is duplicated between
CMUXCLI+CampfireExtension and the shared AgentHookDef config, so path
construction can drift. Update campfireExtensionURL() and the related Campfire
extension constants/methods to derive the URL from the existing AgentHookDef
definition (using its resolvedConfigDir and configFile) instead of rebuilding
the same "extensions/cmux-campfire-session.ts" path locally. Keep the unique
symbols campfireExtensionFilename, campfireExtensionURL(), and AgentHookDef as
the single source of truth so both call sites always resolve the same file.
In `@CLI/CMUXCLI`+SessionsListForkStartupInput.swift:
- Around line 116-121: The sessionsListASCIIPrintfCommandSubstitution helper is
doing per-UTF-8-byte String(format:) work while building the octal escape
sequence. Replace the map/join formatting path with a precomputed lookup-table
or buffer-based encoder that writes each byte’s octal escape directly into a
single preallocated string/buffer, and keep the change localized to
sessionsListASCIIPrintfCommandSubstitution so the returned printf substitution
stays identical without the hot-path allocations.
- Around line 81-84: The sessionsListLaunchEnvironmentParts helper is using an
optional collection for environment even though it immediately normalizes nil to
empty, so make the parameter non-optional and treat empty input as the only “no
environment” case. Update sessionsListLaunchEnvironmentParts to accept [String:
String] and adjust its call site to pass a default empty dictionary from
launchCommand?.environment, keeping the existing behavior while removing the
discouraged_optional_collection warning.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f253d064-46cc-48cc-8d52-df8a8261d645
📒 Files selected for processing (6)
.github/workflows/ci.ymlCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+AgentHookPayload.swiftCLI/CMUXCLI+CampfireExtension.swiftCLI/CMUXCLI+SessionsListForkStartupInput.swiftCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
🛑 Comments failed to post (3)
CLI/CMUXCLI+CampfireExtension.swift (1)
4-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extension path duplicated across two files.
campfireExtensionFilename+ the hardcoded"extensions"path segment here reproduceconfigFile: "extensions/cmux-campfire-session.ts"already declared inCLI/CMUXCLI+AgentHookDefinitions.swift(line 221). They currently agree, but nothing enforces that — if either changes independently,campfireExtensionURL()andAgentHookDef.resolvedConfigDir()+configFilewould silently diverge.♻️ Derive the extension path from the shared `AgentHookDef`
- private func campfireExtensionURL() -> URL { - return Self.resolvedCampfireAgentDirectory() - .appendingPathComponent("extensions", isDirectory: true) - .appendingPathComponent(Self.campfireExtensionFilename, isDirectory: false) - } + private func campfireExtensionURL(def: AgentHookDef = CMUXCLI.agentDef(named: "campfire")!) -> URL { + URL(fileURLWithPath: def.resolvedConfigDir(), isDirectory: true) + .appendingPathComponent(def.configFile, isDirectory: false) + }Also applies to: 327-331
🤖 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 `@CLI/CMUXCLI`+CampfireExtension.swift around lines 4 - 5, The Campfire extension path is duplicated between CMUXCLI+CampfireExtension and the shared AgentHookDef config, so path construction can drift. Update campfireExtensionURL() and the related Campfire extension constants/methods to derive the URL from the existing AgentHookDef definition (using its resolvedConfigDir and configFile) instead of rebuilding the same "extensions/cmux-campfire-session.ts" path locally. Keep the unique symbols campfireExtensionFilename, campfireExtensionURL(), and AgentHookDef as the single source of truth so both call sites always resolve the same file.CLI/CMUXCLI+SessionsListForkStartupInput.swift (2)
81-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Prefer non-optional collection for
environmentparameter.SwiftLint flags
discouraged_optional_collectionhere. The function already normalizes nil viaguard let environment, !environment.isEmpty, so the optional adds no information the empty-dictionary default wouldn't convey.🧹 Proposed fix
private func sessionsListLaunchEnvironmentParts( agent: String, - environment: [String: String]? + environment: [String: String] = [:] ) -> [String] { - guard let environment, !environment.isEmpty else { return [] } + guard !environment.isEmpty else { return [] }And update the call site:
sessionsListLaunchEnvironmentParts(agent: agent, environment: launchCommand?.environment ?? [:]).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.private func sessionsListLaunchEnvironmentParts( agent: String, environment: [String: String] = [:] ) -> [String] {🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 83-83: Prefer empty collection over optional collection
(discouraged_optional_collection)
🤖 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 `@CLI/CMUXCLI`+SessionsListForkStartupInput.swift around lines 81 - 84, The sessionsListLaunchEnvironmentParts helper is using an optional collection for environment even though it immediately normalizes nil to empty, so make the parameter non-optional and treat empty input as the only “no environment” case. Update sessionsListLaunchEnvironmentParts to accept [String: String] and adjust its call site to pass a default empty dictionary from launchCommand?.environment, keeping the existing behavior while removing the discouraged_optional_collection warning.Source: Linters/SAST tools
116-121: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid per-byte
String(format:)allocation.
.map { String(format: #"\%03o"#, Int($0)) }allocates a formatted string for every UTF-8 byte. As per coding guidelines,.github/review-bot-rules/hot-path-allocating-formatting.mdrequires "a fixed lookup table written into a preallocated buffer" instead of per-elementString(format:)for this kind of encoding.⚡ Proposed fix using a precomputed lookup table
+ private static let octalByteEscapes: [String] = (0...255).map { String(format: #"\%03o"#, $0) } + private func sessionsListASCIIPrintfCommandSubstitution(for value: String) -> String { - let octalBytes = value.utf8 - .map { String(format: #"\%03o"#, Int($0)) } - .joined() + let octalBytes = value.utf8.map { Self.octalByteEscapes[Int($0)] }.joined() return #""$(printf '"# + octalBytes + #"')""# }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.private static let octalByteEscapes: [String] = (0...255).map { String(format: #"\%03o"#, $0) } private func sessionsListASCIIPrintfCommandSubstitution(for value: String) -> String { let octalBytes = value.utf8.map { Self.octalByteEscapes[Int($0)] }.joined() return #""$(printf '"# + octalBytes + #"')""# }🤖 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 `@CLI/CMUXCLI`+SessionsListForkStartupInput.swift around lines 116 - 121, The sessionsListASCIIPrintfCommandSubstitution helper is doing per-UTF-8-byte String(format:) work while building the octal escape sequence. Replace the map/join formatting path with a precomputed lookup-table or buffer-based encoder that writes each byte’s octal escape directly into a single preallocated string/buffer, and keep the change localized to sessionsListASCIIPrintfCommandSubstitution so the returned printf substitution stays identical without the hot-path allocations.Source: Coding guidelines
| private let campfireManagedEnvironmentKeys: Set<String> = [ | ||
| "PI_PACKAGE_DIR", | ||
| ] |
There was a problem hiding this comment.
Campfire restores still replay PI_CODING_AGENT_SESSION_DIR because it is allowlisted below and this Campfire-only removal set only contains PI_PACKAGE_DIR. When a user has Pi configured with a custom session directory, cmux can relaunch Campfire with that Pi session root in the environment. Since Campfire embeds Pi, the resumed process can keep writing or resolving session state under the Pi root instead of the Campfire root, even though the scanner-side lookup now avoids that value.
| private let campfireManagedEnvironmentKeys: Set<String> = [ | |
| "PI_PACKAGE_DIR", | |
| ] | |
| private let campfireManagedEnvironmentKeys: Set<String> = [ | |
| "PI_CODING_AGENT_SESSION_DIR", | |
| "PI_PACKAGE_DIR", | |
| ] |
Rule Used: Flag correctness-critical detection/identity deriv... (source)
There was a problem hiding this comment.
Fixed in 322610b: PI_CODING_AGENT_SESSION_DIR is now dropped from campfire resume environments alongside PI_PACKAGE_DIR, matching the scanner-side gating. Added test coverage in AgentLaunchEnvironmentPolicyTests.
— Claude Code
|
@codex review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The scanner already gates PI_CODING_AGENT_SESSION_DIR out when resolving Campfire session roots, but restore still replayed it into resumed Campfire processes. Since Campfire embeds Pi, a user's custom Pi session root could then receive resumed Campfire session state while cmux reads the Campfire root. Drop it for campfire resumes alongside PI_PACKAGE_DIR; pi/omp behavior is unchanged.
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
AgentLaunchCaptureTrust only treated node and bun as hosts that can run a Campfire entrypoint, so PID-based argv fallback dropped campfire hook captures launched via deno, tsx, or ts-node even though the rest of the Campfire support (normalizer, scanner, task manager) recognizes those hosts. Gate the campfire needle check on the same host set; the claude detection stays limited to node/bun.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| static let campfirePolicy: Policy = { | ||
| var policy = piPolicy | ||
| policy.valueOptions.formUnion(["--relay", "--join", "--join-as", "--name"]) | ||
| policy.nonRestorableCommands.insert("init") | ||
| policy.droppedOptions.formUnion(["--join", "--join-as", "--name", "--auto-exit"]) | ||
| policy.droppedOptionPrefixes.append(contentsOf: ["--join=", "--join-as=", "--name="]) | ||
| return policy | ||
| }() |
There was a problem hiding this comment.
This Campfire policy inherits Pi's preserved --session-dir option. When a Campfire host is launched through a Pi-based wrapper or config with --session-dir ~/.pi/agent/sessions, restore can replay campfire --session <id> --session-dir ~/.pi/agent/sessions .... That makes Campfire look for the restored session under the Pi root even though the environment filter now removes the Pi session-root variable. Campfire should drop this argv override unless it is known to point at the Campfire session root.
There was a problem hiding this comment.
Intentional: the scanner resolves the Campfire session root from argv --session-dir first (VaultAgentProcessScanner.swift, sessionRoot lookup), so a session discovered under an explicit --session-dir was indexed from that exact root, and replaying the same flag points the resumed Campfire at the directory where that session actually lives. Dropping it would orphan restores for users who launch with an explicit --session-dir. The env-var case is different because the scanner deliberately ignores PI_CODING_AGENT_SESSION_DIR for campfire, which is why only the env replay was gated (322610b).
— Claude Code
There was a problem hiding this comment.
That makes sense — the scanner's argv-first resolution means the flag is load-bearing for explicit-path sessions, and the env-var gate already handles the Pi-root contamination case. Withdrawing the concern.
workflow-guard-tests failed on the file-length budget: the Campfire feature grows CLI/cmux.swift, VaultAgentProcessScanner, TaskManagerTypes, RestorableAgentSession, the CMUXAgentLaunch sanitizer files, and adds cmuxTests/CampfireSupportTests.swift past the tracked thresholds. Accept the feature growth in the budget; no unrelated entries changed.
|
Deployment failed with the following error: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 653c5d9. Configure here.
| private static let campfireManagedEnvironmentKeys: Set<String> = [ | ||
| "PI_CODING_AGENT_SESSION_DIR", | ||
| "PI_PACKAGE_DIR", | ||
| ] |
There was a problem hiding this comment.
Pi dirs still replay on Campfire
Medium Severity
Campfire resume filtering drops PI_CODING_AGENT_SESSION_DIR and PI_PACKAGE_DIR, but still replays PI_CODING_AGENT_DIR and PI_CONFIG_DIR. Because Campfire embeds Pi, a captured ambient Pi agent/config root can be restored into Campfire and point the embedded runtime at the user’s Pi directories instead of the Campfire roots already allowlisted as CAMPFIRE_CODING_AGENT_DIR / CAMPFIRE_CODING_AGENT_SESSION_DIR.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 653c5d9. Configure here.
This branch was successfully deployed
2 participants
Summary
Adds first-class support for Campfire, a collaborative pi-based harness:
cmux hooks campfireinstalls/uninstalls the hook extension. Current Campfire ships a native built-in bridge (zero setup); the installed extension serves older versions and defers to the native bridge so nothing double-firesCAMPFIRE_SESSION_ROLE) — a joiner is an ephemeral view whose argv carries the invite URL, a capability token that is never persisted or replayedkind=campfirewith a normalized bun-compiled argv, so restore runscampfire --session <id>instead of mis-resuming as plainpinotification_typesignals (Needs Input while a dialog blocks the driver)--relayand pi config flags; drops prompts, session selectors,--join-as, and invite URLs; env replay keepsCAMPFIRE_*roots, never secrets~/.campfire/agent/sessionsVerification
./scripts/reload.sh --tag campfire-supportxcodebuild test -project cmux.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-campfire-support -only-testing:cmuxTests/CampfireSupportTestsswift test --package-path Packages/CMUXAgentLaunchpython3 tests/test_campfire_extension_install.py./scripts/check-pbxproj.sh && ./scripts/lint-pbxproj-test-wiring.shLocalization
New
cli.hooks.campfire.*strings across all 20 locales (modeled oncli.hooks.omp.*); en and ja audited. Docs:agent-hooks.md,vault.md,feed.md, session-restore page.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Note
Medium Risk
Touches session restore, launch argv/env replay, and explicit handling of invite capability tokens; mitigated by host-only persistence, sanitizer drops, and new tests.
Overview
Adds Campfire as a supported agent alongside Pi/OMP:
cmux hooks campfireinstalls a managed TypeScript extension under~/.campfire/agent/extensions, registers the agent in the hook catalog, and wires install/uninstall intohooks setup.The extension forwards host-only session lifecycle to
cmux hooks campfire(joiners and native Campfire cmux bridges are skipped) and pushes collaborative observer events (join requests, capability asks, relay errors) into cmux notifications via new payload fields andsummarizeCampfireObserverNotification.Restore and launch capture treat Campfire as its own kind: resume uses
campfire --session, argv sanitization drops invite URLs and joiner flags while keeping--relay, launch trust recognizes script hosts, andAgentLaunchEnvironmentPolicyreplaysCAMPFIRE_*roots but strips Pi-managed env forkind=campfire. Vault gainsbuiltInCampfire, richer detect rules (alternateProcessNames/alternateArgvContainsAny), host-role gating for restorable snapshots, andCampfireLaunchArgumentNormalizerfor bun/node entrypoints.Supporting changes include Task Manager agent definition, CLI help/docs strings, a CI install test, and localized strings for hook UX and notification copy.
Reviewed by Cursor Bugbot for commit 653c5d9. Bugbot is set up for automated code reviews on this repo. Configure here.