The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
pilot-shell	Ignored	Preview	Mar 31, 2026 11:19am

Walkthrough

Renames reviewer agent keys and UI text to Spec/Changes Review, adds optional Codex adversarial reviewers, reduces hook count to 15, adds an annotation UI and secure share/feedback flow to the docs site, and implements config migration to version 7 plus installer adjustments.

Changes

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Browser as Browser/App
    participant Parser
    participant Renderer as BlockRenderer
    participant Annotations
    participant Compress
    participant Crypto
    participant Share as Sharing

    User->>Browser: Load shared URL / paste hash
    Browser->>Browser: parseHashFragment(hash) → {data, key}
    Browser->>Crypto: decrypt(data, key)
    Crypto-->>Browser: compressed bytes (plaintext)
    Browser->>Compress: decompress(bytes)
    Compress-->>Browser: JSON payload (SharePayload/FeedbackPayload)
    Browser->>Parser: parseMarkdownToBlocks(specContent)
    Parser-->>Browser: Block[]
    Browser->>Renderer: render Block[] + sharer annotations
    Renderer-->>Browser: interactive UI

    User->>Browser: select text
    Browser->>Annotations: handleMouseUp -> pendingSelection
    Annotations-->>Browser: show AnnotationToolbar
    User->>Browser: submit annotation
    Browser->>Annotations: createAnnotation -> update state
    Annotations-->>Renderer: re-render with new annotation

    User->>Browser: send feedback
    Browser->>Share: generateWebFeedbackUrl(feedbackPayload)
    Share->>Compress: compress(JSON)
    Compress-->>Share: compressed
    Share->>Crypto: encrypt(compressed)
    Crypto-->>Share: {ciphertext, key}
    Share-->>Browser: {url, key} (copied/shared)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: Codex adversarial reviewers, spec sharing, and Console improvements #123: similar reviewer-key renames and codexReviewers additions; closely related migration and docs changes.
• feat: redesign extensions page with color-coded categories, project deletion, and console improvements #101: related installer migration and documentation edits touching reviewer config and Console docs.
• feat: replace Teams with Skillshare-based Share system, streamline spec workflow and hooks #95: overlaps on installer config migration and related tests.

Suggested labels

released

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 @coderabbitai help to get the list of available commands and usage tips.}

Claude finished @maxritter's task in 3m 4s —— View job

Code Review — feat: Codex adversarial reviewers, spec sharing, and Console improvements

• Gather context and read CLAUDE.md
• Review installer changes (config_migration.py, dependencies.py, finalize.py, tests)
• Review docs/site sharing & annotation library
• Review docs/site UI components (feedback/, pages/Shared.tsx)
• Review documentation updates (docs/docusaurus/, README.md)
• Post final review

Overall this is a well-structured PR with a clean migration path, good test coverage for migrations, and a solid E2E-encrypted sharing implementation. A few issues worth addressing below.

installer/steps/config_migration.py

Dead-code if modified: guard (line 77)

raw["_configVersion"] = CURRENT_CONFIG_VERSION
modified = True           # always True here

if modified:              # always executes — condition is dead code
    _write_atomic(config_path, raw)

modified is unconditionally set to True on line 75, so the if modified: guard on line 77 never prevents the write. The intent is presumably "only write if something changed," but modified = True overrides any earlier False. The file is always written whenever migrations ran (which is correct), but the guard is misleading. Fix this →

docs/site/src/lib/sharing/sharing.ts

DRY violation: generateWebShareUrl and generateWebFeedbackUrl are identical

Both functions have the exact same body — compress, encrypt, size-check, and return the URL. The only difference is the input type (SharePayload vs FeedbackPayload). Consider consolidating into one generic helper, or at minimum extracting the shared body to reduce the maintenance surface. Fix this →

docs/site/src/pages/Shared.tsx

extractFromPastedUrl — both branches are identical

function extractFromPastedUrl(input: string): { data: string; key: string } | null {
  const trimmed = input.trim();
  if (!trimmed) return null;
  if (trimmed.startsWith("#")) return parseHashFragment(trimmed); // same as below
  return parseHashFragment(trimmed);                               // same as above
}

The if (trimmed.startsWith("#")) branch is unreachable dead code — parseHashFragment already handles #-prefixed inputs correctly. The function reduces to two lines. Fix this →

Hardcoded production URL in handleSendFeedback

const baseUrl = "https://pilot-shell.com/shared";

This is buried inside a useCallback. If the domain ever changes, this won't be caught by TypeScript — consider hoisting it to a module-level constant alongside MAX_INLINE_BYTES.

onSelectAnnotation no-op in the spec viewer

onSelectAnnotation={() => {}}

Clicking annotation badges in the viewer silently does nothing. If annotation selection is not yet wired to the sidebar in read-only mode, a // TODO or disabling the button entirely would be clearer than a silent no-op.

