Conversation
* `pilot update` (alias `pilot upgrade`) now updates Claude Code alongside Pilot Shell via npm registry lookup * branch isolation toggle replaces worktreeSupport, dispatcher skips branch question when disabled, Settings page restructured into left-vertical-tab sidebar with extracted section components * spec skills step count cut 60→42 (-30%) by merging tiny conditional steps; eliminates user-reported phantom Step 1 → Step 4 gaps; behavior preserved * /spec gains stop-guard objective re-injection and a blocking completion-audit gate before reaching VERIFIED (ported from claude-goal) * spec share URLs replaced with short pilot-shell.com/s/<id> backed by a Vercel Edge endpoint on Upstash Redis (non-spoofable per-IP rate limiting, base64url validation, atomic INCR+EXPIRE, NX collision retry, runtime feedback-payload guard); legacy fragment URLs still decode; dead local paste service removed; new vitest harness in docs/site with 24 tests
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR adds a config migration (v11) renaming ChangesConfig Schema Migration v11
Short URL Sharing System
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant App
participant ShareAPI as /api/share
participant Redis
User->>Browser: Paste /s/:id URL
Browser->>App: Route to /s/:id
App->>ShareAPI: GET /api/share?id=...
ShareAPI->>Redis: Retrieve compressed payload
Redis-->>ShareAPI: Return payload
ShareAPI-->>App: 200 + payload
App->>App: Decompress and render
App-->>Browser: Display shared spec
User->>Browser: Click Send Feedback
Browser->>App: Trigger generateShortFeedbackUrl
App->>App: Compress feedback payload
App->>ShareAPI: POST /api/share
ShareAPI->>Redis: Store payload with 3d TTL
ShareAPI-->>App: 201 + id
App->>App: Construct https://pilot-shell.com/s/<id>
App-->>Browser: Copy URL, show success/fallback toast
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
Claude finished @maxritter's task in 2m 49s —— View job Code Review — PR #144
|
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | Low | Shared.tsx:~170 |
Redundant "reason" in result check — direct result.reason is cleaner |
| 2 | Low | SectionedBlockRenderer.tsx |
Stale open=false local state when expanded transitions from true to undefined could cause unexpected collapse |
| 3 | Low | _lib/util.py:extract_behavior_contract |
Fallback uses substring match where _extract_section_bullets uses exact match |
| 4 | Informational | sharing.ts |
apiBaseUrl controls where the payload is POSTed, but the returned short URL always uses pilot-shell.com — intentional, but worth a docstring note |
All five features are consistent with their plans, tests are comprehensive (19 endpoint tests + 5 feedback-flow tests in docs/site, 145+ new assertions in hooks tests, 164 lines of new migration tests), and the adversarial review findings from the PR description are all resolved. The implementation is ready to merge.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/docusaurus/docs/workflows/spec.md (1)
78-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate toggle name to match the renamed config key.
The config key was renamed from
worktreeSupporttobranchIsolation, but the toggle label still shows "Worktree Support". The description "Worktree and new-branch options" actually confirms the broader scope that "Branch Isolation" captures better.📝 Proposed fix
-| **Worktree Support** | On | Worktree and new-branch options are hidden — implementation always runs on the current branch | +| **Branch Isolation** | On | Worktree and new-branch options are hidden — implementation always runs on the current branch |🤖 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 `@docs/docusaurus/docs/workflows/spec.md` at line 78, The toggle label "Worktree Support" should be updated to "Branch Isolation" to match the renamed config key (from worktreeSupport to branchIsolation); find the table row containing the string "Worktree Support" and replace that label, and ensure the adjacent description text ("Worktree and new-branch options are hidden — implementation always runs on the current branch") still reads correctly or is adjusted to reflect branchIsolation semantics so the UI label and config key (branchIsolation) are consistent.docs/docusaurus/docs/features/console.md (1)
149-149:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate toggle name to match the renamed config key.
The config key was renamed from
worktreeSupporttobranchIsolation(line 192), but the toggle label still shows "Worktree Support". Since the PR rationale states the rename covers "new-branch and worktree creation" (broader than just worktrees), the label should be updated for consistency.📝 Proposed fix
-| **Worktree Support** | On | Asks how to handle branching at `/spec` start | Skips the branch question — changes go on the current branch | +| **Branch Isolation** | On | Asks how to handle branching at `/spec` start | Skips the branch question — changes go on the current branch |🤖 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 `@docs/docusaurus/docs/features/console.md` at line 149, The docs table still labels the toggle "Worktree Support" but the config key was renamed from worktreeSupport to branchIsolation; update the table cell text from "Worktree Support" to "Branch Isolation" (or similar wording matching branchIsolation) so the UI label matches the new config key, and ensure any adjacent explanatory text that references worktreeSupport is updated to branchIsolation as well.
🧹 Nitpick comments (2)
installer/tests/unit/steps/test_config_migration.py (1)
1366-1369: ⚡ Quick winRename the test to match the asserted version.
The method name says v10 but Line 1369 asserts 11, which makes failures/debugging misleading after quick scan.
Suggested tweak
- def test_current_version_is_10(self) -> None: + def test_current_version_is_11(self) -> None: from installer.steps.config_migration import CURRENT_CONFIG_VERSION assert CURRENT_CONFIG_VERSION == 11🤖 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 `@installer/tests/unit/steps/test_config_migration.py` around lines 1366 - 1369, Rename the test function test_current_version_is_10 to reflect the asserted value by updating its name to test_current_version_is_11 so it matches the constant check of CURRENT_CONFIG_VERSION == 11 in installer/tests/unit/steps/test_config_migration.py; this keeps the test name and the assertion consistent and avoids confusion when the test fails.docs/site/src/lib/sharing/sharing.test.ts (1)
30-39: ⚡ Quick winAssert the outbound fetch contract in the success test.
You validate the result shape, but not the request shape. Adding assertions for URL, method, headers, and body on Line 34 will catch API contract regressions early.
Suggested test addition
it("returns short URL on 201", async () => { @@ const result = await generateShortFeedbackUrl(samplePayload, ""); + expect(fetchMock).toHaveBeenCalledWith( + "/api/share", + expect.objectContaining({ + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ data: expect.any(String) }), + }), + ); expect(result).toEqual({ ok: true, url: "https://pilot-shell.com/s/ABCDEFGH", }); });🤖 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 `@docs/site/src/lib/sharing/sharing.test.ts` around lines 30 - 39, The test for generateShortFeedbackUrl validates the response but not the outbound fetch call; update the "returns short URL on 201" test to assert fetch was called with the expected request shape by checking fetchMock was invoked once and that the request URL, HTTP method, headers (e.g., Content-Type/authorization if applicable), and JSON body (matching samplePayload) match the contract used by generateShortFeedbackUrl; use fetchMock.mockResolvedValueOnce as before and inspect fetchMock.mock.calls[0] (or equivalent helper) to compare the URL string, method, serialized body, and headers to prevent API contract regressions.
🤖 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 `@docs/site/package.json`:
- Around line 92-93: The package.json lists "vite" ^5.4.19 and "vitest" ^1.6.1
which are incompatible with current Vitest v4; to fix, either leave versions
as-is (no change) or upgrade both together: update the "vite" and "vitest"
entries to matching, compatible ranges (e.g., bump "vite" to a supported major
for Vitest 4 and set "vitest" to a compatible ^4.x range), run your test suite
and update the lockfile (npm/yarn/pnpm) to ensure dependency resolution, and
adjust any breaking-change updates in test configs (references: the "vite" and
"vitest" dependency keys in package.json).
In `@docs/site/src/components/feedback/SectionedBlockRenderer.tsx`:
- Around line 60-75: The H2/H3 grouping helpers remove the heading block from
the group's blocks so annotations anchored to those heading blocks can't render;
update groupByH2 (and the analogous H3 helper at 77-97) so that when you create
a new group for a heading (e.g., in groupByH2 where you set current = { heading:
block.content, headingBlock: block, blocks: [] }), you also include the heading
block in current.blocks (e.g., push or initialize blocks with [block]) so the
heading remains renderable by BlockRenderer; keep the existing heading and
headingBlock fields and adjust the final filter only if necessary to avoid
dropping groups that contain only a heading.
- Around line 77-97: groupByTaskH3 currently ignores any blocks before the first
"### Task N:" heading; collect those initial blocks into a leading TaskGroup
instead of dropping them. Modify groupByTaskH3 to accumulate blocks into a
temporary prelude array until the first heading match, then when you create the
first TaskGroup (in the branch that sets current), if prelude is non-empty push
a TaskGroup (e.g., number: 0 or null, title: '', headingBlock: null) containing
the prelude blocks before pushing the first matched group; ensure TaskGroup
consumers can handle a null/0 headingBlock/title or adjust fields accordingly so
intro paragraphs, notes, or alternate headings are preserved.
- Around line 177-182: The effect that syncs forceOpenBlockId with
selectedAnnotationId never clears the forced block when selection is removed, so
update the useEffect that references selectedAnnotationId, annotations, ann,
setForceOpenBlockId and forceOpenBlockId to explicitly setForceOpenBlockId(null)
when selectedAnnotationId is falsy (or when the matching annotation is not
found); keep the existing behavior of setting the block id when an annotation is
found and ensure both branches (found vs not found / no selection) are handled
in the effect.
In `@docs/site/src/lib/sharing/sharing.ts`:
- Around line 80-89: The POST fetch to `${apiBaseUrl}/api/share` can hang
indefinitely; wrap the request with an AbortController and a timeout: create an
AbortController before calling fetch, pass controller.signal to fetch, schedule
a setTimeout to call controller.abort() after a chosen ms timeout, and clear the
timeout when fetch completes; update the catch block to detect an abort/timeout
(e.g., AbortError or signal.aborted) and return { ok: false, reason: "timeout" }
whereas other errors still return { ok: false, reason: "network" }; apply these
changes around the existing variables and symbols (apiBaseUrl, compressed, res)
and the current try/catch that performs fetch.
In `@docs/site/src/pages/Shared.tsx`:
- Around line 168-195: When generateShortFeedbackUrl(feedbackPayload) returns
ok, don't let navigator.clipboard.writeText rejection be handled by the outer
catch that treats the whole operation as a generation failure; instead, after
confirming result.ok and before writing to clipboard, separate the clipboard
write into its own try/catch so that on clipboard failure you still show the
successful feedback URL (result.url) to the user and a descriptive toast (e.g.,
"URL copied to clipboard failed — tap to copy" or include the URL in the
toast/modal) while logging the clipboard error; ensure the outer catch only
handles failures from generateShortFeedbackUrl and use the existing toast()
function to surface both success-with-manual-copy and true generation errors.
- Around line 36-49: The regex PILOT_SHELL_SHORT_RE used by extractFromPastedUrl
only matches URLs with an explicit http(s) scheme, so scheme-less pastes like
"pilot-shell.com/s/..." fail; update PILOT_SHELL_SHORT_RE to accept an optional
scheme (e.g. change the start to ^(?:https?:\/\/)? so it matches both
"https://host/s/<id>" and "host/s/<id>"), keep the existing host-agnostic and
id-capture group ([A-Za-z0-9]{8}), and keep the rest of extractFromPastedUrl and
parseHashFragment logic unchanged.
---
Outside diff comments:
In `@docs/docusaurus/docs/features/console.md`:
- Line 149: The docs table still labels the toggle "Worktree Support" but the
config key was renamed from worktreeSupport to branchIsolation; update the table
cell text from "Worktree Support" to "Branch Isolation" (or similar wording
matching branchIsolation) so the UI label matches the new config key, and ensure
any adjacent explanatory text that references worktreeSupport is updated to
branchIsolation as well.
In `@docs/docusaurus/docs/workflows/spec.md`:
- Line 78: The toggle label "Worktree Support" should be updated to "Branch
Isolation" to match the renamed config key (from worktreeSupport to
branchIsolation); find the table row containing the string "Worktree Support"
and replace that label, and ensure the adjacent description text ("Worktree and
new-branch options are hidden — implementation always runs on the current
branch") still reads correctly or is adjusted to reflect branchIsolation
semantics so the UI label and config key (branchIsolation) are consistent.
---
Nitpick comments:
In `@docs/site/src/lib/sharing/sharing.test.ts`:
- Around line 30-39: The test for generateShortFeedbackUrl validates the
response but not the outbound fetch call; update the "returns short URL on 201"
test to assert fetch was called with the expected request shape by checking
fetchMock was invoked once and that the request URL, HTTP method, headers (e.g.,
Content-Type/authorization if applicable), and JSON body (matching
samplePayload) match the contract used by generateShortFeedbackUrl; use
fetchMock.mockResolvedValueOnce as before and inspect fetchMock.mock.calls[0]
(or equivalent helper) to compare the URL string, method, serialized body, and
headers to prevent API contract regressions.
In `@installer/tests/unit/steps/test_config_migration.py`:
- Around line 1366-1369: Rename the test function test_current_version_is_10 to
reflect the asserted value by updating its name to test_current_version_is_11 so
it matches the constant check of CURRENT_CONFIG_VERSION == 11 in
installer/tests/unit/steps/test_config_migration.py; this keeps the test name
and the assertion consistent and avoids confusion when the test fails.
🪄 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: CHILL
Plan: Pro
Run ID: cdafdf4d-e52c-4a7d-8939-d010b0a38254
⛔ Files ignored due to path filters (170)
console/src/services/worker-service.tsis excluded by!console/**console/src/services/worker/http/routes/SettingsRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/ShareRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/utils/shareStore.tsis excluded by!console/**console/src/shared/sharing/index.tsis excluded by!console/**console/src/shared/sharing/sharing.tsis excluded by!console/**console/src/ui/viewer/App.tsxis excluded by!console/**console/src/ui/viewer/hooks/useSettings.tsis excluded by!console/**console/src/ui/viewer/hooks/useStats.tsis excluded by!console/**console/src/ui/viewer/views/Requirements/index.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/index.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/ConsoleSection.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/ModelsSection.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/SecuritySection.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/SpecWorkflowSection.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/_shared.tsxis excluded by!console/**console/src/ui/viewer/views/Shared/FeedbackImportView.tsxis excluded by!console/**console/src/ui/viewer/views/Shared/SharedSpecView.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/SpecSection.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/SpecTaskCard.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/annotation/PlanAnnotator.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/annotation/SectionedBlockRenderer.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/index.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/parsePlanContent.tsis excluded by!console/**console/src/ui/viewer/views/Spec/sharing/ImportFeedbackDialog.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/sharing/ShareDialog.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/sharing/useShare.tsis excluded by!console/**console/tests/hooks/useSettings.test.tsis excluded by!console/**console/tests/settings-routes.test.tsis excluded by!console/**console/tests/sharing/shareRoutes.test.tsis excluded by!console/**console/tests/sharing/shareStore.test.tsis excluded by!console/**console/tests/ui/use-stats-sse-coalescing.test.tsxis excluded by!console/**docs/site/api/_lib/cors.tsis excluded by!docs/site/api/**docs/site/api/share.test.tsis excluded by!docs/site/api/**docs/site/api/share.tsis excluded by!docs/site/api/**docs/site/package-lock.jsonis excluded by!**/package-lock.jsonlauncher/banner.pyis excluded by!launcher/**launcher/build.pyis excluded by!launcher/**launcher/cli.pyis excluded by!launcher/**launcher/config.pyis excluded by!launcher/**launcher/model_config.pyis excluded by!launcher/**launcher/tests/unit/test_banner.pyis excluded by!launcher/**launcher/tests/unit/test_build.pyis excluded by!launcher/**launcher/tests/unit/test_cli.pyis excluded by!launcher/**launcher/tests/unit/test_model_config.pyis excluded by!launcher/**launcher/tests/unit/test_updater.pyis excluded by!launcher/**launcher/tests/unit/test_wrapper.pyis excluded by!launcher/**launcher/updater.pyis excluded by!launcher/**launcher/wrapper.pyis excluded by!launcher/**pilot/hooks/_lib/util.pyis excluded by!pilot/**pilot/hooks/hooks.jsonis excluded by!pilot/**pilot/hooks/spec_stop_guard.pyis excluded by!pilot/**pilot/hooks/spec_verify_validator.pyis excluded by!pilot/**pilot/hooks/tests/test_spec_stop_guard.pyis excluded by!pilot/**pilot/hooks/tests/test_spec_validators.pyis excluded by!pilot/**pilot/hooks/tests/test_spec_verify_validator.pyis excluded by!pilot/**pilot/hooks/tests/test_tool_redirect.pyis excluded by!pilot/**pilot/hooks/tests/test_util_plan_parsing.pyis excluded by!pilot/**pilot/hooks/tool_redirect.pyis excluded by!pilot/**pilot/rules/cli-tools.mdis excluded by!pilot/**pilot/rules/task-and-workflow.mdis excluded by!pilot/**pilot/rules/testing.mdis excluded by!pilot/**pilot/scripts/mcp-server.cjsis excluded by!pilot/**pilot/scripts/worker-service.cjsis excluded by!pilot/**pilot/settings.jsonis excluded by!pilot/**pilot/skills/create-skill/steps/01-reference.mdis excluded by!pilot/**pilot/skills/create-skill/steps/02-understand.mdis excluded by!pilot/**pilot/skills/create-skill/steps/03-check.mdis excluded by!pilot/**pilot/skills/create-skill/steps/04-create.mdis excluded by!pilot/**pilot/skills/create-skill/steps/05-quality.mdis excluded by!pilot/**pilot/skills/create-skill/steps/06-test.mdis excluded by!pilot/**pilot/skills/create-skill/steps/07-anti-patterns.mdis excluded by!pilot/**pilot/skills/create-skill/steps/08-example.mdis excluded by!pilot/**pilot/skills/prd/manifest.jsonis excluded by!pilot/**pilot/skills/prd/steps/01-understand.mdis excluded by!pilot/**pilot/skills/prd/steps/02-research.mdis excluded by!pilot/**pilot/skills/prd/steps/03-ideate.mdis excluded by!pilot/**pilot/skills/prd/steps/04-clarify.mdis excluded by!pilot/**pilot/skills/prd/steps/05-propose.mdis excluded by!pilot/**pilot/skills/prd/steps/06-converge.mdis excluded by!pilot/**pilot/skills/prd/steps/07-write-prd.mdis excluded by!pilot/**pilot/skills/prd/steps/08-handoff.mdis excluded by!pilot/**pilot/skills/prd/steps/09-anti-patterns.mdis excluded by!pilot/**pilot/skills/prd/steps/10-key-principles.mdis excluded by!pilot/**pilot/skills/setup-rules/steps/01-reference.mdis excluded by!pilot/**pilot/skills/setup-rules/steps/02-read.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/manifest.jsonis excluded by!pilot/**pilot/skills/spec-bugfix-plan/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/00-setup-and-red-flags.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/00-toggles.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/01-create-header.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/02-investigation.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/03-plan-fix.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/04-write-plan.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/05-annotation-check.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/06-approval.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/08-continuing.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/manifest.jsonis excluded by!pilot/**pilot/skills/spec-bugfix-verify/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/01-test-suite.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/01-verify-fix.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/02-quality-checks.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/03-verification-scenario.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/04-final-worktree-sync.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/05-check-feedback.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/06-code-review-gate.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/07-update-status.mdis excluded by!pilot/**pilot/skills/spec-implement/manifest.jsonis excluded by!pilot/**pilot/skills/spec-implement/steps/01-read-plan.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/01-setup.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/02-detect-worktree.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/02-tdd-loop.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/03-completion.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/03-setup-tasks.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/05-update-plan.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/06-completion.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/07-migration-additions.mdis excluded by!pilot/**pilot/skills/spec-plan/manifest.jsonis excluded by!pilot/**pilot/skills/spec-plan/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/00-setup-and-questions.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/00-toggles.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/01-special-cases.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/02-create-header.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/02-extending-plans.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/03-migration-tasks.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/03-workspace-scan.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/04-task-understanding.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/05-exploration.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/06-approach.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/07-approach.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/07-implementation-planning.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/08-e2e-scenarios.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/08-implementation-planning.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/09-write-plan.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/10-plan-verification.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/10-write-plan.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/11-annotation-check.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/12-approval.mdis excluded by!pilot/**pilot/skills/spec-verify/manifest.jsonis excluded by!pilot/**pilot/skills/spec-verify/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/00-setup-and-phase-plan.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/00-toggles.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/01-launch-review.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/01-process-overview.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/02-automated-checks.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/02-classify-runtime.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/03-collect-results.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/03-phase-a-cleanup-stale.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/04-phase-b-build.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/05-execution.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/06-dod-and-gaps.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/06-feature-parity.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/07-e2e-and-final-regression.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/08-reverify.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/08-worktree-sync-and-post-merge.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/09-check-feedback.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/10-review-gate.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/11-dod-audit.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/11-update-status.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/12-not-verified.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/14-final-regression.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/16-post-merge.mdis excluded by!pilot/**pilot/skills/spec/orchestrator.mdis excluded by!pilot/**pilot/skills/spec/steps/00-permission-mode.mdis excluded by!pilot/**pilot/skills/spec/steps/01-parse-route.mdis excluded by!pilot/**pilot/ui/PlanAnnotator.jsis excluded by!pilot/**pilot/ui/index3.jsis excluded by!pilot/**pilot/ui/index5.jsis excluded by!pilot/**pilot/ui/viewer-bundle.jsis excluded by!pilot/**pilot/ui/viewer.cssis excluded by!pilot/**
📒 Files selected for processing (14)
docs/docusaurus/docs/features/cli.mddocs/docusaurus/docs/features/console.mddocs/docusaurus/docs/workflows/spec.mddocs/site/package.jsondocs/site/src/App.tsxdocs/site/src/components/feedback/SectionedBlockRenderer.tsxdocs/site/src/lib/sharing/compress.tsdocs/site/src/lib/sharing/index.tsdocs/site/src/lib/sharing/sharing.test.tsdocs/site/src/lib/sharing/sharing.tsdocs/site/src/pages/Shared.tsxdocs/site/vercel.jsoninstaller/steps/config_migration.pyinstaller/tests/unit/steps/test_config_migration.py
* sharing.ts: add 10s timeout/AbortController to share POST; distinguish timeout from network errors with new ShortShareResult reason * Shared.tsx: separate clipboard write try/catch so a successful generation isn't lost when clipboard rejects; show URL in toast for manual copy * Shared.tsx: scheme-less paste regex (`pilot-shell.com/s/<id>` without `https://`) now resolves correctly instead of falling through to parse-error toast * Shared.tsx: surface new "timeout" failure with a dedicated toast * SectionedBlockRenderer (both docs/site + console copies): include heading blocks in their groups so annotations anchored to H2/H3 headings still render and selectedAnnotationId auto-expansion can find them * SectionedBlockRenderer: prelude TaskGroup (number=0, headingBlock=null) preserves content appearing before the first `### Task N:` heading; consumers null-check and render the prelude inline without a card * SectionedBlockRenderer: clear forceOpenBlockId when selection is removed so old enclosing cards no longer stay permanently open
|
Claude finished @maxritter's task in 3m 2s —— View job Incremental Code Review — PR #144
✅ Resolved in this push
🆕 New issue introduced in this push
The fix that includes the heading block in
For task sections the problem compounds: Minimal fix — strip heading blocks from the rendered lists while keeping them in // In the non-task render path (line ~296):
renderLeaf(section.blocks.filter(b => b !== section.headingBlock))
// In groupByTaskH3 call (line ~240):
groupByTaskH3(section.blocks.filter(b => b !== section.headingBlock))
// In renderLeaf per task (inside CollapsibleCard for tasks):
renderLeaf(task.blocks.filter(b => b !== task.headingBlock))
Still open (prior reviews)
The timeout fix and clipboard isolation are clean. The one blocking concern for this push is the heading duplication — it will visibly double-render every section and task title in the shared spec viewer. |
Owner
Author
|
@coderabbitai resolve All 6 actionable findings from the previous review are addressed in commit
Verification: docs/site 24/24 vitest pass, build clean, typecheck clean. Console 304/304 sharing+ui tests pass, typecheck clean. |
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Actions performedComments resolved. Approval is disabled; enable |
Contributor
|
🎉 This PR is included in version 8.9.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five user-facing features verified via /spec on the dev branch. All non-breaking.
pilot updateupdates Claude Code toopilot update(aliaspilot upgrade) now updates Claude Code alongside Pilot Shell via the npm registry. Run it once, get both up-to-date.Branch toggle + Settings UI overhaul
[Plan:
docs/plans/2026-05-13-branch-toggle-and-settings-ui.md]worktreeSupportrenamed tobranchIsolation(covers both new-branch and worktree creation)Spec skills step consolidation
[Plan:
docs/plans/2026-05-13-spec-skills-step-consolidation.md]spec/dispatcher andfix/skill untouched (already lean)/spec process improvements (ported from claude-goal)
[Plan:
docs/plans/2026-05-13-spec-process-improvements-from-claude-goal.md]Status: VERIFIEDSpec share URL shortener
[Plan:
docs/plans/2026-05-13-spec-share-url-shortener.md]Replace the 30 KB-fragment share URL with a short
https://pilot-shell.com/s/<id>link:docs/site/api/share.tsbacked by Upstash Redis (the trial endpoint's existing store; no new infra)EXPIRE … NX), NX collision retry/s/:idroute + fetch-by-ID; landing-page paste input recognizes the new format (host-agnostic regex)ShareDialogsimplified (no toggle),useShare.tssingle POST flow,ImportFeedbackDialogrecognizespilot-shell.com/s/<id>with runtimeisFeedbackPayloadguard (rejects empty objects, malformed annotations, share-payloads-pasted-as-feedback)pilot-shell.com/shared#<fragment>URLs still decode (no broken in-flight links)ShareRoutes,ShareStore, tests,isPasteServiceIdcallers across 4 files)docs/sitewith 19 endpoint tests + 5 feedback-flow testscompress.tsUint8Array typing issue fixed as part of the cleanupVerification
/s/<id>SPA route render, legacy fragment URL back-compat (zero/api/sharecalls), feedback round-trip with runtime payload guardReviewer findings resolved
Two adversarial reviewers (Claude + Codex) flagged 9 issues during /spec verify — all resolved before merge:
x-forwarded-forentry was spoofable → fixed viax-real-ipwith last-XFF fallbackisFeedbackPayloadruntime guardSummary by CodeRabbit
Documentation
New Features
Bug Fixes
Tests
Chores