Conversation
…ject level Also compressed standards
Replace Endless Mode's kill-and-restart cycle with Claude Code's built-in
compaction mechanism using PreCompact and SessionStart(compact) hooks for
seamless context preservation and restoration.
Key changes:
- New PreCompact hook captures session state to Pilot Memory before compaction
- New SessionStart(compact) hook re-injects plan state, task progress, and
working files after compaction completes
- Wrapper stripped of all Endless Mode restart logic, retains license/update/
crash-recovery duties only
- Installer no longer overwrites user settings — uses project-level config
- Rules consolidated from 20+ files into 10 focused modules (cli-tools,
development-practices, research-tools, testing, verification, etc.)
- /spec commands streamlined with clearer phase dispatch and verification
- Tool redirect hook updated with relaxed agent tooling restrictions
- Context monitor simplified by removing unreliable session ID cross-checks
- Blog content updated to reflect compaction-based architecture
- Website copy refreshed across all sections
Plans: docs/plans/2026-02-16-compaction-based-endless-mode.md
docs/plans/2026-02-16-issue-54-friction-fixes.md
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Claude finished @maxritter's task in 2m 23s —— View job Code Review CompleteReviewed PR #56: "We Make Pilots Great Again" (100 files, +2793/-5892) SummaryThis is a well-executed comprehensive release that addresses user feedback and introduces compaction-based endless mode. The changes are architecturally sound with good test coverage. I found several minor issues and recommendations but no blocking concerns. Major Changes Reviewed✅ Compaction-Based Endless ModeFiles: Strengths:
Issues Found:
✅ Installer ImprovementsFile: Strengths:
Issues Found:
✅ Rules ConsolidationFiles: Consolidated 20+ files → 10 focused modules Strengths:
Issues Found:
|
|
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:
WalkthroughThis PR replaces "Endless Mode" terminology with "Auto-Compaction" and "Persistent Memory" throughout documentation and UI components. It updates the installer to use project-local settings instead of global settings, removes mcp_servers.json generation, adds license gating to the console viewer, and enhances the installation script with wrapper-based execution and local installer support. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as App Component
participant License as useLicense Hook
participant API as License API
participant Gate as LicenseGate
User->>App: Load Console Viewer
App->>License: useLicense()
License->>API: GET /api/license (periodic refresh)
API-->>License: License Data
License-->>App: License State + Loading
alt License Data Loading
App->>User: Show Loading Indicator
else License Invalid/Expired
App->>Gate: Render LicenseGate
User->>Gate: Enter License Key
Gate->>API: POST /api/license/activate
API-->>Gate: Activation Success/Error
Gate->>App: onActivated()
App->>User: Render Main App
else License Valid
App->>User: Render Main App
end
Note over License: Auto-refresh every 60s
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
installer/steps/dependencies.py (1)
169-173:⚠️ Potential issue | 🟡 MinorFallback version string may be misleading when npm install fails.
When
_run_bash_with_retryfails butclaudeexists, line 172 returnsactual_version or version. If_get_installed_claude_version()returnsNone(e.g.,claude --versionoutput is unexpected), the fallback isversion— which is the forced version that just failed to install. Downstream code and the user message at line 532 would then report this forced version as installed, even though the actual installed version may differ.Proposed fix
if not _run_bash_with_retry(npm_cmd): if command_exists("claude"): actual_version = _get_installed_claude_version() - return True, actual_version or version + return True, actual_version or "unknown" return False, version
🤖 Fix all issues with AI agents
In `@docs/site/src/components/DeepDiveSection.tsx`:
- Line 170: The subtitle in DeepDiveSection.tsx is wrong: update the text that
currently reads "12 hooks across 6 lifecycle events — fire automatically at
every stage" to reflect the actual count from the hooksPipeline array (3 + 3 + 4
+ 1 + 2 + 2 = 15). Locate the subtitle string in the DeepDiveSection component
and change it to "15 hooks across 6 lifecycle events — fire automatically at
every stage" (or equivalent wording that uses 15) so it matches the
hooksPipeline definition.
In `@docs/site/src/content/blog/claude-code-hooks-guide.md`:
- Around line 163-165: The SessionEnd hook description is incorrect; update the
text for the SessionEnd item so it states that SessionEnd stops the worker when
no other sessions are active and sends session completion notifications (instead
of saying it saves session state for compaction); also, ensure the
compaction-related responsibilities are attributed to PreCompact (captures state
before compaction) and SessionStart(compact) (restores state after compaction)
so the list accurately reflects SessionEnd, PreCompact, and
SessionStart(compact).
In `@docs/site/src/content/blog/managing-context-long-sessions.md`:
- Line 104: Update the sentence "At 90%, auto-compaction triggers and state is
preserved via hooks." to clarify that the 90% threshold only fires Pilot's
state-preservation hooks in preparation for Claude Code's built-in
auto-compaction at roughly 95% (e.g., reword to "At 90%, state-preservation
hooks fire to prepare for Claude Code's built-in auto-compaction at ~95%"). Also
scan nearby lines (the earlier mention of "roughly 95% full") to ensure the two
sentences consistently state that Pilot triggers hooks at 90% while Claude Code
performs the actual compaction around 95%.
In `@README.md`:
- Line 434: Update the table row that currently reads the Backend cell as
"`**/models/**`, `**/routes/**`, `**/api/**`, etc" to use "etc." (add the
missing period). Locate the row containing the pipe-delimited entry beginning
with "| Backend | `**/models/**`, `**/routes/**`, `**/api/**`, etc |" and change
"etc" to "etc." so the sentence conforms to American English punctuation.
🧹 Nitpick comments (3)
installer/steps/claude_files.py (3)
484-512: Unusedctxparameter in_merge_app_config.Ruff correctly flags that
ctxis unused (ARG002). This method only reads fromPath.home()and the installed template path — it never accessesctx. Consider removing it or using it to derive paths for consistency with the other new methods.Proposed fix
- def _merge_app_config(self, ctx: InstallContext) -> None: + def _merge_app_config(self) -> None:And update the call site at line 440:
- self._merge_app_config(ctx) + self._merge_app_config()
53-80:patch_global_settingsmutates the input dict — document or make explicit.Both
patch_global_settingsandmerge_app_configmutate the passed-in dicts in place and then return them (orNone). The callers already rely on this behavior, but it's a subtle API contract. The current callers (_patch_overlapping_settingsat line 540 and_merge_app_configat line 507) work correctly since they only use the return value for theis not Nonecheck, but the mutation-based design means the original dicts are always modified regardless of the return value.This is currently safe because the dicts are freshly parsed from JSON on every call, but worth noting if these helpers are ever reused in a context where the caller needs the original dict preserved.
20-20:SETTINGS_FILEconstant is defined but unused; hardcoded string appears in multiple locations.Line 20 defines
SETTINGS_FILE = "settings.local.json", but this constant is never used in the file. The hardcoded string appears at both line 425 and line 521. Use the constant in both locations for consistency.Proposed fix
elif category == "settings": - return ctx.project_dir / ".claude" / "settings.local.json" + return ctx.project_dir / ".claude" / SETTINGS_FILElocal_settings_path = ctx.project_dir / ".claude" / "settings.local.json" + local_settings_path = ctx.project_dir / ".claude" / SETTINGS_FILE
| ### 1. Context Monitoring | ||
|
|
||
| A background hook continuously tracks context usage percentage. At 80%, it warns that context is getting high. At 90%, it triggers the handoff protocol. | ||
| A background hook continuously tracks context usage percentage. At 80%, it warns that context is getting high. At 90%, auto-compaction triggers and state is preserved via hooks. |
There was a problem hiding this comment.
Minor: Clarify the 90% threshold vs. actual autocompaction trigger.
Line 26–28 of this same post explains that Claude Code triggers autocompaction at "roughly 95% full." Line 104 then says "At 90%, auto-compaction triggers and state is preserved via hooks." This could imply Pilot itself triggers compaction at 90%, when in reality the hooks fire at 90% to prepare for Claude Code's built-in compaction at ~95%. Consider rewording to something like "At 90%, state preservation hooks fire in preparation for auto-compaction."
🤖 Prompt for AI Agents
In `@docs/site/src/content/blog/managing-context-long-sessions.md` at line 104,
Update the sentence "At 90%, auto-compaction triggers and state is preserved via
hooks." to clarify that the 90% threshold only fires Pilot's state-preservation
hooks in preparation for Claude Code's built-in auto-compaction at roughly 95%
(e.g., reword to "At 90%, state-preservation hooks fire to prepare for Claude
Code's built-in auto-compaction at ~95%"). Also scan nearby lines (the earlier
mention of "roughly 95% full") to ensure the two sentences consistently state
that Pilot triggers hooks at 90% while Claude Code performs the actual
compaction around 95%.
…ture Auto-compaction is a built-in Claude Code feature. What Pilot adds are hooks (PreCompact + SessionStart(compact)) that capture and restore Pilot-specific state, and persistent memory that carries context across sessions. Updated all website copy, testimonials, FAQ, README comparison table, and agent descriptions to correctly attribute each. Also fixes: hook count 12→15, SessionEnd hook description, 90%→83% compaction threshold, unused ctx parameter, SETTINGS_FILE constant usage.
|
Claude finished @maxritter's task in 1m 24s —— View job Incremental Code Review — PR #56Reviewing changes from commit ✅ Issues Resolved1. Hook Count Fixed ✅
2. SessionEnd Hook Description Corrected ✅
3. Context Threshold Clarified ✅
4. Installer Code Quality Improvements ✅
5. Minor Punctuation Fix ✅
🔍 New Issues FoundNone — This commit is a clean-up pass that addresses all the documentation and code quality issues from the previous review. No new problems introduced. 📋 Still Open from Previous ReviewThe following issues from CodeRabbit's initial review remain unaddressed (not touched by this commit):
These are all minor/suggestions from the original review and don't block merging. ✨ SummaryThis follow-up commit (
The changes are accurate, internally consistent, and well-targeted. No new issues introduced. The remaining open items from the previous review are implementation-level concerns in files not touched by this commit. Recommendation: Approve this commit. The remaining open items can be addressed in follow-up work if needed. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/site/src/content/blog/claude-code-hooks-guide.md`:
- Line 163: Update the description for the "Context Monitor (PostToolUse)" entry
to use the consistent auto-compaction threshold of ~83% instead of 90%; ensure
the text in docs/site/src/content/blog/claude-code-hooks-guide.md matches the
references in DeepDiveSection.tsx and managing-context-long-sessions.md so all
mention the ~83% trigger for preparing auto-compaction.
In `@installer/steps/claude_files.py`:
- Around line 546-563: _cleanup_stale_rules currently only unlinks files and
ignores stale subdirectories under global_rules_dir; update the function to also
detect item.is_dir() and, for directories whose resolved path is not in the
installed set, remove them (use shutil.rmtree for non-empty dirs inside the same
try/except block used for item.unlink()) while preserving the existing
installed-path resolution logic and error swallowing behavior; reference the
global_rules_dir, installed set, and item variables when making the change.
In `@README.md`:
- Around line 378-383: The README incorrectly states auto-compaction fires at
"~83% context"; update that wording to "80% context" to match the implementation
constant THRESHOLD_AUTOCOMPACT in pilot/hooks/context_monitor.py so
documentation and code are consistent.
🧹 Nitpick comments (1)
installer/steps/claude_files.py (1)
53-80:patch_global_settingsmutates its input dict in-place — document or copy.The function deletes keys from
global_settingsand then returns the same object (orNone). Callers today load a fresh dict from disk, so this works, but the contract is easy to misuse — a caller passing a shared dict would see silent side-effects regardless of the return value.Consider either:
- Documenting the in-place mutation clearly in the docstring, or
- Operating on a shallow copy (
global_settings = copy.copy(global_settings)) so the return value is the only way changes propagate.
|
|
||
| - **TDD Enforcer** (PostToolUse): Reminds Claude to write tests before production code | ||
| - **Context Monitor** (PostToolUse): Tracks context usage and triggers session handoff at 90% | ||
| - **Context Monitor** (PostToolUse): Tracks context usage and prepares for auto-compaction at 90% |
There was a problem hiding this comment.
Inconsistent compaction threshold: 90% here vs. ~83% elsewhere.
This line says the Context Monitor "prepares for auto-compaction at 90%," but DeepDiveSection.tsx (line 61) and managing-context-long-sessions.md (line 104) both reference ~83% as the auto-compaction trigger. The commit messages also mention updating from 90% → 83%. Align this to ~83% for consistency.
Proposed fix
-- **Context Monitor** (PostToolUse): Tracks context usage and prepares for auto-compaction at 90%
+- **Context Monitor** (PostToolUse): Tracks context usage and prepares for auto-compaction at ~83%📝 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.
| - **Context Monitor** (PostToolUse): Tracks context usage and prepares for auto-compaction at 90% | |
| - **Context Monitor** (PostToolUse): Tracks context usage and prepares for auto-compaction at ~83% |
🤖 Prompt for AI Agents
In `@docs/site/src/content/blog/claude-code-hooks-guide.md` at line 163, Update
the description for the "Context Monitor (PostToolUse)" entry to use the
consistent auto-compaction threshold of ~83% instead of 90%; ensure the text in
docs/site/src/content/blog/claude-code-hooks-guide.md matches the references in
DeepDiveSection.tsx and managing-context-long-sessions.md so all mention the
~83% trigger for preparing auto-compaction.
| def _cleanup_stale_rules(self, ctx: InstallContext) -> None: | ||
| """Remove stale rule files from ~/.claude/rules/ not present in this installation. | ||
|
|
||
| ~/.claude/rules/ is purely installer-managed (user rules go in project .claude/rules/). | ||
| Any file there that wasn't just installed is stale from a previous version. | ||
| """ | ||
| global_rules_dir = Path.home() / ".claude" / "rules" | ||
| if not global_rules_dir.exists(): | ||
| return | ||
|
|
||
| installed = {Path(p).resolve() for p in ctx.config.get("installed_files", [])} | ||
|
|
||
| for item in global_rules_dir.iterdir(): | ||
| if item.is_file() and item.resolve() not in installed: | ||
| try: | ||
| item.unlink() | ||
| except (OSError, IOError): | ||
| pass |
There was a problem hiding this comment.
Stale subdirectories in ~/.claude/rules/ won't be cleaned up.
The loop on line 558 only removes files (item.is_file()). If a previous installer version created rule subdirectories (e.g., rules/standard/ — which _cleanup_legacy_project_dirs handles at the project level but not globally), those would persist. This is a minor gap since _cleanup_old_directories already nukes the entire ~/.claude/rules/ directory in the normal (non-local) flow, but in source_is_destination mode the cleanup is skipped and this method becomes the sole cleaner.
Consider extending to handle stale subdirectories as well:
Proposed fix
for item in global_rules_dir.iterdir():
- if item.is_file() and item.resolve() not in installed:
+ resolved = item.resolve()
+ if item.is_file() and resolved not in installed:
try:
item.unlink()
except (OSError, IOError):
pass
+ elif item.is_dir() and not any(
+ child.resolve() in installed for child in item.rglob("*")
+ ):
+ _clear_directory_safe(item)🤖 Prompt for AI Agents
In `@installer/steps/claude_files.py` around lines 546 - 563, _cleanup_stale_rules
currently only unlinks files and ignores stale subdirectories under
global_rules_dir; update the function to also detect item.is_dir() and, for
directories whose resolved path is not in the installed set, remove them (use
shutil.rmtree for non-empty dirs inside the same try/except block used for
item.unlink()) while preserving the existing installed-path resolution logic and
error swallowing behavior; reference the global_rules_dir, installed set, and
item variables when making the change.
| The context monitor tracks usage in real-time, and auto-compaction handles context limits transparently: | ||
|
|
||
| - As context grows, Pilot warns, then forces a handoff before hitting limits | ||
| - Session state is saved to `~/.pilot/sessions/` with continuation files — picks up seamlessly in the next session | ||
| - During `/spec`, Pilot won't start a new phase when context is high — it hands off instead | ||
| - At ~83% context, Claude's built-in compaction fires automatically — no process restart needed | ||
| - `pre_compact.py` captures Pilot state (active plan, tasks, key context) to persistent memory | ||
| - `post_compact_restore.py` re-injects Pilot context after compaction — agent continues seamlessly | ||
| - Multiple Pilot sessions can run in parallel on the same project without interference |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate context monitor and compaction threshold constants/usages.
echo "=== Looking for context-related files ==="
fd -a 'context_monitor\.py|pre_compact\.py|post_compact_restore\.py|config\.py' . 2>/dev/null
echo -e "\n=== Searching for threshold and percentage values ==="
rg -n '(75|80|83|context|threshold|percent)' --type py -i . 2>/dev/null | head -100Repository: maxritter/claude-pilot
Length of output: 9835
🏁 Script executed:
#!/bin/bash
# More targeted search for config and constant definitions
echo "=== Looking for all Python files in hooks directory ==="
find . -path '*/hooks/*.py' -type f 2>/dev/null
echo -e "\n=== Searching for percentage or threshold assignments ==="
rg -n '(:\s*0\.[0-9]+|%|threshold|context.*limit|compaction)' --type py . 2>/dev/null | head -80Repository: maxritter/claude-pilot
Length of output: 5153
🏁 Script executed:
#!/bin/bash
# Read the context_monitor.py file to find threshold constants
echo "=== Reading context_monitor.py for threshold constants ==="
cat -n ./pilot/hooks/context_monitor.py | head -250Repository: maxritter/claude-pilot
Length of output: 9020
🏁 Script executed:
#!/bin/bash
# Also read the test file to understand expected behavior
echo "=== Reading test_context_monitor.py ==="
cat -n ./pilot/tests/hooks/test_context_monitor.pyRepository: maxritter/claude-pilot
Length of output: 5419
🏁 Script executed:
#!/bin/bash
# Read README.md around the mentioned lines
echo "=== README.md lines 370-390 ==="
sed -n '370,390p' README.md
echo -e "\n=== Search for threshold mentions in README ==="
rg -n '(75|80|83|percent|context|threshold|compaction)' README.md -i | head -40Repository: maxritter/claude-pilot
Length of output: 7558
Fix documentation: auto-compaction threshold is 80%, not ~83%.
The README correctly documents 75% and 80% thresholds in line 354, but line 380 incorrectly claims auto-compaction fires at "~83% context." The actual constant in pilot/hooks/context_monitor.py is THRESHOLD_AUTOCOMPACT = 80 (line 21). Update README line 380 to reflect the correct 80% threshold to match the implementation.
🤖 Prompt for AI Agents
In `@README.md` around lines 378 - 383, The README incorrectly states
auto-compaction fires at "~83% context"; update that wording to "80% context" to
match the implementation constant THRESHOLD_AUTOCOMPACT in
pilot/hooks/context_monitor.py so documentation and code are consistent.
|
Claude finished @maxritter's task in 1m 54s —— View job Incremental Code Review — PR #56Reviewing changes from commits ✅ Issues ResolvedFrom Previous CodeRabbit Review:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
console/src/ui/viewer/hooks/useLicense.ts (1)
14-25:⚠️ Potential issue | 🟡 MinorMissing HTTP status check before parsing response JSON.
If the server returns a non-2xx status (e.g., 500),
res.json()may throw or parse an unexpected body, silently leavinglicenseas stale/null. Checkres.okbefore parsing.Proposed fix
const fetchLicense = useCallback((refresh = false) => { const url = refresh ? '/api/license?refresh=1' : '/api/license'; fetch(url) - .then((res) => res.json()) + .then((res) => { + if (!res.ok) throw new Error(`HTTP ${res.status}`); + return res.json(); + }) .then((data: LicenseResponse) => { setLicense(data); setIsLoading(false); }) .catch(() => { setIsLoading(false); }); }, []);
🤖 Fix all issues with AI agents
In `@console/src/ui/viewer/components/LicenseGate.tsx`:
- Around line 21-35: In LicenseGate's activation flow, don't call res.json()
unconditionally; first check res.ok on the Response returned by fetch in the
async handler inside the component (where res and data are used), and handle
non-2xx statuses separately: if !res.ok attempt to read res.text() (or safe JSON
parse) to extract a server message and call setError with a message including
the HTTP status and server text, otherwise parse JSON and proceed with the
existing success branch (setKey, setError(null), onActivated()) or failure
branch (setError(data.error ?? 'Activation failed')); ensure the catch block
still reports network/connection errors but not server-side 5xx as the same
message.
🧹 Nitpick comments (2)
console/src/ui/viewer/App.tsx (1)
85-101: License gating flow looks correct; consider a network-error state.Hook call order is maintained before early returns — good. However, if the license fetch fails (network error),
licenseremainsnulland the user seesLicenseGatewith no context that it's a connectivity issue rather than a missing license. TheuseLicensehook doesn't surface an error state, so the user may enter a valid key, have that also fail, and only then see "Connection failed."This is a minor UX gap — worth considering an explicit error/offline state in a follow-up.
docs/site/src/components/DeepDiveSection.tsx (1)
116-122: Theextfield mixes file extensions and path glob patterns.Lines 117-119 use actual file extensions (
*.py,*.ts,*.go), but line 121 (Backend) uses directory-level globs (**/models/**,**/routes/**). Similarly, line 120 (Frontend) mixes both styles. Since the UI renders this as a monospace label (line 313), users may find the inconsistency confusing.Consider using a more descriptive field name or normalizing the patterns.
| try { | ||
| const res = await fetch('/api/license/activate', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ key: trimmed }), | ||
| }); | ||
| const data = await res.json(); | ||
|
|
||
| if (data.success) { | ||
| setKey(''); | ||
| setError(null); | ||
| onActivated(); | ||
| } else { | ||
| setError(data.error ?? 'Activation failed'); | ||
| } |
There was a problem hiding this comment.
Non-2xx responses produce a misleading error message.
If the server returns a 500 or other non-2xx status with a non-JSON body, res.json() will throw and the catch block shows "Connection failed. Is the Pilot worker running?" — which is misleading for server errors. Check res.ok first.
Proposed fix
const res = await fetch('/api/license/activate', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ key: trimmed }),
});
+ if (!res.ok) {
+ setError(`Server error (${res.status}). Please try again.`);
+ return;
+ }
const data = await res.json();📝 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.
| try { | |
| const res = await fetch('/api/license/activate', { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ key: trimmed }), | |
| }); | |
| const data = await res.json(); | |
| if (data.success) { | |
| setKey(''); | |
| setError(null); | |
| onActivated(); | |
| } else { | |
| setError(data.error ?? 'Activation failed'); | |
| } | |
| try { | |
| const res = await fetch('/api/license/activate', { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ key: trimmed }), | |
| }); | |
| if (!res.ok) { | |
| setError(`Server error (${res.status}). Please try again.`); | |
| return; | |
| } | |
| const data = await res.json(); | |
| if (data.success) { | |
| setKey(''); | |
| setError(null); | |
| onActivated(); | |
| } else { | |
| setError(data.error ?? 'Activation failed'); | |
| } |
🤖 Prompt for AI Agents
In `@console/src/ui/viewer/components/LicenseGate.tsx` around lines 21 - 35, In
LicenseGate's activation flow, don't call res.json() unconditionally; first
check res.ok on the Response returned by fetch in the async handler inside the
component (where res and data are used), and handle non-2xx statuses separately:
if !res.ok attempt to read res.text() (or safe JSON parse) to extract a server
message and call setError with a message including the HTTP status and server
text, otherwise parse JSON and proceed with the existing success branch (setKey,
setError(null), onActivated()) or failure branch (setError(data.error ??
'Activation failed')); ensure the catch block still reports network/connection
errors but not server-side 5xx as the same message.
|
Claude finished @maxritter's task in 2m 49s —— View job Incremental Code Review — PR #56Reviewing latest push (installer build system, workflows, and script improvements) ✅ Issues ResolvedNone from the previous reviews were addressed in this push (different files changed). 🆕 New Issues Found1. Minor: sys.path manipulation may affect user environment —
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@install.sh`:
- Around line 267-279: download_installer (and the wrapper download block and
download_pilot_binary) silently do nothing if neither curl nor wget exists; add
an else branch after the curl/wget checks to print an error and exit. Update the
blocks in download_installer, the wrapper download section, and
download_pilot_binary to mirror download_file's behavior: detect absence of both
tools, echo a clear failure message (e.g., "[!!] curl or wget required to
download installer/wrapper/pilot") and exit 1 so the script fails fast when no
downloader is available.
🧹 Nitpick comments (6)
install.sh (2)
65-71:http_codeis assigned but never read — dead code.The
http_codevariable (declared on Line 65, assigned on Line 70) is never used. This was likely intended for rate-limit or error-code checking that wasn't implemented. Either remove it or add the intended check.🧹 Proposed fix
get_latest_release() { local api_url="https://api.github.com/repos/${REPO}/releases/latest" local version="" - local http_code="" if command -v curl >/dev/null 2>&1; then local response="" - response=$(curl -sL -w "\n%{http_code}" "$api_url" 2>/dev/null) || true - http_code=$(echo "$response" | tail -1) - version=$(echo "$response" | sed '$d' | grep -m1 '"tag_name"' | sed 's/.*"v\([^"]*\)".*/\1/') || true + response=$(curl -sL "$api_url" 2>/dev/null) || true + version=$(echo "$response" | grep -m1 '"tag_name"' | sed 's/.*"v\([^"]*\)".*/\1/') || true
459-460: Declare and assign separately to avoid masking return values (SC2155).If
$(pwd)were to fail,exportwould mask its non-zero exit code. Low risk in practice, but easy to fix.🧹 Proposed fix
if [ "$USE_LOCAL_INSTALLER" = true ]; then - export PYTHONPATH="$(pwd):${PYTHONPATH:-}" + local _pwd + _pwd="$(pwd)" + export PYTHONPATH="${_pwd}:${PYTHONPATH:-}"installer/build.py (4)
295-300: Broadexcept Exception: passsilently swallows errors.If codesign fails for a reason other than "not found" (e.g., signing identity issue, permission error), the failure is silently ignored. Consider logging at debug level.
🧹 Proposed fix
try: subprocess.run(["xattr", "-c", str(dst_so)], capture_output=True) subprocess.run(["codesign", "-s", "-", str(dst_so)], capture_output=True) - except Exception: - pass + except (FileNotFoundError, OSError): + pass
370-370: F-string without any placeholders.Static analysis flags this as F541. Remove the
fprefix.🧹 Proposed fix
- print(f"\n✓ Release artifacts:") + print("\n✓ Release artifacts:")
73-94:set_build_timestamp/reset_build_timestampmutate source files in-place — beware of concurrent builds.These functions rewrite
__init__.pyduring the build. Thefinallyblock inmain()ensures reset, which is good. However, if two local builds run simultaneously, they'll race on the same file. Not a problem in CI (isolated checkouts), but worth a comment for local dev safety.
162-167: Docstring stripping regex may not match multi-line docstrings.Line 167's
re.sub(r'^"""[^"]*"""\s*', "", code, count=1)uses[^"]*which won't cross line boundaries (nore.DOTALL) and won't match docstrings containing quote characters. If any module has a multi-line file-level docstring, it will remain in the merged output. This is benign (extra docstring in compiled code) but may not match intent.🧹 Fix to handle multi-line docstrings
- code = re.sub(r'^"""[^"]*"""\s*', "", code, count=1) + code = re.sub(r'^""".*?"""\s*', "", code, count=1, flags=re.DOTALL)
| if command -v curl >/dev/null 2>&1; then | ||
| tree_json=$(curl -fsSL "$api_url" 2>/dev/null) || true | ||
| if ! curl -fsSL "$so_url" -o "$so_path" 2>/dev/null; then | ||
| echo " [!!] Failed to download installer binary" | ||
| echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash" | ||
| exit 1 | ||
| fi | ||
| elif command -v wget >/dev/null 2>&1; then | ||
| tree_json=$(wget -qO- "$api_url" 2>/dev/null) || true | ||
| if ! wget -q "$so_url" -O "$so_path" 2>/dev/null; then | ||
| echo " [!!] Failed to download installer binary" | ||
| echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash" | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Silent fall-through if neither curl nor wget is available.
In download_installer, if neither curl nor wget is found, execution silently continues past the download block. The binary won't be downloaded, but the script won't fail until later (or worse, succeeds with an empty/missing file). Compare with download_file (Line 129-132) which explicitly checks and exits.
🛡️ Proposed fix — add an else clause
if command -v curl >/dev/null 2>&1; then
if ! curl -fsSL "$so_url" -o "$so_path" 2>/dev/null; then
echo " [!!] Failed to download installer binary"
echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash"
exit 1
fi
elif command -v wget >/dev/null 2>&1; then
if ! wget -q "$so_url" -O "$so_path" 2>/dev/null; then
echo " [!!] Failed to download installer binary"
echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash"
exit 1
fi
+ else
+ echo " [!!] Error: Neither curl nor wget found"
+ exit 1
fiThe same issue exists in the wrapper download block (Lines 286-298) and in download_pilot_binary (Lines 382-392, 399-411).
📝 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.
| if command -v curl >/dev/null 2>&1; then | |
| tree_json=$(curl -fsSL "$api_url" 2>/dev/null) || true | |
| if ! curl -fsSL "$so_url" -o "$so_path" 2>/dev/null; then | |
| echo " [!!] Failed to download installer binary" | |
| echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash" | |
| exit 1 | |
| fi | |
| elif command -v wget >/dev/null 2>&1; then | |
| tree_json=$(wget -qO- "$api_url" 2>/dev/null) || true | |
| if ! wget -q "$so_url" -O "$so_path" 2>/dev/null; then | |
| echo " [!!] Failed to download installer binary" | |
| echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash" | |
| exit 1 | |
| fi | |
| fi | |
| if command -v curl >/dev/null 2>&1; then | |
| if ! curl -fsSL "$so_url" -o "$so_path" 2>/dev/null; then | |
| echo " [!!] Failed to download installer binary" | |
| echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash" | |
| exit 1 | |
| fi | |
| elif command -v wget >/dev/null 2>&1; then | |
| if ! wget -q "$so_url" -O "$so_path" 2>/dev/null; then | |
| echo " [!!] Failed to download installer binary" | |
| echo " [!!] Please try again or specify a version: VERSION=X.Y.Z curl ... | bash" | |
| exit 1 | |
| fi | |
| else | |
| echo " [!!] Error: Neither curl nor wget found" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In `@install.sh` around lines 267 - 279, download_installer (and the wrapper
download block and download_pilot_binary) silently do nothing if neither curl
nor wget exists; add an else branch after the curl/wget checks to print an error
and exit. Update the blocks in download_installer, the wrapper download section,
and download_pilot_binary to mirror download_file's behavior: detect absence of
both tools, echo a clear failure message (e.g., "[!!] curl or wget required to
download installer/wrapper/pilot") and exit 1 so the script fails fast when no
downloader is available.
sync_worktree now handles diverged base branches automatically: - Auto-stashes uncommitted changes on base branch before sync - Rebases worktree branch onto base branch before squash merge - Aborts rebase cleanly on conflict - Restores stash in finally block even on failure
|
Claude finished @maxritter's task in 1m 24s —— View job Incremental Code Review — PR #56Reviewing latest push: commits 47e6dd9 and 08ed28b (30+ files changed since last review) ✅ Issues ResolvedFrom Previous CodeRabbit Reviews:
🔍 New Issues FoundNone — The latest commits (47e6dd9, 08ed28b) are clean-up and bug-fix commits that:
No new problems introduced. 📋 Still Open from Previous ReviewsThe following issues from earlier CodeRabbit reviews remain unaddressed (not touched by these commits):
These are all minor/suggestions and don't block merging. ✨ SummaryThe latest commits (47e6dd9 + 08ed28b) successfully resolved 6 documentation and code quality issues from previous reviews:
Changes are accurate, internally consistent, and well-targeted. No new issues introduced. The 9 remaining open items from previous reviews are implementation-level concerns in files not touched by these commits and can be addressed in follow-up work if needed. Recommendation: These incremental changes are high quality and ready to merge. The remaining open items are minor and don't block this PR. |
…tivation error masking - Replace "Intelligent Context" with "Persistent Memory" as headline feature - Reorder features: Spec-Driven first, Persistent Memory last - Merge Built-in Rules and Coding Standards into single section - Compress hooks pipeline into 3-column grid on website - Remove QualifierSection from website - Rename Context7 to lib-docs in MCP servers display - Fix license activation: PILOT- keys no longer fall through to Gumroad on network errors, preventing misleading "Invalid license key" message - Add retry with 2s delay for Polar network errors - Fix installer reading JSON error from stdout instead of empty stderr
Incremental Code Review — PR #56
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/site/src/components/DeepDiveSection.tsx`:
- Line 107: The Tools rules array in DeepDiveSection (the entry "Context7"
inside the rules: [...] list) is inconsistent with the MCP servers name
"lib-docs" used elsewhere; update the rules array to replace "Context7" with
"lib-docs" so the Tools rules string reads e.g. "lib-docs + grep-mcp + web
search + GitHub CLI" to match the MCP servers naming (locate the rules array in
the DeepDiveSection component and update that specific string).
| category: "Tool Integration", | ||
| rules: ["Vexor semantic search", "Context7 library docs", "grep-mcp GitHub search", "Web search + fetch", "Playwright CLI (E2E)", "MCP CLI"], | ||
| category: "Tools", | ||
| rules: ["Context7 + grep-mcp + web search + GitHub CLI", "Pilot CLI + MCP-CLI + Vexor search", "Playwright CLI (E2E browser testing)"], |
There was a problem hiding this comment.
Inconsistent naming: "Context7" in rules vs. "lib-docs" in MCP servers.
The Tools rules category on Line 107 references "Context7" but the MCP servers list on Line 127 was renamed to "lib-docs". These should be consistent.
Proposed fix
- rules: ["Context7 + grep-mcp + web search + GitHub CLI", "Pilot CLI + MCP-CLI + Vexor search", "Playwright CLI (E2E browser testing)"],
+ rules: ["lib-docs + grep-mcp + web search + GitHub CLI", "Pilot CLI + MCP-CLI + Vexor search", "Playwright CLI (E2E browser testing)"],📝 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.
| rules: ["Context7 + grep-mcp + web search + GitHub CLI", "Pilot CLI + MCP-CLI + Vexor search", "Playwright CLI (E2E browser testing)"], | |
| rules: ["lib-docs + grep-mcp + web search + GitHub CLI", "Pilot CLI + MCP-CLI + Vexor search", "Playwright CLI (E2E browser testing)"], |
🤖 Prompt for AI Agents
In `@docs/site/src/components/DeepDiveSection.tsx` at line 107, The Tools rules
array in DeepDiveSection (the entry "Context7" inside the rules: [...] list) is
inconsistent with the MCP servers name "lib-docs" used elsewhere; update the
rules array to replace "Context7" with "lib-docs" so the Tools rules string
reads e.g. "lib-docs + grep-mcp + web search + GitHub CLI" to match the MCP
servers naming (locate the rules array in the DeepDiveSection component and
update that specific string).
…ugfixes (#56) Replace Endless Mode's kill-and-restart cycle with Claude Code's built-in compaction mechanism. PreCompact and SessionStart(compact) hooks capture and restore Pilot-specific state seamlessly. Key changes: - Compaction hooks for context preservation (PreCompact + SessionStart) - Rules consolidated from 20+ files into 10 focused modules - Installer no longer overwrites user settings — uses project-level config - Rebrand "Intelligent Context" to "Persistent Memory", reorder features - Website: compress hooks grid, merge rules/standards sections, remove QualifierSection, refresh copy and correct attribution - Worktree sync: auto-rebase onto base branch before squash merge - License activation: PILOT- keys no longer mask network errors as "Invalid license key" via Gumroad fallback; add retry on failure - Fix installer reading JSON error from stdout instead of stderr - /spec commands streamlined with clearer phase dispatch
# [6.6.0](v6.5.9...v6.6.0) (2026-02-16) ### Features * Compaction-based context preservation, branding overhaul, and bugfixes ([#56](#56)) ([5aa1bc1](5aa1bc1))
1 participant
Summary
Comprehensive release driven by user feedback (Issue #54) and architectural improvements:
PreCompactandSessionStart(compact)hooks preserve and restore session state seamlessly — no more process termination, no context losscli-tools,development-practices,research-tools,testing,verification, etc.) — reducing context cost and improving clarity/speccommand dispatchPlans
2026-02-16-compaction-based-endless-mode.md— VERIFIED2026-02-16-issue-54-friction-fixes.md— VERIFIEDTest plan
uv run pytest -q,cd console && bun test)pilot/hooks/tests/test_pre_compact.py,pilot/hooks/tests/test_post_compact_restore.py)/specworkflow completes end-to-end (plan → implement → verify)Summary by CodeRabbit
New Features
Documentation
Chores