Skip to content

Preserve Pi provider credentials for workspace auto-naming - #9479

Closed
austinywang wants to merge 6 commits into
mainfrom
issue-8718-pi-workspace-auto-naming-does-not-run-fo
Closed

austinywang wants to merge 6 commits into
mainfrom
issue-8718-pi-workspace-auto-naming-does-not-run-fo

Conversation

@austinywang

@austinywang austinywang commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • preserve Pi's built-in provider authentication environment only for the Stop hook that can launch workspace auto-naming
  • pin the installing cmux executable into the generated extension, so socket/provider credentials never go to environment- or PATH-selected executables
  • cover Pi 0.81.1 Bedrock and proxy settings while retaining the strict allowlist for every other hook subprocess

When --no-extensions removes a custom provider, Pi's built-in fallback can now authenticate without broadly exposing credentials.

Fixes #8718

Testing

  • Red on ba19285046: CMUX_CLI_BIN="$CLI_BIN" python3 tests/test_pi_extension_install.py failed with No API key found for the selected model.; autoNameLastAttemptAt was present and autoNameLastNamedAt was absent.
  • Green on ec5656dbfd: the same regression passed, including fallback exit 0/title output, title application, and both timestamps.
  • Latest HEAD: python3 tests/test_pi_extension_install.py and python3 tests/test_pi_extension_dispatch.py pass against the tagged CLI.
  • ruff check tests/claude_teams_test_utils.py tests/test_pi_extension_install.py tests/test_pi_extension_dispatch.py
  • ./scripts/reload.sh --tag sym8718 --derived-data /private/tmp/cmux-sym8718-isolated
  • Tagged-socket dogfood on /tmp/cmux-debug-sym8718.sock: a generated Pi 0.81.1-shaped extension ignored a conflicting absolute CMUX_BUNDLED_CLI_PATH, invoked the installer-pinned CLI, ran the fallback with --no-extensions at exit 0, populated autoNameLastAttemptAt/autoNameLastNamedAt, and changed the live generic workspace title to Pi Pinned Credential Handoff. The tagged app was then quit.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Pi stop hook now preserves allowlisted provider variables only when it uses a trusted bundled cmux executable. Tests cover auto-naming, credential isolation, executable selection, persisted state, and applied workspace titles.

Changes

Pi auto-naming environment flow

Layer / File(s) Summary
Provider environment allowlist and filtering
CLI/CMUXCLI+PiExtensionSourcePart1.swift
The CLI defines provider, cloud, credential, and proxy variables. Hook-environment construction preserves them only when auto-naming is enabled.
Trusted Pi stop hook dispatch
CLI/CMUXCLI+PiExtensionSourcePart1.swift, CLI/CMUXCLI+PiExtensionSourceDispatch.swift
The CLI validates the bundled executable, detects hooks pi stop, and enables provider propagation only for credential-trusted execution.
Auto-naming integration validation
tests/test_pi_extension_install.py, tests/test_pi_extension_dispatch.py
The tests mock cmux RPC calls and verify provider propagation, secret isolation, trusted CLI selection, generated-hook auto-naming, persisted state, and applied titles.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: lawrencecchen

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #8718 by enabling trusted fallback authentication and verifying turn-end dispatch, title application, and naming timestamps.
Out of Scope Changes check ✅ Passed The implementation and tests remain focused on Pi hook credential handling and workspace auto-naming behavior.
Cmux Swift Actor Isolation ✅ Passed The Swift diff only changes TypeScript inside existing #""" literals; it adds no Swift models, protocols, Sendable references, actors, or MainActor access.
Cmux Swift Blocking Runtime ✅ Passed The Swift-labeled production diff only changes executable trust and environment allowlisting; it adds no blocking or timing primitive. New waits/timeouts are test-only scaffolding.
Cmux Browser Automation Off-Main ✅ Passed The PR changes Pi CLI and tests only; TerminalController, socket-worker policy, and policy tests are unchanged, with no browser automation routing terms in the diff.
Cmux Expensive Synchronous Load ✅ Passed The Swift diff only adds bounded realpath/stat/access checks for one configured executable; it adds no agent-history load, transcript/JSON parsing, directory scan, or per-record syscall on an inter...
Cmux Cache Substitution Correctness ✅ Passed The production diff only changes Pi environment filtering and cmux executable resolution; it makes no cache substitution in persistence, history, undo, or snapshot paths.
Cmux No Hacky Sleeps ✅ Passed The production diff adds credential allowlisting and executable resolution, with no new or worsened sleeps, timers, polling, or wall-clock waits; timing waits are test-only scaffolding.
Cmux Algorithmic Complexity ✅ Passed The production diff adds a fixed provider-key Set, one existing environment pass, and constant-time executable checks; it adds no nested scalable scans or rescans. New collection scans are test-only.
Cmux Swift Concurrency ✅ Passed The changed Swift files contain embedded TypeScript; added lines only resolve executables and filter environment variables, with no new DispatchQueue, Combine, completion-handler, or fire-and-forge...
Cmux Swift @Concurrent ✅ Passed The Swift diff only changes embedded TypeScript environment and executable logic; no Swift async, nonisolated, @concurrent, actor-isolation, or UI-isolation code changed.
Cmux Swift Package Boundaries ✅ Passed The Swift diff only edits embedded TypeScript raw-string templates used to install the Pi extension; it adds no independently testable Swift domain logic requiring a SwiftPM target.
Cmux Swiftpm Lockfiles ✅ Passed The diff changes only two CLI Swift source files and two Python tests; it contains no SwiftPM, Xcode project, .gitignore, workflow, dependency, or Package.resolved changes.
Cmux Swift Logging ✅ Passed The full PR Swift diff adds no print, Logger, NSLog, file, or diagnostic logging; it only changes environment allowlisting and executable resolution. Existing console.warn is unchanged, and test ou...
Cmux User-Facing Error Privacy ✅ Passed Production changes only alter executable resolution and environment filtering; no new user-facing error, alert, command output, or recovery text exposes provider names, flags, variables, or secrets.
Cmux Full Internationalization ✅ Passed The production diff adds only hook logic, comments, and literal environment/command tokens; tests are exempt, and no user-facing text or localization/web catalog files changed.
Cmux Swiftui State Layout ✅ Passed The Swift diff only edits TypeScript embedded in CLI source generators; it adds no SwiftUI views, observation state, GeometryReader, lazy rows, or render-time state mutation.
Cmux Architecture Rethink ✅ Passed The Swift diff adds a local trust resolution and explicit Stop-only environment flag; it adds no timing repair, observer, lock, side channel, duplicate entrypoint, or split UI lifecycle.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only generated Pi hook TypeScript strings and Python tests; it adds no standalone window code, and scripts/lint_auxiliary_window_close_shortcuts.py passes.
Cmux Source Artifacts ✅ Passed All four changed paths are tracked Swift/Python source or tests; no artifact paths were added. Test logs, sockets, and executables are runtime files under temporary directories.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The full PR range changes only CLI Swift files and no Swift file under /Sources/; the production-source seam rule therefore does not apply.
Cmux No Ambient Global State ✅ Passed The Swift diff only edits existing CMUXCLI source-template properties; it adds no Swift file-scope function, mutable variable, static namespace type, or singleton.
Title check ✅ Passed The title clearly summarizes the main change: preserving Pi provider credentials for workspace auto-naming.
Description check ✅ Passed The description provides a detailed summary and testing evidence, but it omits the template's demo video, review trigger, and checklist sections.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-8718-pi-workspace-auto-naming-does-not-run-fo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ce-auto-naming-does-not-run-fo