docs/site/src/components/feedback/BlockRenderer.tsx

allAnnotations passed to every BlockItem for index lookup

globalIndex is computed via allAnnotations.findIndex() per-badge, which is O(n) for each annotation badge rendered. For any reasonable spec size this is fine, but worth noting since the full list is passed to every block. A pre-computed index map passed down would be more efficient.

installer/steps/finalize.py

kill -9 without SIGTERM first

subprocess.run(["kill", "-9", pid.strip()], ...)

SIGKILL prevents the worker from flushing state or releasing file locks. Sending SIGTERM first (with a short wait) would be more graceful. Fix this →

docs/site/src/lib/annotation/parser.ts

Module-level mutable counter

let blockIdCounter = 0;
// ...
export function parseMarkdownToBlocks(markdown: string): Block[] {
  blockIdCounter = 0;  // reset on each call

This works correctly in a single-threaded browser context but is fragile in test environments where multiple concurrent calls could occur. No change required unless test reliability issues emerge, just worth being aware of.

Crypto/Sharing — Good Practices Observed

• AES-256-GCM with a random 96-bit IV per encryption: correct
• Key exported to raw bytes and embedded in the URL fragment (never sent to server): correct
• checkBrowserSupport() computed at module load for stable error handling: good
• Compression before encryption (not after) is the correct order for size reduction: good
• Fragment-based URL format (#ciphertext?key=k) is correctly parsed by parseHashFragment: verified

Migration v7 — Well Structured

The migration chain is clean and the test coverage for v7 is thorough. The key rename logic correctly handles all cases: keys present, missing, or already renamed.

Minor Nits

• docs/docusaurus/docs/getting-started/prerequisites.md: claude plugin install @openai/codex — worth confirming this is the final published package name before merging
• docs/site/src/lib/sharing/compress.ts: Browser support comment accurately calls out the Uint8Array.toBase64 minimum versions — the runtime check in Shared.tsx covers this correctly

Claude finished @maxritter's task in 1m 38s —— View job

Incremental Code Review — Push Delta

• Gather context
• Review installer/steps/config_migration.py
• Review installer/steps/finalize.py + test_finalize.py
• Review docs/site/src/lib/sharing/sharing.ts
• Review docs/site/src/pages/Shared.tsx
• Post final review

✅ Resolved from prior review

All five flagged issues are fixed in this push:

• ✅ config_migration.py dead-code guard — modified = True is now conditional on the version actually needing a bump (if raw.get("_configVersion") != CURRENT_CONFIG_VERSION:), making the if modified: write guard meaningful again.
• ✅ sharing.ts DRY violation — generateWebShareUrl and generateWebFeedbackUrl now both delegate to a private buildEncryptedUrl(payload, baseUrl) helper. Clean extraction.
• ✅ Shared.tsx dead branch in extractFromPastedUrl — simplified to return parseHashFragment(trimmed) after the null guard.
• ✅ Shared.tsx hardcoded production URL in handleSendFeedback — removed; now uses the module-level SHARE_BASE_URL constant.
• ✅ finalize.py kill -9 without SIGTERM — now sends SIGTERM (kill <pid>), sleeps 1s, checks liveness with kill -0, and only sends SIGKILL if the process is still alive. Test coverage for both the SIGTERM-only and SIGTERM+SIGKILL paths is solid.

New observations

sharing.ts — decryptSharePayload / decryptFeedbackPayload still have identical bodies

With the new decryptHashPayload (which handles auto-detection), the two individual typed decrypt functions now have duplicate implementations:

// Both have exactly the same body — only the cast differs
const compressed = await decrypt(data, key);
return JSON.parse(await decompress(compressed)) as SharePayload; // or FeedbackPayload

These were not flagged in the prior review, but since decryptHashPayload was added in this push, the duplication is newly visible. If callers need typed decryption, consider having them call decryptHashPayload and narrow on .type, or extract a private _decryptRaw helper the typed functions delegate to. Not blocking.

Shared.tsx line 19 — Vite define self-reference pattern

const SHARE_BASE_URL = SHARE_BASE_URL;

This relies on Vite's define plugin replacing the RHS with a string literal before the TypeScript compiler sees it. It works, but it's fragile — if the define is ever missing from vite.config.ts, this becomes a ReferenceError at runtime with no compile-time warning. The prior version with an inline hardcoded string was easier to diagnose when misconfigured. Consider at minimum adding a declare const SHARE_BASE_URL: string; ambient declaration so TypeScript catches a missing define at build time.

Still open (unchanged from prior review)

• onSelectAnnotation={() => {}} no-op on BlockRenderer (Shared.tsx:341) — annotation badge clicks silently do nothing in read-only view; a // TODO or disabled state would be clearer.

🎉 This PR is included in version 7.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