Repository navigation
Harden docs deployment authentication - #8347
Conversation
|
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:
📝 WalkthroughWalkthroughThe changes pin Bun and Vercel CLI versions for documentation deployment, add scheduled/manual authentication checks, validate workflow configuration in CI, make process-related tests deterministic, refine termination assertions, and update two AppKit sidebar comments. ChangesDocumentation deployment authentication
Fork capability probe tests
Process termination state assertions
Sidebar comment clarifications
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Bunx
participant Vercel
GitHubActions->>Bunx: Install Bun 1.3.14
Bunx->>Vercel: Run vercel@56.3.1 whoami with VERCEL_TOKEN
Vercel-->>GitHubActions: Return authentication result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 23 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR hardens docs deployment authentication by pinning Bun 1.3.14 and
Confidence Score: 5/5Safe to merge — workflow and CI changes are well-tested by guard scripts, Swift changes are mechanical cleanup, and the large SharedLiveAgentIndex refactor is covered by the existing fork-validation test suite. The docs-deploy and health-workflow changes are narrow and guarded by new Python tests. The SharedLiveAgentIndex restructure is a pure de-nesting refactor with no observable behaviour change; the outer guard bindings it removes were unused variables (Xcode 26 warnings). All timing-based Task.sleep assertions are replaced with deterministic PID-file checks, and new .timeLimit(.minutes(1)) annotations replace wall-clock duration comparisons. No blocking runtime primitives, ambient globals, or test seams are introduced in production source. SharedLiveAgentIndex.swift contains a large structural refactor of applyPendingForkValidations; worth a manual pass to confirm the guard-continue flattening preserves all early-return paths, particularly the restorePendingForkValidationsAfterCancellation call sites. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant GH as GitHub Actions
participant DW as docs-deploy-reusable.yml
participant HW as vercel-auth-health.yml
participant Bun as Bun 1.3.14
participant VC as vercel@56.3.1
participant API as Vercel API
Note over HW: Daily cron (06:17 UTC) or workflow_dispatch
GH->>HW: trigger
HW->>Bun: setup-bun (pin 1.3.14)
HW->>VC: "bunx vercel@56.3.1 whoami"
Note over HW,VC: VERCEL_TOKEN via env var only
VC->>API: authenticate
API-->>VC: user info
VC-->>HW: success / failure
Note over DW: On docs deploy trigger
GH->>DW: trigger
DW->>Bun: setup-bun (pin 1.3.14)
DW->>DW: bun install --frozen-lockfile
DW->>DW: write .vercel/project.json
DW->>VC: "bunx vercel@56.3.1 deploy --prod --yes"
Note over DW,VC: VERCEL_TOKEN via env var only (no --token arg)
VC->>API: deploy with CMUX_DOCS_CHANNEL
API-->>VC: deployment URL
VC-->>DW: success / failure
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant GH as GitHub Actions
participant DW as docs-deploy-reusable.yml
participant HW as vercel-auth-health.yml
participant Bun as Bun 1.3.14
participant VC as vercel@56.3.1
participant API as Vercel API
Note over HW: Daily cron (06:17 UTC) or workflow_dispatch
GH->>HW: trigger
HW->>Bun: setup-bun (pin 1.3.14)
HW->>VC: "bunx vercel@56.3.1 whoami"
Note over HW,VC: VERCEL_TOKEN via env var only
VC->>API: authenticate
API-->>VC: user info
VC-->>HW: success / failure
Note over DW: On docs deploy trigger
GH->>DW: trigger
DW->>Bun: setup-bun (pin 1.3.14)
DW->>DW: bun install --frozen-lockfile
DW->>DW: write .vercel/project.json
DW->>VC: "bunx vercel@56.3.1 deploy --prod --yes"
Note over DW,VC: VERCEL_TOKEN via env var only (no --token arg)
VC->>API: deploy with CMUX_DOCS_CHANNEL
API-->>VC: deployment URL
VC-->>DW: success / failure
Reviews (8): Last reviewed commit: "test: keep suite timeout guard determini..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - name: Verify Vercel token | ||
| run: bunx vercel@56.3.1 whoami --token "$VERCEL_TOKEN" | ||
| env: | ||
| VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} |
There was a problem hiding this comment.
--token "$VERCEL_TOKEN" flag is redundant here — the Vercel CLI automatically reads VERCEL_TOKEN from the environment, which is already set in the env: block. Passing the token as a CLI argument expands it into the process argument list, where it is visible to other processes via /proc/PID/cmdline on Linux (even if GitHub Actions masks it in log output). Dropping the flag achieves the same authentication without that exposure.
| - name: Verify Vercel token | |
| run: bunx vercel@56.3.1 whoami --token "$VERCEL_TOKEN" | |
| env: | |
| VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} | |
| - name: Verify Vercel token | |
| run: bunx vercel@56.3.1 whoami | |
| env: | |
| VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} |
![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
🤖 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 `@cmuxTests/WorkspaceForkConversationContextMenuTests.swift`:
- Around line 4853-4866: Update expectProcessExited to poll Darwin.kill(pid, 0)
until it returns ESRCH or a short deadline expires, preserving the latest
errno/result for the final assertion. After the deadline, force-kill the
descendant only as cleanup if it is still running, then assert that the
deadline-bounded poll observed ESRCH rather than relying on the single immediate
probe.
🪄 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: 74b6615b-5f05-4c05-910a-cf177f5ff00e
📒 Files selected for processing (1)
cmuxTests/WorkspaceForkConversationContextMenuTests.swift
| private func expectProcessExited(pidFile: URL) throws { | ||
| let rawPID = try String(contentsOf: pidFile, encoding: .utf8) | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| let pid = try #require(pid_t(rawPID)) | ||
| errno = 0 | ||
| let result = Darwin.kill(pid, 0) | ||
| let processError = errno | ||
| if result == 0 { | ||
| _ = Darwin.kill(pid, SIGKILL) | ||
| } | ||
| #expect( | ||
| result == -1 && processError == ESRCH, | ||
| "The timed-out fork probe must terminate descendant process \(pid)." | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Poll for ESRCH before declaring descendant cleanup failed.
The single immediate probe can observe termination or orphan reaping in progress and fail nondeterministically. Use a short deadline-bounded poll, then force-kill only for cleanup after the deadline.
Proposed fix
- try expectProcessExited(pidFile: childPIDFile)
+ try await expectProcessExited(pidFile: childPIDFile)- private func expectProcessExited(pidFile: URL) throws {
+ private func expectProcessExited(pidFile: URL) async throws {
let rawPID = try String(contentsOf: pidFile, encoding: .utf8)
.trimmingCharacters(in: .whitespacesAndNewlines)
let pid = try `#require`(pid_t(rawPID))
- errno = 0
- let result = Darwin.kill(pid, 0)
- let processError = errno
- if result == 0 {
- _ = Darwin.kill(pid, SIGKILL)
+
+ let clock = ContinuousClock()
+ let deadline = clock.now.advanced(by: .seconds(2))
+ while clock.now < deadline {
+ errno = 0
+ if Darwin.kill(pid, 0) == -1, errno == ESRCH {
+ return
+ }
+ await Task.yield()
}
- `#expect`(
- result == -1 && processError == ESRCH,
- "The timed-out fork probe must terminate descendant process \(pid)."
- )
+
+ _ = Darwin.kill(pid, SIGKILL)
+ Issue.record("The timed-out fork probe must terminate descendant process \(pid).")
}As per coding guidelines, tests must “await real completion signals or deadline-bounded polls of real predicates rather than fixed-duration waits before assertions.”
🤖 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 `@cmuxTests/WorkspaceForkConversationContextMenuTests.swift` around lines 4853
- 4866, Update expectProcessExited to poll Darwin.kill(pid, 0) until it returns
ESRCH or a short deadline expires, preserving the latest errno/result for the
final assertion. After the deadline, force-kill the descendant only as cleanup
if it is still running, then assert that the deadline-bounded poll observed
ESRCH rather than relying on the single immediate probe.
Source: Coding guidelines
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/test_docs_deploy_auth_guard.py (2)
11-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the deployment token mapping.
The deploy command now relies on
VERCEL_TOKENfrom the environment, but this guard only checks that--tokenis absent. It would still pass if the secret-to-environment mapping were accidentally removed.Suggested assertion
self.assertIn("bunx vercel@56.3.1 deploy", workflow) self.assertNotIn("bunx vercel deploy", workflow) + self.assertIn( + "VERCEL_TOKEN: ${{ secrets.vercel_token }}", + workflow, + ) self.assertNotIn("--token", workflow)🤖 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 `@tests/test_docs_deploy_auth_guard.py` around lines 11 - 17, Update test_docs_deploy_uses_pinned_vercel_cli to assert that the deployment workflow maps the Vercel secret to the VERCEL_TOKEN environment variable, while retaining the existing checks for the pinned CLI and absence of --token.
19-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify the schedule is actually daily.
Checking only for
schedule:allows a weekly or monthly cron expression to pass this test, despite the workflow’s daily-probe requirement. Assert the intended cron expression or parse the schedule and validate its cadence.🤖 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 `@tests/test_docs_deploy_auth_guard.py` around lines 19 - 27, Update test_vercel_auth_is_checked_daily to validate that the workflow’s schedule uses the intended daily cron expression, rather than only asserting the presence of “schedule:”. Preserve the existing checks and ensure weekly or monthly schedules fail the test.
🤖 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.
Outside diff comments:
In `@tests/test_docs_deploy_auth_guard.py`:
- Around line 11-17: Update test_docs_deploy_uses_pinned_vercel_cli to assert
that the deployment workflow maps the Vercel secret to the VERCEL_TOKEN
environment variable, while retaining the existing checks for the pinned CLI and
absence of --token.
- Around line 19-27: Update test_vercel_auth_is_checked_daily to validate that
the workflow’s schedule uses the intended daily cron expression, rather than
only asserting the presence of “schedule:”. Preserve the existing checks and
ensure weekly or monthly schedules fail the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f6a86f4-f883-47a0-b6a8-ac214fcadae8
📒 Files selected for processing (3)
.github/workflows/docs-deploy-reusable.yml.github/workflows/vercel-auth-health.ymltests/test_docs_deploy_auth_guard.py
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
1 participant
Summary
Testing
python3 tests/test_docs_deploy_auth_guard.pypython3 -m py_compile tests/test_docs_deploy_auth_guard.pyactionlint .github/workflows/docs-deploy-reusable.yml .github/workflows/vercel-auth-health.yml .github/workflows/ci.ymlbunx vercel@56.3.1 whoami --token "$VERCEL_TOKEN"Context
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Pins Bun 1.3.14 and
vercel@56.3.1for docs deploys, adds a scheduled/on‑demand Vercel auth check, and removes--tokento keep secrets out of process args. Adds per‑suite Swift Testing timeouts with one retry to prevent CI hangs by killing hung process groups.Bug Fixes
bunx vercel@56.3.1 deploy; use envVERCEL_TOKENand remove--token.workflow_dispatch).Refactors
@Sendablehelpers, const‑correctness tweaks, and a nil‑guard in Quick Look.FeatureFlags.swift; update related comments.Written for commit 83a2710. Summary will update on new commits.
Summary by CodeRabbit