# Conflicts:
#	tests/test_pi_extension_install.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@tests/test_pi_extension_install.py`:
- Around line 182-183: Add a concise comment immediately before the
TemporaryDirectory call explaining that dir="/tmp" keeps the AF_UNIX socket path
short because macOS has a roughly 104-byte path limit and its default TMPDIR may
be deeply nested. Preserve the existing socket_dir and socket_path logic.
- Line 21: Update the Iterator import in tests/test_pi_extension_install.py to
use the collections.abc alias instead of typing.Iterator, preserving all
existing Iterator usage.
🪄 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 Plus

Run ID: 1a9970d1-a794-4154-a47c-9e72ec6904de

📥 Commits

Reviewing files that changed from the base of the PR and between cec7ac3 and c2ddff6.

📒 Files selected for processing (3)
  • CLI/CMUXCLI+PiExtensionSourceDispatch.swift
  • CLI/CMUXCLI+PiExtensionSourcePart1.swift
  • tests/test_pi_extension_install.py

Comment thread tests/test_pi_extension_install.py Outdated
Comment thread tests/test_pi_extension_install.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@CLI/CMUXCLI`+PiExtensionSourcePart1.swift:
- Around line 595-603: Stop treating CMUX_BUNDLED_CLI_PATH as credential-trusted
in trustedBundledCmuxExecutable; derive trustedForCredentials only from an
authenticated bundled-CLI source. In CLI/CMUXCLI+PiExtensionSourceDispatch.swift
lines 426-433, gate forwarding CMUX_SOCKET_PASSWORD and provider credentials on
that stronger trust result. In tests/test_pi_extension_install.py lines
1462-1526, add coverage using an absolute PATH-shadow executable from
CMUX_BUNDLED_CLI_PATH and assert it receives no credentials.
🪄 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 Plus

Run ID: b5c99de6-8af1-45e4-acb6-6cc350573202

📥 Commits

Reviewing files that changed from the base of the PR and between c2ddff6 and 8acfacf.

📒 Files selected for processing (4)
  • CLI/CMUXCLI+PiExtensionSourceDispatch.swift
  • CLI/CMUXCLI+PiExtensionSourcePart1.swift
  • tests/test_pi_extension_dispatch.py
  • tests/test_pi_extension_install.py

Comment thread CLI/CMUXCLI+PiExtensionSourcePart1.swift Outdated
@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pi workspace auto-naming does not run for existing/resumed sessions

3 participants