Conversation
- Issue parcadei#169: Replace COUNT(*) FILTER(WHERE) with SUM(CASE WHEN...) in BTQL query - Issue parcadei#168: Use spawn context for ProcessPoolExecutor on macOS - Issue parcadei#152: Fix relative path resolution using input.cwd in tldr-read-enforcer - Issue parcadei#156/157: Port ensureMemoryDaemon() from Python to TypeScript session-start hook
/parcadei#155) - Add _copy_dotfile_scripts() to copy root-level .py/.sh files from .claude/scripts/ - Called from both install_opc_integration() and install_opc_integration_symlink() - Add missing 'Installed N scripts' print line in wizard for both install paths - Files now deployed: status.py, recall_learnings.py, tldr_stats.py
- Add fish shell detection (use 'set -gx' syntax, ~/.config/fish/config.fish) - Create ~/.config/fish/ directory if it doesn't exist - Remove CLAUDE_OPC_DIR from shell config on uninstall (issue parcadei#147)
- Remove 4 archived skills from active list (async-repl-protocol, router-first-architecture, search-hierarchy, search-tools) - Rename 7 underscore directories to kebab-case (continuity_ledger, create_handoff, describe_pr, implement_task, implement_plan, resume_handoff, system_overview) - Fix name: field in implement-task and implement-plan SKILL.md - Remove outdated model pin claude-opus-4-5-20251101 from discovery-interview and research skills
…dei#144) - Add Step 11/15: qlty code quality tool (70+ linters, 40+ languages) - Add Step 12/15: ast-grep AST-based code search and refactoring - Update TLDR to Step 13/15, Diagnostics to Step 14/15, Loogle to Step 15/15 - Total steps increased from 13 to 15
…parcadei#146) - Add Firecrawl API key prompt (web scraping) → FIRECRAWL_API_KEY env var - Add Morph API key prompt (data extraction) → MORPH_API_KEY env var - Unlocks: firecrawl-scrape, research-agent, research-external skills
- New hook blocks scripts/(mcp|core)/ commands outside OPC directory - Prevents ModuleNotFoundError when uv run misses pyproject.toml - Returns corrected command with cd prefix suggestion - Fixes parcadei#148
- Add CLAUDE_CC_DIR alongside CLAUDE_OPC_DIR in wizard.py setup - Update 8 agents to use CLAUDE_CC_DIR for project paths - Update 8 skills to use CLAUDE_CC_DIR for project paths - References paradei PR parcadei#138
- Removes timeout from npm install, npm build - Removes timeout from wait_for_services() - Removes timeout from uv sync, uv tool install, cargo install - Removes timeout from lake build - Removes timeout from model download subprocess calls - Removes related comments
- uv tool install qlty: 120s -> 300s (5 min for first-time install) - curl install script: 30s -> 60s - curl | sh qlty install: 60s -> 120s - cargo install ast-grep: 300s -> 600s (10 min for build from source) - git clone loogle: 120s -> 600s (2GB repo download) These prevent premature timeout failures while still protecting against infinite hangs.
When user selects 'embedded' or 'sqlite' database mode, skip: - Step 6: Docker stack (no need for Docker containers) - Step 7: Database migrations (handled automatically by embedded or not needed for sqlite)
After setting up embedded PostgreSQL: 1. Start the pgserver 2. Run docker/init-schema.sql via run_migrations_direct() 3. Show warnings for optional extensions that failed
start_embedded_postgres() now accepts venv_path parameter and uses venv's Python executable to run pgserver as subprocess when pgserver is not available in the current Python environment.
Instead of running pgserver as subprocess, now adds venv's site-packages to sys.path before importing pgserver module.
Since global Python (3.14) cannot import modules compiled for venv Python (3.12), use subprocess to run pgserver startup script with the correct venv Python interpreter.
The docker/ directory is at repo root, not opc/ root.
Embedded PostgreSQL setup is now simpler - just creates venv. User handles server start and schema migration manually.
qlty is a standalone binary installed to ~/.qlty/bin/qlty, not a uv tool.
- ast-grep: check with shutil.which before install - tldr: check if llm-tldr is installed (not tldr-pages)
- qlty: just check shutil.which('qlty')
- tldr: check shutil.which and verify it's llm-tldr
- ast-grep: just check shutil.which('ast-grep')
Model download via subprocess doesn't work because tldr module is installed in uv tool environment, not system Python. User should run 'tldr semantic index .' manually after setup.
Instead of sys.executable + module import, use 'uv run tldr semantic download-model' which works regardless of where tldr is installed.
There is no 'download-model' command - model downloads automatically when running 'tldr semantic index'. Using correct command now.
Using sys.executable fails because tldr module is in uv tool environment, not system Python. Using 'uv run python' instead.
Add detailed docstring explaining: - Output format with examples - Color coding scheme - All components (tokens, context%, branch, S/U/A counts, goal, now) - Temporary file locations
CONTEXT: {tokens} {context}% | GIT: {branch} S:{staged} U:{unstaged} A:{added} | GOAL: {goal} → NOW: {now}
- review-agent.md: replace implement-agent reference with kraken (implement-agent does not exist) - compiler-in-the-loop.sh: replace $CLAUDE_PROJECT_DIR with $HOME - compiler-in-the-loop-stop.sh: replace $CLAUDE_PROJECT_DIR with $HOME
- implement-plan: fix implement_task → implement-task (underscore vs hyphen) - skill-developer: fix multi-tool-pipeline/SKILL.md reference + CLAUDE_PROJECT_DIR - compound-learnings: replace $CLAUDE_PROJECT_DIR → $CLAUDE_CC_DIR - debug-hooks: replace $CLAUDE_PROJECT_DIR → $CLAUDE_CC_DIR - braintrust-tracing: replace $CLAUDE_PROJECT_DIR → $CLAUDE_CC_DIR - hook-developer: replace $CLAUDE_PROJECT_DIR → $CLAUDE_CC_DIR - hooks: replace $CLAUDE_PROJECT_DIR → $CLAUDE_CC_DIR - wiring: replace $CLAUDE_PROJECT_DIR → $CLAUDE_CC_DIR
Added 'When to Use' section to: - build, debug, fix - user-facing workflow skills - implement-plan, implement-task, plan-agent - orchestration skills - premortem - risk analysis skill - create-handoff, resume-handoff, describe-pr - handoff/description skills - explore, research - exploration skills Also fixed skill-developer broken file references (lines 142-144)
Added to: commit, help, validate-agent, recall
…, observe-before-editing, dead-code
…nt-pings, braintrust-tracing, hooks
Added 'When to Use' sections to 24 skills that were missing them: - agentica-prompts, explicit-identity, graceful-degradation - idempotent-redundancy, implement_plan_micro, index-at-creation - math-help, math-router, mcp-scripts, modular-code - nia-docs, no-polling-agents, no-task-output - opc-architecture, parallel-agent-contracts, parallel-agents - prove, qlty-during-development, repo-research-analyst - research-agent, research-external, tldr-overview - tldr-router, tour
|
PR author is not in the allowed authors list. |
📝 WalkthroughWalkthroughComprehensive refactoring migrating environment variable references from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (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)
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 |
| # Build script to start pgserver and get URI | ||
| start_script = f""" | ||
| import sys | ||
| sys.path.insert(0, '{venv_path}/lib/python3.12/site-packages') |
There was a problem hiding this comment.
Bug: The function start_embedded_postgres injects an invalid path into a subprocess's sys.path when venv_path is None and hardcodes the Python version.
Severity: MEDIUM
Suggested Fix
Conditionally add the site-packages path to the subprocess environment only if venv_path is not None. Replace the hardcoded Python version string with a dynamic version lookup, for example by using sys.version_info to construct the path like f"python{sys.version_info.major}.{sys.version_info.minor}".
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: opc/scripts/setup/embedded_postgres.py#L50
Potential issue: When the `venv_path` argument passed to `start_embedded_postgres` is
`None`, the function constructs an invalid path string,
'None/lib/python3.12/site-packages', and injects it into the `sys.path` of the
subprocess it creates. This will likely cause `ImportError` exceptions within the
subprocess. Additionally, the path construction hardcodes the Python version
`python3.12`, which will cause the function to fail when executed with a different
version of Python.
Did we get this right? 👍 / 👎 to inform future reviews.
| return { | ||
| "success": True, | ||
| "uri": uri, | ||
| } |
There was a problem hiding this comment.
Bug: The start_embedded_postgres function no longer returns the documented server key in its result dictionary, which will cause a KeyError for callers.
Severity: MEDIUM
Suggested Fix
Update the start_embedded_postgres function to return a dictionary that includes the server key, matching its documented API. Alternatively, update the function's docstring and all call sites to reflect the new return structure and handle the absence of the server key.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: opc/scripts/setup/embedded_postgres.py#L68-L71
Potential issue: The `start_embedded_postgres` function was refactored to run in a
subprocess, but its return value was changed. It no longer includes the `server` key in
the returned dictionary, which is still documented as part of its return API. Any
calling code that relies on accessing `result['server']`, for instance to perform
cleanup operations, will now encounter a `KeyError` and fail.
Did we get this right? 👍 / 👎 to inform future reviews.
| ctx = multiprocessing.get_context("spawn") | ||
| try: | ||
| with ProcessPoolExecutor(max_workers=1) as executor: | ||
| future = executor.submit(_wrapper) | ||
| with ProcessPoolExecutor(max_workers=1, mp_context=ctx) as executor: | ||
| future = executor.submit(func, *args, **kwargs) |
There was a problem hiding this comment.
Bug: Using a 'spawn' multiprocessing context in safe_compute will cause a PicklingError for decorated functions like those using @math_command.
Severity: HIGH
Suggested Fix
Revert the multiprocessing context in safe_compute from 'spawn' back to 'fork' if the target environment supports it. If 'spawn' is required, refactor the decorated functions or the decorators themselves (e.g., @math_command) to ensure the target functions are picklable, for instance by defining them at the top level of a module.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: opc/scripts/cc_math/math_base.py#L771-L774
Potential issue: The `safe_compute` function was modified to use a 'spawn'
multiprocessing context. This change will cause a `PicklingError` when `safe_compute` is
used to decorate a function that is not defined at the top level of a module. This
affects decorators like `@math_command`, which create non-serializable wrapper
functions. Any attempt to call a function decorated with `@math_command` will now fail.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/settings.json (1)
228-242:⚠️ Potential issue | 🔴 CriticalMove
mcp-directory-guard.mjstoPreToolUsesection.As noted in the TypeScript file review, this hook is designed to prevent commands from running incorrectly (uses
permissionDecision: 'deny'). Placing it underPostToolUsemeans it runs after the command has already executed, which is too late.🔧 Proposed fix: Register under PreToolUse instead
Remove from
PostToolUse:{ "matcher": "Bash", "hooks": [ - { - "type": "command", - "command": "node $HOME/.claude/hooks/dist/mcp-directory-guard.mjs", - "timeout": 5 - }, { "type": "command", "command": "node $HOME/.claude/hooks/dist/import-error-detector.mjs", "timeout": 5 } ] }Add to
PreToolUsesection (e.g., after line 80):{ "matcher": "Bash", "hooks": [ { "type": "command", "command": "node $HOME/.claude/hooks/dist/mcp-directory-guard.mjs", "timeout": 5 } ] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings.json around lines 228 - 242, The mcp-directory-guard hook is currently registered in PostToolUse but must run before commands to block bad executions; move the Bash hook entry that runs "node $HOME/.claude/hooks/dist/mcp-directory-guard.mjs" (the command hook with timeout 5) out of the PostToolUse block and add the same matcher+hook object under the PreToolUse section so the guard executes prior to tool/command invocation.opc/scripts/setup/wizard.py (1)
481-497:⚠️ Potential issue | 🟠 MajorReuse the shared DB URL builder here.
This interpolates
passworddirectly intoCONTINUOUS_CLAUDE_DB_URL. Passwords containing@,:,/,#, or spaces produce an invalid URI, and the empty-password case now diverges fromgenerate_database_url().🛠️ Suggested fix
if mode == "docker": + from scripts.setup.embedded_postgres import generate_database_url + host = db.get("host", "localhost") port = db.get("port", 5432) database = db.get("database", "continuous_claude") user = db.get("user", "claude") password = db.get("password", "") @@ lines.append("") lines.append("# Connection string for scripts (canonical name)") - lines.append( - f"CONTINUOUS_CLAUDE_DB_URL=postgresql://{user}:{password}@{host}:{port}/{database}" - ) + lines.append(f"CONTINUOUS_CLAUDE_DB_URL={generate_database_url(db)}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@opc/scripts/setup/wizard.py` around lines 481 - 497, The Docker branch manually interpolates password into CONTINUOUS_CLAUDE_DB_URL which breaks on special chars and deviates from generate_database_url(); replace the manual url construction in the mode == "docker" block with a call to the shared generate_database_url(db) (or the canonical DB URL builder used elsewhere) so the password is properly encoded and empty-password behavior matches the rest of the code, while keeping the existing POSTGRES_* env lines and the CONTINUOUS_CLAUDE_DB_URL variable name.
🧹 Nitpick comments (9)
.claude/skills/parallel-agents/SKILL.md (1)
11-13: Good addition to clarify usage context.The new "When to Use" section improves documentation clarity by explaining when this skill is invoked. However, the phrase "Not user-invocable directly" at line 13 is redundant with the
user-invocable: falsefrontmatter at line 4.♻️ Optional: Remove redundancy with frontmatter
-This skill is invoked when launching multiple agents in parallel. Not user-invocable directly. +This skill is invoked when launching multiple agents in parallel.The
user-invocable: falsefrontmatter already conveys this constraint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/parallel-agents/SKILL.md around lines 11 - 13, The "When to Use" section contains a redundant sentence "Not user-invocable directly" that repeats the frontmatter setting 'user-invocable: false'; remove that sentence from the "When to Use" paragraph (or reword the paragraph to omit that constraint) so the documentation no longer duplicates the frontmatter while keeping the explanatory line "This skill is invoked when launching multiple agents in parallel.".claude/skills/tldr-router/SKILL.md (1)
10-12: Nice addition! The "When to Use" section improves discoverability.The new section provides helpful context about skill invocation and aligns well with the PR's documentation improvements.
For slightly better readability, consider expanding the second sentence:
📝 Optional enhancement
-This skill is invoked when routing questions to the optimal TLDR command. User-invocable via /tldr-router. +This skill is invoked when routing questions to the optimal TLDR command. You can also manually invoke it using the `/tldr-router` command.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/tldr-router/SKILL.md around lines 10 - 12, The "When to Use" section currently has a short second sentence; improve readability by expanding that sentence in .claude/skills/tldr-router/SKILL.md under the "When to Use" header to give a bit more context about when and why users would invoke the skill (for example, mention typical question types, expected output, or when to prefer /tldr-router over other commands); update the sentence that references "/tldr-router" to a clearer, slightly longer phrasing so readers immediately understand invocation and purpose..claude/skills/index-at-creation/SKILL.md (1)
11-13: Documentation improvement adds helpful clarity.The new "When to Use" section clearly explains the skill's invocation context and aligns with the metadata.
Minor grammatical refinement: Consider converting the sentence fragment on line 13 to a complete sentence for slightly better readability.
✍️ Optional grammatical refinement
-This skill is invoked when creating artifacts. Not user-invocable directly. +This skill is invoked when creating artifacts. It is not directly user-invocable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/index-at-creation/SKILL.md around lines 11 - 13, The "When to Use" section contains a sentence fragment under the heading "When to Use" ("Not user-invocable directly."); update that fragment to a complete sentence such as "It is not user-invocable directly." so the section reads clearly and aligns with the metadata while keeping the same meaning..claude/skills/premortem/SKILL.md (1)
11-19: Nice documentation improvement!The "When to Use" section clearly documents activation triggers for the skill. This aligns well with the broader pattern of improving skill discoverability.
One minor observation: lines 16 and 18 describe essentially the same timing window (after planning, before implementation). You could optionally consolidate them for brevity.
📝 Optional consolidation for brevity
Activate when: - User says "/premortem" - User says "what could go wrong?" or "identify risks" -- Before starting implementation of a new feature or plan - User says "run premortem on this plan" -- After a plan is created and before implementation starts + - After a plan is created, before starting implementation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/premortem/SKILL.md around lines 11 - 19, Consolidate duplicate timing bullets in the "When to Use" section by merging the two similar items "Before starting implementation of a new feature or plan" and "After a plan is created and before implementation starts" into a single concise bullet (e.g., "After planning and before implementation starts"); update the list under the "When to Use" header in .claude/skills/premortem/SKILL.md to replace those two lines with the single consolidated line to improve brevity and avoid redundancy..claude/skills/repo-research-analyst/SKILL.md (1)
12-14: Consider making the "When to Use" section more specific and actionable.The current description is somewhat circular with the skill's existing description (line 3). Consider providing concrete scenarios or decision criteria, such as:
- When you encounter a new codebase and need to understand its structure
- Before making contributions to unfamiliar repositories
- When you need to document conventions for a team
Also consider separating the invocation method into its own line for clarity:
## When to Use Use this skill when you need to quickly understand a new codebase's architecture, conventions, and contribution guidelines—especially before planning features or fixes in unfamiliar repositories. **Invocation:** User-invocable via `/repo-research-analyst`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/repo-research-analyst/SKILL.md around lines 12 - 14, The "When to Use" section is too generic and mixes invocation details; update the "When to Use" block to list concrete scenarios (e.g., onboarding to a new codebase, preparing to contribute to unfamiliar repositories, documenting team conventions) and move the invocation detail to its own line or bolded "Invocation: /repo-research-analyst" so the section is actionable and clearer; edit the header text "When to Use" and the paragraph below it in SKILL.md to reflect these changes..claude/skills/tour/SKILL.md (1)
10-12: Documentation enhancement looks good.The new "When to Use" section provides helpful context about when the tour skill is invoked. The information is clear and accurate.
For a minor readability improvement, consider rephrasing the second sentence:
📝 Optional phrasing refinement
-This skill is invoked when the user asks about capabilities or wants an overview. User-invocable via /tour. +This skill is invoked when the user asks about capabilities or wants an overview. It can also be triggered directly using the `/tour` command.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/tour/SKILL.md around lines 10 - 12, The second sentence in the "When to Use" section ("User-invocable via /tour.") is terse—reword it in SKILL.md to improve readability by replacing that sentence with a clearer phrase (e.g., "Invokable by users via the /tour command.") so the "When to Use" block and the sentence about invocation are more natural; update the text under the "When to Use" heading accordingly..claude/skills/completion-check/SKILL.md (1)
11-13: Good addition to clarify skill invocation context.The new "When to Use" section helpfully specifies that this skill is invoked after infrastructure building (not directly by users), which aligns with the
user-invocable: falsesetting in the frontmatter.Optionally, consider adding a sentence about what/who actually invokes this skill (e.g., "automatically triggered by the system after infrastructure tasks complete" or "called by the task orchestrator") to make the invocation mechanism clearer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/completion-check/SKILL.md around lines 11 - 13, Update the "When to Use" section to explicitly state who or what triggers the skill by adding a short sentence (e.g., "automatically triggered by the system after infrastructure tasks complete" or "called by the task orchestrator") so it complements the frontmatter setting user-invocable: false; edit the "When to Use" paragraph in SKILL.md to append one clear invocation sentence that references the automated system/orchestrator..claude/hooks/src/mcp-directory-guard.ts (1)
33-44: Static analysis: ReDoS warning with low practical risk.The static analysis tool flags potential ReDoS due to building a RegExp from
escapedDir. The risk is mitigated because:
opcDiris a filesystem path fromgetOpcDir(), not user input- Special regex chars are properly escaped (line 34)
- The pattern lacks nested quantifiers that cause exponential backtracking
For added safety, consider validating
opcDiris a reasonable path before use, or add a length check.🛡️ Optional defensive validation
function buildCdPrefixPattern(opcDir: string | null): RegExp { + // Limit path length to prevent pathological regex + const safeDir = opcDir && opcDir.length < 500 ? opcDir : null; - const escapedDir = opcDir ? opcDir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') : ''; + const escapedDir = safeDir ? safeDir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') : '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/src/mcp-directory-guard.ts around lines 33 - 44, The RegExp built in buildCdPrefixPattern(opcDir) can be flagged for ReDoS because it interpolates escapedDir; to fix, validate opcDir before using it: in buildCdPrefixPattern (and the caller getOpcDir flow) enforce a reasonable length limit (e.g., max path length) and ensure the value only contains safe filesystem/path characters (e.g., letters, digits, /, -, _, ., ~) and if validation fails, omit using escapedDir (fall back to only the $CLAUDE_OPC_DIR variants) or throw/return a safe default; this keeps the existing escaping but prevents long or unexpected strings from being embedded in the regex..claude/hooks/src/session-start-continuity.ts (1)
316-351: Consider handling async spawn errors or adding a comment explaining the fire-and-forget pattern.The daemon startup uses a fire-and-forget pattern with
detached: true,stdio: 'ignore', andunref(). While synchronous spawn errors (e.g.,uvnot in PATH) are caught, asynchronous errors emitted via the'error'event on the child process are not handled. This is acceptable for best-effort daemon startup, but could benefit from:
- A brief comment explaining this is intentional fire-and-forget behavior
- Optionally, attaching a no-op error handler to prevent potential unhandled rejection warnings in some Node.js versions
Additionally, the return value (
string | null) is unused at the call site (line 357). Consider either using it for logging or simplifying the return type tovoid.💡 Optional improvement for clarity
try { const child = spawn('uv', ['run', 'python', daemonScript, 'start'], { detached: true, stdio: 'ignore', cwd: path.dirname(path.dirname(path.dirname(daemonScript))), }); + // Fire-and-forget: ignore async errors, daemon manages its own lifecycle + child.on('error', () => {}); child.unref(); return 'Memory daemon: Started'; } catch (e) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/src/session-start-continuity.ts around lines 316 - 351, ensureMemoryDaemon currently fire-and-forgets starting the daemon via spawn('uv'...) with detached/unref and only catches synchronous throws; add a brief comment near ensureMemoryDaemon explaining this is intentional best-effort fire-and-forget startup, and attach a no-op error handler to the spawned child (listen for the child's 'error' event) to avoid unhandled-error warnings; also either use the returned string at the call site or simplify ensureMemoryDaemon's signature to return void (update callers accordingly) so the return type (string | null) reflects actual use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/phoenix.md:
- Around line 89-92: Add a fence language to the markdown code block that shows
the output path to satisfy markdownlint MD040: change the triple-backtick fence
around the snippet
"$CLAUDE_CC_DIR/.claude/cache/agents/phoenix/output-{timestamp}.md" to include a
language tag (use "text" or "bash"), i.e. replace ``` with ```text so the code
block becomes fenced with a language; update the block in the
.claude/agents/phoenix.md file where the "Also write summary to:" snippet
appears.
In @.claude/agents/plan-agent.md:
- Around line 77-79: The markdown fence shown as "```" containing
"$CLAUDE_CC_DIR/thoughts/shared/plans/[descriptive-name].md" should include a
language token to satisfy MD040; update the fenced block in
.claude/agents/plan-agent.md (the code fence at the plans example) to use a
language such as "text" (e.g., change the opening "```" to "```text") so the
block is properly annotated.
In @.claude/agents/session-analyst.md:
- Line 16: The shell command uses an unquoted variable expansion (cat
$CLAUDE_CC_DIR/.claude/skills/braintrust-analyze/SKILL.md) which will break on
paths with spaces; update the occurrences on line 16 and lines 24-25 to quote
the variable expansion (e.g., use "$CLAUDE_CC_DIR" or quote the entire path) so
the CLAUDE_CC_DIR expansion is protected; search for other uses of CLAUDE_CC_DIR
in this file (and specifically the SKILL.md cat invocation) and apply the same
quoting consistently.
In @.claude/hooks/compiler-in-the-loop-stop.sh:
- Line 15: Guard the directory before the cd to avoid hard failures: replace the
bare cd "$HOME/.claude/hooks" invocation with an existence check using [ -d
"$HOME/.claude/hooks" ] (or test -d) and if it does not exist perform the same
early-exit behavior used elsewhere in the script (return {} or exit 0) so the
hook returns gracefully instead of failing under set -e; update the block
containing the cd "$HOME/.claude/hooks" line to first check the directory and
only cd when present, otherwise return {}.
In @.claude/hooks/src/mcp-directory-guard.ts:
- Around line 1-18: The hook mcp-directory-guard is implemented as a PreToolUse
hook (see imports PreToolUseInput, PreToolUseHookOutput and its
deny/permissionDecision behavior) but is registered under PostToolUse in
settings.json; update settings.json to register this entry under the PreToolUse
array for the "Bash" matcher (keep the same command string that invokes node
$HOME/.claude/hooks/dist/mcp-directory-guard.mjs and the timeout value) so the
guard runs before Bash commands execute; ensure no other PostToolUse entries
reference mcp-directory-guard and adjust tests/coverage that assume PreToolUse
semantics if present.
In @.claude/hooks/src/session-start-continuity.ts:
- Around line 319-327: The TOCTOU can cause fs.unlinkSync(pidFile) to throw if
the file was removed after the existsSync check; update the cleanup in the catch
block around the pid handling (the block that reads pidFile, calls process.kill
and then currently does fs.unlinkSync(pidFile)) to handle ENOENT gracefully by
wrapping the unlink in a try/catch (or using fs.rmSync with force: true) and
only rethrowing unexpected errors — reference the pidFile variable and the use
of fs.unlinkSync / fs.existsSync / process.kill when making this change.
In @.claude/scripts/status.py:
- Around line 13-16: The docstring at the top says Red (ctx ≥ 80%) should show
only the NOW focus, but the rendering logic currently can emit both "GOAL:" and
"NOW:" together; update the rendering to match the docstring by suppressing
emission of the "GOAL:" label when ctx >= 0.80 so only the "NOW:" section is
produced in critical mode. Find the places that append or format the "GOAL:" and
"NOW:" strings (search for the literals "GOAL:" and "NOW:" in this file) and add
a conditional that skips adding "GOAL:" (and its content) when ctx is in the red
range; ensure any helper functions that assemble the critical-path output follow
the same condition so no duplicate GOAL appears in critical mode and adjust or
add a small unit/test or comment to reflect the intended behavior.
In @.claude/skills/agent-context-isolation/SKILL.md:
- Around line 11-13: Edit the "When to Use" section header text to fix the
sentence fragment: change the two sentences into one complete sentence — e.g.,
"This skill is invoked when coordinating with sub-agents and is not
user-invocable directly." — so the section reads as a single clear statement;
update the text under the "When to Use" heading in SKILL.md accordingly.
In @.claude/skills/discovery-interview/SKILL.md:
- Line 16: The trigger text containing the mixed-language fragment '"I want to
build X" but can't describe的具体' is malformed; replace that phrase with a clear
English wording such as: "User says 'I want to build X' but can't describe the
specifics" (or "can't describe the details") so the trigger is grammatically
correct and unambiguous; update the SKILL.md entry that currently contains the
quoted fragment to the suggested wording.
In @.claude/skills/git-commits/SKILL.md:
- Line 45: The script invocation uses an undefined env var CLAUDE_CC_DIR; change
it to the correct project variable (either CLAUDE_CONFIG_DIR or CLAUDE_OPC_DIR)
that actually points to the directory containing
.claude/scripts/generate-reasoning.sh, update the line bash
"$CLAUDE_CC_DIR/.claude/scripts/generate-reasoning.sh" <hash> "<message>" to use
the chosen variable (e.g., bash
"$CLAUDE_CONFIG_DIR/.claude/scripts/generate-reasoning.sh" ...), and
search/update other occurrences of CLAUDE_CC_DIR in the repo so all references
are consistent.
In @.claude/skills/hook-developer/SKILL.md:
- Line 712: Update the "Available to All Hooks" env-var table to include
CLAUDE_CC_DIR (and clarify its relationship to CLAUDE_PROJECT_DIR) so it matches
the guidance on line 712; find the table that currently lists only
CLAUDE_PROJECT_DIR and add a row or column for CLAUDE_CC_DIR with a short
description (e.g., "C/C++ project root used by hooks — preferred for C/C++
paths") and, if relevant, note that CLAUDE_CC_DIR supersedes or is distinct from
CLAUDE_PROJECT_DIR for C/C++ hooks.
In @.claude/skills/implement-plan/SKILL.md:
- Line 2: The skill metadata name was changed to the hyphenated form
"implement-plan" but the invocation example still shows "/implement_plan";
update the invocation example(s) to "/implement-plan" or explicitly add an alias
mapping so both "/implement-plan" and "/implement_plan" work; specifically
locate the "name: implement-plan" entry and change the trigger example string(s)
from "/implement_plan" to "/implement-plan" (or add the alias) so the documented
command matches the metadata.
In @.claude/skills/math-unified/SKILL.md:
- Around line 157-179: The fenced example blocks in SKILL.md lack language tags
and trigger markdownlint MD040; update each triple-backtick block that contains
the example commands (the ones showing uv run python
"$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" solve ..., eigenvalues ...,
z3_solve.py prove ..., and pint_compute.py convert ...) to include a language
hint (e.g., ```text) immediately after the opening backticks so the code fences
are language-tagged and lint-compliant.
- Line 34: The script invocations currently reference the wrong environment
variable and path (e.g., the line starting with uv run python
"$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" <command> <args>); update
those calls to use "$CLAUDE_PROJECT_DIR/.claude/scripts/cc_math/" instead of
"$CLAUDE_OPC_DIR/scripts/cc_math/". Replace every occurrence of
CLAUDE_OPC_DIR/scripts/cc_math/ (lines noted in the review) with
CLAUDE_PROJECT_DIR/.claude/scripts/cc_math/ so the commands (e.g.,
sympy_compute.py, other cc_math scripts) point to the correct
.claude/scripts/cc_math directory.
In @.claude/skills/resume-handoff/SKILL.md:
- Around line 13-17: The docs use mixed command names (/resume-handoff vs
/resume_handoff) causing invocation confusion; pick a single canonical command
(e.g., /resume-handoff) and update all occurrences in SKILL.md to that form, or
explicitly declare both as aliases (e.g., "Commands: /resume-handoff (alias:
/resume_handoff)") wherever the commands are listed or shown (references: the
commands "/resume-handoff" and "/resume_handoff" in the file and any
examples/usage lines) so readers know both are accepted.
In `@opc/scripts/setup/claude_integration.py`:
- Around line 967-991: The cleanup only removes CLAUDE_OPC_DIR from the rc for
the current SHELL; update the logic in this block so it targets all supported
shell config files (always include ~/.zshrc, ~/.bashrc, and
~/.config/fish/config.fish when present) instead of branching on
_os.environ["SHELL"]; then change the removal filter so new_lines excludes lines
containing either "CLAUDE_OPC_DIR" or "CLAUDE_CC_DIR" (e.g., use a condition
like: keep line if neither var string is in it), and keep the existing
try/except and msg_parts update behavior (refer to configs_to_clean,
config_path, and the CLAUDE_OPC_DIR/CLAUDE_CC_DIR strings).
In `@opc/scripts/setup/wizard.py`:
- Around line 947-979: The current logic only appends exports when variable
names are missing, leaving stale values; update the block that handles
shell_config/export_opc/export_cc so it rewrites any existing managed exports
instead of just checking for presence: detect a managed marker comment (or lines
matching CLAUDE_OPC_DIR/CLAUDE_CC_DIR) in the content read from shell_config,
build the correct export strings for the detected shell (export_opc/export_cc
for bash/zsh or set -gx for fish), remove or replace any existing managed lines,
and write back a single managed block (e.g. wrapped with "# Continuous-Claude
directories - managed start" / " - managed end") containing the current
export_opc and export_cc; ensure changed is set when replacement or addition
occurs and preserve unrelated content.
- Around line 689-693: The container runtime prerequisite check currently blocks
progress even when db_mode is "embedded" or "sqlite"; update the
prereqs["docker"]/container runtime validation to skip or not require
Docker/Podman when db_mode is "embedded" or "sqlite" (i.e., only enforce
prereqs["docker"] when db_mode requires containers), and ensure the user prompt
that uses runtime/Confirm.ask still reflects that check; locate and modify the
logic around db_mode, prereqs (prereqs["docker"]), and the Confirm.ask/runtime
branch so embedded/sqlite flows bypass the container requirement and allow the
script to continue.
---
Outside diff comments:
In @.claude/settings.json:
- Around line 228-242: The mcp-directory-guard hook is currently registered in
PostToolUse but must run before commands to block bad executions; move the Bash
hook entry that runs "node $HOME/.claude/hooks/dist/mcp-directory-guard.mjs"
(the command hook with timeout 5) out of the PostToolUse block and add the same
matcher+hook object under the PreToolUse section so the guard executes prior to
tool/command invocation.
In `@opc/scripts/setup/wizard.py`:
- Around line 481-497: The Docker branch manually interpolates password into
CONTINUOUS_CLAUDE_DB_URL which breaks on special chars and deviates from
generate_database_url(); replace the manual url construction in the mode ==
"docker" block with a call to the shared generate_database_url(db) (or the
canonical DB URL builder used elsewhere) so the password is properly encoded and
empty-password behavior matches the rest of the code, while keeping the existing
POSTGRES_* env lines and the CONTINUOUS_CLAUDE_DB_URL variable name.
---
Nitpick comments:
In @.claude/hooks/src/mcp-directory-guard.ts:
- Around line 33-44: The RegExp built in buildCdPrefixPattern(opcDir) can be
flagged for ReDoS because it interpolates escapedDir; to fix, validate opcDir
before using it: in buildCdPrefixPattern (and the caller getOpcDir flow) enforce
a reasonable length limit (e.g., max path length) and ensure the value only
contains safe filesystem/path characters (e.g., letters, digits, /, -, _, ., ~)
and if validation fails, omit using escapedDir (fall back to only the
$CLAUDE_OPC_DIR variants) or throw/return a safe default; this keeps the
existing escaping but prevents long or unexpected strings from being embedded in
the regex.
In @.claude/hooks/src/session-start-continuity.ts:
- Around line 316-351: ensureMemoryDaemon currently fire-and-forgets starting
the daemon via spawn('uv'...) with detached/unref and only catches synchronous
throws; add a brief comment near ensureMemoryDaemon explaining this is
intentional best-effort fire-and-forget startup, and attach a no-op error
handler to the spawned child (listen for the child's 'error' event) to avoid
unhandled-error warnings; also either use the returned string at the call site
or simplify ensureMemoryDaemon's signature to return void (update callers
accordingly) so the return type (string | null) reflects actual use.
In @.claude/skills/completion-check/SKILL.md:
- Around line 11-13: Update the "When to Use" section to explicitly state who or
what triggers the skill by adding a short sentence (e.g., "automatically
triggered by the system after infrastructure tasks complete" or "called by the
task orchestrator") so it complements the frontmatter setting user-invocable:
false; edit the "When to Use" paragraph in SKILL.md to append one clear
invocation sentence that references the automated system/orchestrator.
In @.claude/skills/index-at-creation/SKILL.md:
- Around line 11-13: The "When to Use" section contains a sentence fragment
under the heading "When to Use" ("Not user-invocable directly."); update that
fragment to a complete sentence such as "It is not user-invocable directly." so
the section reads clearly and aligns with the metadata while keeping the same
meaning.
In @.claude/skills/parallel-agents/SKILL.md:
- Around line 11-13: The "When to Use" section contains a redundant sentence
"Not user-invocable directly" that repeats the frontmatter setting
'user-invocable: false'; remove that sentence from the "When to Use" paragraph
(or reword the paragraph to omit that constraint) so the documentation no longer
duplicates the frontmatter while keeping the explanatory line "This skill is
invoked when launching multiple agents in parallel."
In @.claude/skills/premortem/SKILL.md:
- Around line 11-19: Consolidate duplicate timing bullets in the "When to Use"
section by merging the two similar items "Before starting implementation of a
new feature or plan" and "After a plan is created and before implementation
starts" into a single concise bullet (e.g., "After planning and before
implementation starts"); update the list under the "When to Use" header in
.claude/skills/premortem/SKILL.md to replace those two lines with the single
consolidated line to improve brevity and avoid redundancy.
In @.claude/skills/repo-research-analyst/SKILL.md:
- Around line 12-14: The "When to Use" section is too generic and mixes
invocation details; update the "When to Use" block to list concrete scenarios
(e.g., onboarding to a new codebase, preparing to contribute to unfamiliar
repositories, documenting team conventions) and move the invocation detail to
its own line or bolded "Invocation: /repo-research-analyst" so the section is
actionable and clearer; edit the header text "When to Use" and the paragraph
below it in SKILL.md to reflect these changes.
In @.claude/skills/tldr-router/SKILL.md:
- Around line 10-12: The "When to Use" section currently has a short second
sentence; improve readability by expanding that sentence in
.claude/skills/tldr-router/SKILL.md under the "When to Use" header to give a bit
more context about when and why users would invoke the skill (for example,
mention typical question types, expected output, or when to prefer /tldr-router
over other commands); update the sentence that references "/tldr-router" to a
clearer, slightly longer phrasing so readers immediately understand invocation
and purpose.
In @.claude/skills/tour/SKILL.md:
- Around line 10-12: The second sentence in the "When to Use" section
("User-invocable via /tour.") is terse—reword it in SKILL.md to improve
readability by replacing that sentence with a clearer phrase (e.g., "Invokable
by users via the /tour command.") so the "When to Use" block and the sentence
about invocation are more natural; update the text under the "When to Use"
heading accordingly.
🪄 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: bf93fa9e-c7c8-4929-93b9-b7afbaedd578
⛔ Files ignored due to path filters (3)
.claude/hooks/dist/mcp-directory-guard.mjsis excluded by!**/dist/**,!**/dist/**,!**/.claude/hooks/dist/**.claude/hooks/dist/session-start-continuity.mjsis excluded by!**/dist/**,!**/dist/**,!**/.claude/hooks/dist/**.claude/hooks/dist/tldr-read-enforcer.mjsis excluded by!**/dist/**,!**/dist/**,!**/.claude/hooks/dist/**
📒 Files selected for processing (128)
.claude/agents/aegis.json.claude/agents/aegis.md.claude/agents/agentica-agent.md.claude/agents/arbiter.json.claude/agents/arbiter.md.claude/agents/architect.json.claude/agents/architect.md.claude/agents/atlas.json.claude/agents/atlas.md.claude/agents/braintrust-analyst.md.claude/agents/critic.md.claude/agents/debug-agent.md.claude/agents/herald.md.claude/agents/judge.md.claude/agents/kraken.json.claude/agents/kraken.md.claude/agents/liaison.md.claude/agents/maestro.json.claude/agents/maestro.md.claude/agents/memory-extractor.md.claude/agents/oracle.json.claude/agents/oracle.md.claude/agents/pathfinder.json.claude/agents/pathfinder.md.claude/agents/phoenix.json.claude/agents/phoenix.md.claude/agents/plan-agent.md.claude/agents/profiler.json.claude/agents/profiler.md.claude/agents/review-agent.md.claude/agents/scout.json.claude/agents/scout.md.claude/agents/scribe.json.claude/agents/scribe.md.claude/agents/sentinel.json.claude/agents/session-analyst.md.claude/agents/sleuth.json.claude/agents/sleuth.md.claude/agents/spark.json.claude/agents/spark.md.claude/agents/surveyor.md.claude/agents/validate-agent.md.claude/agents/warden.json.claude/hooks/compiler-in-the-loop-stop.sh.claude/hooks/compiler-in-the-loop.sh.claude/hooks/src/mcp-directory-guard.ts.claude/hooks/src/session-start-continuity.ts.claude/hooks/src/tldr-read-enforcer.ts.claude/scripts/status.py.claude/settings.json.claude/skills/agent-context-isolation/SKILL.md.claude/skills/agentic-workflow/SKILL.md.claude/skills/agentica-prompts/SKILL.md.claude/skills/async-repl-protocol/SKILL.md.claude/skills/background-agent-pings/SKILL.md.claude/skills/braintrust-tracing/SKILL.md.claude/skills/build/SKILL.md.claude/skills/commit/SKILL.md.claude/skills/commit/SKILL.v6.md.claude/skills/completion-check/SKILL.md.claude/skills/compound-learnings/SKILL.md.claude/skills/continuity-ledger/SKILL.md.claude/skills/create-handoff/SKILL.md.claude/skills/create-handoff/SKILL.v6.md.claude/skills/dead-code/SKILL.md.claude/skills/debug-hooks/SKILL.md.claude/skills/debug/SKILL.md.claude/skills/describe-pr/SKILL.md.claude/skills/discovery-interview/SKILL.md.claude/skills/environment-triage/SKILL.md.claude/skills/explicit-identity/SKILL.md.claude/skills/explore/SKILL.md.claude/skills/fix/SKILL.md.claude/skills/git-commits/SKILL.md.claude/skills/graceful-degradation/SKILL.md.claude/skills/help/SKILL.md.claude/skills/hook-developer/SKILL.md.claude/skills/hooks/SKILL.md.claude/skills/idempotent-redundancy/SKILL.md.claude/skills/implement-plan/SKILL.md.claude/skills/implement-plan/SKILL.md.backup.claude/skills/implement-task/SKILL.md.claude/skills/implement_plan_micro/SKILL.md.claude/skills/index-at-creation/SKILL.md.claude/skills/llm-tuning-patterns/SKILL.md.claude/skills/math-help/SKILL.md.claude/skills/math-router/SKILL.md.claude/skills/math-unified/SKILL.md.claude/skills/mcp-scripts/SKILL.md.claude/skills/modular-code/SKILL.md.claude/skills/mot/SKILL.md.claude/skills/nia-docs/SKILL.md.claude/skills/no-polling-agents/SKILL.md.claude/skills/no-task-output/SKILL.md.claude/skills/observe-before-editing/SKILL.md.claude/skills/opc-architecture/SKILL.md.claude/skills/parallel-agent-contracts/SKILL.md.claude/skills/parallel-agents/SKILL.md.claude/skills/plan-agent/SKILL.md.claude/skills/premortem/SKILL.md.claude/skills/prove/SKILL.md.claude/skills/qlty-during-development/SKILL.md.claude/skills/recall-reasoning/SKILL.md.claude/skills/recall/SKILL.md.claude/skills/remember/SKILL.md.claude/skills/repo-research-analyst/SKILL.md.claude/skills/research-agent/SKILL.md.claude/skills/research-external/SKILL.md.claude/skills/research/SKILL.md.claude/skills/resume-handoff/SKILL.md.claude/skills/router-first-architecture/SKILL.md.claude/skills/search-hierarchy/SKILL.md.claude/skills/search-tools/SKILL.md.claude/skills/skill-developer/SKILL.md.claude/skills/skill-rules.json.claude/skills/system-overview/SKILL.md.claude/skills/tldr-overview/SKILL.md.claude/skills/tldr-router/SKILL.md.claude/skills/tldr-stats/SKILL.md.claude/skills/tour/SKILL.md.claude/skills/validate-agent/SKILL.md.claude/skills/wiring/SKILL.mdopc/scripts/braintrust_analyze.pyopc/scripts/cc_math/math_base.pyopc/scripts/cc_math/sympy_compute.pyopc/scripts/setup/claude_integration.pyopc/scripts/setup/embedded_postgres.pyopc/scripts/setup/wizard.py
💤 Files with no reviewable changes (4)
- .claude/skills/search-tools/SKILL.md
- .claude/skills/router-first-architecture/SKILL.md
- .claude/skills/async-repl-protocol/SKILL.md
- .claude/skills/search-hierarchy/SKILL.md
| **Also write summary to:** | ||
| ``` | ||
| $CLAUDE_PROJECT_DIR/.claude/cache/agents/phoenix/output-{timestamp}.md | ||
| $CLAUDE_CC_DIR/.claude/cache/agents/phoenix/output-{timestamp}.md | ||
| ``` |
There was a problem hiding this comment.
Add fence language for markdownlint compliance.
The code block at Line 90 lacks a language tag (MD040). Use text (or bash if you want shell semantics).
Suggested fix
-```
+```text
$CLAUDE_CC_DIR/.claude/cache/agents/phoenix/output-{timestamp}.md</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
**Also write summary to:**
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 90-90: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/phoenix.md around lines 89 - 92, Add a fence language to the
markdown code block that shows the output path to satisfy markdownlint MD040:
change the triple-backtick fence around the snippet
"$CLAUDE_CC_DIR/.claude/cache/agents/phoenix/output-{timestamp}.md" to include a
language tag (use "text" or "bash"), i.e. replace ``` with ```text so the code
block becomes fenced with a language; update the block in the
.claude/agents/phoenix.md file where the "Also write summary to:" snippet
appears.
| ``` | ||
| $CLAUDE_PROJECT_DIR/thoughts/shared/plans/[descriptive-name].md | ||
| $CLAUDE_CC_DIR/thoughts/shared/plans/[descriptive-name].md | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced block (MD040).
The fence near Line 77 should specify a language, e.g. text.
Suggested fix
-```
+```text
$CLAUDE_CC_DIR/thoughts/shared/plans/[descriptive-name].md</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/plan-agent.md around lines 77 - 79, The markdown fence shown
as "```" containing "$CLAUDE_CC_DIR/thoughts/shared/plans/[descriptive-name].md"
should include a language token to satisfy MD040; update the fenced block in
.claude/agents/plan-agent.md (the code fence at the plans example) to use a
language such as "text" (e.g., change the opening "```" to "```text") so the
block is properly annotated.
|
|
||
| ```bash | ||
| cat $CLAUDE_PROJECT_DIR/.claude/skills/braintrust-analyze/SKILL.md | ||
| cat $CLAUDE_CC_DIR/.claude/skills/braintrust-analyze/SKILL.md |
There was a problem hiding this comment.
Quote $CLAUDE_CC_DIR in shell commands to avoid path-splitting.
Line 16 and Line 24 should quote the variable expansion; otherwise paths with spaces will fail.
Suggested fix
-cat $CLAUDE_CC_DIR/.claude/skills/braintrust-analyze/SKILL.md
+cat "$CLAUDE_CC_DIR/.claude/skills/braintrust-analyze/SKILL.md"-cd $CLAUDE_CC_DIR
+cd "$CLAUDE_CC_DIR"
uv run python -m runtime.harness scripts/braintrust_analyze.py --last-sessionAlso applies to: 24-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/session-analyst.md at line 16, The shell command uses an
unquoted variable expansion (cat
$CLAUDE_CC_DIR/.claude/skills/braintrust-analyze/SKILL.md) which will break on
paths with spaces; update the occurrences on line 16 and lines 24-25 to quote
the variable expansion (e.g., use "$CLAUDE_CC_DIR" or quote the entire path) so
the CLAUDE_CC_DIR expansion is protected; search for other uses of CLAUDE_CC_DIR
in this file (and specifically the SKILL.md cat invocation) and apply the same
quoting consistently.
| # Only invoke TypeScript if Lean compiler state exists | ||
| set -e | ||
| cd "$CLAUDE_PROJECT_DIR/.claude/hooks" | ||
| cd "$HOME/.claude/hooks" |
There was a problem hiding this comment.
Guard missing hooks directory before cd to prevent hard stop-hook failures.
At Line 15, cd "$HOME/.claude/hooks" will terminate the hook under set -e if that directory is absent. Add a directory guard and return {} like the existing early-exit pattern.
Proposed fix
set -e
-cd "$HOME/.claude/hooks"
+HOOKS_DIR="$HOME/.claude/hooks"
+if [[ ! -d "$HOOKS_DIR" ]]; then
+ echo '{}'
+ exit 0
+fi
+cd "$HOOKS_DIR"
cat | npx tsx src/compiler-in-the-loop-stop.ts📝 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.
| cd "$HOME/.claude/hooks" | |
| set -e | |
| HOOKS_DIR="$HOME/.claude/hooks" | |
| if [[ ! -d "$HOOKS_DIR" ]]; then | |
| echo '{}' | |
| exit 0 | |
| fi | |
| cd "$HOOKS_DIR" | |
| cat | npx tsx src/compiler-in-the-loop-stop.ts |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/compiler-in-the-loop-stop.sh at line 15, Guard the directory
before the cd to avoid hard failures: replace the bare cd "$HOME/.claude/hooks"
invocation with an existence check using [ -d "$HOME/.claude/hooks" ] (or test
-d) and if it does not exist perform the same early-exit behavior used elsewhere
in the script (return {} or exit 0) so the hook returns gracefully instead of
failing under set -e; update the block containing the cd "$HOME/.claude/hooks"
line to first check the directory and only cd when present, otherwise return {}.
| /** | ||
| * PreToolUse:Bash Hook - OPC Script Directory Guard | ||
| * | ||
| * Prevents running scripts from `scripts/(mcp|core)/` without first | ||
| * changing to $CLAUDE_OPC_DIR. When Claude runs these scripts from the | ||
| * wrong directory, `uv run` misses `opc/pyproject.toml` and its | ||
| * dependencies, causing ModuleNotFoundError. | ||
| * | ||
| * Detection: any Bash command referencing `scripts/(mcp|core)/` paths | ||
| * Allowed: commands prefixed with `cd $CLAUDE_OPC_DIR &&` (or resolved path) | ||
| * Denied: returns corrected command in the reason message | ||
| * | ||
| * Fixes: #148 | ||
| */ | ||
|
|
||
| import { readFileSync } from 'fs'; | ||
| import { getOpcDir } from './shared/opc-path.js'; | ||
| import type { PreToolUseInput, PreToolUseHookOutput } from './shared/types.js'; |
There was a problem hiding this comment.
Critical: Hook registered as PostToolUse but designed as PreToolUse.
The code is designed as a PreToolUse hook (comments, type imports, output structure with permissionDecision: 'deny'), but in settings.json it's registered under PostToolUse. This defeats the purpose—by the time PostToolUse fires, the Bash command has already executed and potentially failed with ModuleNotFoundError.
Move the hook registration to PreToolUse in settings.json:
"PreToolUse": [
// ... existing entries ...
{
"matcher": "Bash",
"hooks": [
{
"type": "command",
"command": "node $HOME/.claude/hooks/dist/mcp-directory-guard.mjs",
"timeout": 5
}
]
}
]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/src/mcp-directory-guard.ts around lines 1 - 18, The hook
mcp-directory-guard is implemented as a PreToolUse hook (see imports
PreToolUseInput, PreToolUseHookOutput and its deny/permissionDecision behavior)
but is registered under PostToolUse in settings.json; update settings.json to
register this entry under the PreToolUse array for the "Bash" matcher (keep the
same command string that invokes node
$HOME/.claude/hooks/dist/mcp-directory-guard.mjs and the timeout value) so the
guard runs before Bash commands execute; ensure no other PostToolUse entries
reference mcp-directory-guard and adjust tests/coverage that assume PreToolUse
semantics if present.
| Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" solve "x**2 - 5*x + 6" --var x | ||
| Result: x = 2 or x = 3 | ||
| ``` | ||
|
|
||
| ### Compute Eigenvalues | ||
| ``` | ||
| User: Find eigenvalues of [[2, 1], [1, 2]] | ||
| Claude: uv run python "$CLAUDE_PROJECT_DIR/.claude/scripts/cc_math/sympy_compute.py" eigenvalues "[[2,1],[1,2]]" | ||
| Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" eigenvalues "[[2,1],[1,2]]" | ||
| Result: {1: 1, 3: 1} (eigenvalue 1 with multiplicity 1, eigenvalue 3 with multiplicity 1) | ||
| ``` | ||
|
|
||
| ### Prove Inequality | ||
| ``` | ||
| User: Is x² + y² ≥ 2xy always true? | ||
| Claude: uv run python "$CLAUDE_PROJECT_DIR/.claude/scripts/cc_math/z3_solve.py" prove "x**2 + y**2 >= 2*x*y" | ||
| Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/z3_solve.py" prove "x**2 + y**2 >= 2*x*y" | ||
| Result: PROVED (equivalent to (x-y)² ≥ 0) | ||
| ``` | ||
|
|
||
| ### Convert Units | ||
| ``` | ||
| User: How many kilometers in 26.2 miles? | ||
| Claude: uv run python "$CLAUDE_PROJECT_DIR/.claude/scripts/cc_math/pint_compute.py" convert 26.2 miles kilometers | ||
| Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/pint_compute.py" convert 26.2 miles kilometers | ||
| Result: 42.16 km |
There was a problem hiding this comment.
Add fence languages in the examples to clear markdownlint MD040.
The example fenced blocks in this section are missing a language tag and trigger linter warnings.
Suggested fix
-```
+```text
User: Solve x² - 5x + 6 = 0
Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" solve "x**2 - 5*x + 6" --var x
Result: x = 2 or x = 3- +text
User: Find eigenvalues of [[2, 1], [1, 2]]
Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" eigenvalues "[[2,1],[1,2]]"
Result: {1: 1, 3: 1} (eigenvalue 1 with multiplicity 1, eigenvalue 3 with multiplicity 1)
-```
+```text
User: Is x² + y² ≥ 2xy always true?
Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/z3_solve.py" prove "x**2 + y**2 >= 2*x*y"
Result: PROVED (equivalent to (x-y)² ≥ 0)
- +text
User: How many kilometers in 26.2 miles?
Claude: uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/pint_compute.py" convert 26.2 miles kilometers
Result: 42.16 km
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 162-162: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 176-176: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/math-unified/SKILL.md around lines 157 - 179, The fenced
example blocks in SKILL.md lack language tags and trigger markdownlint MD040;
update each triple-backtick block that contains the example commands (the ones
showing uv run python "$CLAUDE_OPC_DIR/scripts/cc_math/sympy_compute.py" solve
..., eigenvalues ..., z3_solve.py prove ..., and pint_compute.py convert ...) to
include a language hint (e.g., ```text) immediately after the opening backticks
so the code fences are language-tagged and lint-compliant.
| - User says "/resume" or "/resume-handoff" | ||
| - User says "resume this handoff" or "continue from handoff" | ||
| - User says "I have a handoff from previous session" | ||
| - User provides a path to a handoff file to continue | ||
|
|
There was a problem hiding this comment.
Unify command naming (- vs _) to avoid invocation confusion.
Line 13 introduces /resume-handoff, but Lines 46 and 48 still document /resume_handoff. Please standardize or explicitly list both as aliases.
Suggested doc fix
Activate when:
-- User says "/resume" or "/resume-handoff"
+- User says "/resume", "/resume-handoff", or "/resume_handoff"📝 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.
| - User says "/resume" or "/resume-handoff" | |
| - User says "resume this handoff" or "continue from handoff" | |
| - User says "I have a handoff from previous session" | |
| - User provides a path to a handoff file to continue | |
| - User says "/resume", "/resume-handoff", or "/resume_handoff" | |
| - User says "resume this handoff" or "continue from handoff" | |
| - User says "I have a handoff from previous session" | |
| - User provides a path to a handoff file to continue |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/resume-handoff/SKILL.md around lines 13 - 17, The docs use
mixed command names (/resume-handoff vs /resume_handoff) causing invocation
confusion; pick a single canonical command (e.g., /resume-handoff) and update
all occurrences in SKILL.md to that form, or explicitly declare both as aliases
(e.g., "Commands: /resume-handoff (alias: /resume_handoff)") wherever the
commands are listed or shown (references: the commands "/resume-handoff" and
"/resume_handoff" in the file and any examples/usage lines) so readers know both
are accepted.
| # Clean up CLAUDE_OPC_DIR from shell configs | ||
| import os as _os | ||
|
|
||
| shell = _os.environ.get("SHELL", "") | ||
| configs_to_clean = [] | ||
| if "zsh" in shell: | ||
| configs_to_clean.append(_os.path.expanduser("~/.zshrc")) | ||
| elif "bash" in shell: | ||
| configs_to_clean.append(_os.path.expanduser("~/.bashrc")) | ||
| elif "fish" in shell: | ||
| fish_config = _os.path.expanduser("~/.config/fish/config.fish") | ||
| if _os.path.exists(fish_config): | ||
| configs_to_clean.append(fish_config) | ||
|
|
||
| for config_path in configs_to_clean: | ||
| try: | ||
| with open(config_path, "r") as f: | ||
| lines = f.readlines() | ||
| new_lines = [l for l in lines if "CLAUDE_OPC_DIR" not in l] | ||
| if len(new_lines) < len(lines): | ||
| with open(config_path, "w") as f: | ||
| f.writelines(new_lines) | ||
| msg_parts.append(f" Removed CLAUDE_OPC_DIR from {_os.path.basename(config_path)}") | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Remove both managed vars from every supported shell config.
This cleanup only strips CLAUDE_OPC_DIR from the rc file selected by the current SHELL. The installer now writes CLAUDE_CC_DIR too, and uninstall can be run from a different or non-interactive shell, so stale exports are left behind.
🛠️ Suggested fix
- shell = _os.environ.get("SHELL", "")
- configs_to_clean = []
- if "zsh" in shell:
- configs_to_clean.append(_os.path.expanduser("~/.zshrc"))
- elif "bash" in shell:
- configs_to_clean.append(_os.path.expanduser("~/.bashrc"))
- elif "fish" in shell:
- fish_config = _os.path.expanduser("~/.config/fish/config.fish")
- if _os.path.exists(fish_config):
- configs_to_clean.append(fish_config)
+ configs_to_clean = [
+ _os.path.expanduser("~/.zshrc"),
+ _os.path.expanduser("~/.bashrc"),
+ _os.path.expanduser("~/.config/fish/config.fish"),
+ ]
+ managed_vars = ("CLAUDE_OPC_DIR", "CLAUDE_CC_DIR")
for config_path in configs_to_clean:
try:
with open(config_path, "r") as f:
lines = f.readlines()
- new_lines = [l for l in lines if "CLAUDE_OPC_DIR" not in l]
+ new_lines = [
+ line for line in lines if not any(var in line for var in managed_vars)
+ ]
if len(new_lines) < len(lines):
with open(config_path, "w") as f:
f.writelines(new_lines)
- msg_parts.append(f" Removed CLAUDE_OPC_DIR from {_os.path.basename(config_path)}")
+ msg_parts.append(
+ f" Removed Claude directory exports from {_os.path.basename(config_path)}"
+ )
except Exception:
pass📝 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.
| # Clean up CLAUDE_OPC_DIR from shell configs | |
| import os as _os | |
| shell = _os.environ.get("SHELL", "") | |
| configs_to_clean = [] | |
| if "zsh" in shell: | |
| configs_to_clean.append(_os.path.expanduser("~/.zshrc")) | |
| elif "bash" in shell: | |
| configs_to_clean.append(_os.path.expanduser("~/.bashrc")) | |
| elif "fish" in shell: | |
| fish_config = _os.path.expanduser("~/.config/fish/config.fish") | |
| if _os.path.exists(fish_config): | |
| configs_to_clean.append(fish_config) | |
| for config_path in configs_to_clean: | |
| try: | |
| with open(config_path, "r") as f: | |
| lines = f.readlines() | |
| new_lines = [l for l in lines if "CLAUDE_OPC_DIR" not in l] | |
| if len(new_lines) < len(lines): | |
| with open(config_path, "w") as f: | |
| f.writelines(new_lines) | |
| msg_parts.append(f" Removed CLAUDE_OPC_DIR from {_os.path.basename(config_path)}") | |
| except Exception: | |
| pass | |
| # Clean up CLAUDE_OPC_DIR from shell configs | |
| import os as _os | |
| configs_to_clean = [ | |
| _os.path.expanduser("~/.zshrc"), | |
| _os.path.expanduser("~/.bashrc"), | |
| _os.path.expanduser("~/.config/fish/config.fish"), | |
| ] | |
| managed_vars = ("CLAUDE_OPC_DIR", "CLAUDE_CC_DIR") | |
| for config_path in configs_to_clean: | |
| try: | |
| with open(config_path, "r") as f: | |
| lines = f.readlines() | |
| new_lines = [ | |
| line for line in lines if not any(var in line for var in managed_vars) | |
| ] | |
| if len(new_lines) < len(lines): | |
| with open(config_path, "w") as f: | |
| f.writelines(new_lines) | |
| msg_parts.append( | |
| f" Removed Claude directory exports from {_os.path.basename(config_path)}" | |
| ) | |
| except Exception: | |
| pass |
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 983-983: Unnecessary mode argument
Remove mode argument
(UP015)
[error] 985-985: Ambiguous variable name: l
(E741)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@opc/scripts/setup/claude_integration.py` around lines 967 - 991, The cleanup
only removes CLAUDE_OPC_DIR from the rc for the current SHELL; update the logic
in this block so it targets all supported shell config files (always include
~/.zshrc, ~/.bashrc, and ~/.config/fish/config.fish when present) instead of
branching on _os.environ["SHELL"]; then change the removal filter so new_lines
excludes lines containing either "CLAUDE_OPC_DIR" or "CLAUDE_CC_DIR" (e.g., use
a condition like: keep line if neither var string is in it), and keep the
existing try/except and msg_parts update behavior (refer to configs_to_clean,
config_path, and the CLAUDE_OPC_DIR/CLAUDE_CC_DIR strings).
| if db_mode == "embedded": | ||
| console.print(" [dim]Skipped - using embedded PostgreSQL (no Docker needed)[/dim]") | ||
| elif db_mode == "sqlite": | ||
| console.print(" [dim]Skipped - using SQLite (no Docker needed)[/dim]") | ||
| elif Confirm.ask(f"Start {runtime} stack (PostgreSQL, Redis)?", default=True): |
There was a problem hiding this comment.
Make the container runtime check depend on db_mode.
These branches say Docker isn't needed, but Step 1 still exits at Line 600 when prereqs["docker"] is false. On a machine without Docker/Podman, users can never reach embedded or sqlite.
🛠️ Suggested fix
- if not prereqs["all_present"]:
+ if not (prereqs["python"] and prereqs["uv"]):
console.print("\n[red]Cannot continue without all prerequisites.[/red]")
sys.exit(1)
@@
+ if db_mode == "docker" and not prereqs["docker"]:
+ console.print("\n[red]Container runtime required for docker mode.[/red]")
+ sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@opc/scripts/setup/wizard.py` around lines 689 - 693, The container runtime
prerequisite check currently blocks progress even when db_mode is "embedded" or
"sqlite"; update the prereqs["docker"]/container runtime validation to skip or
not require Docker/Podman when db_mode is "embedded" or "sqlite" (i.e., only
enforce prereqs["docker"] when db_mode requires containers), and ensure the user
prompt that uses runtime/Confirm.ask still reflects that check; locate and
modify the logic around db_mode, prereqs (prereqs["docker"]), and the
Confirm.ask/runtime branch so embedded/sqlite flows bypass the container
requirement and allow the script to continue.
| if shell_config and (shell_config.exists() or "fish" in shell): | ||
| if shell_config.exists(): | ||
| content = shell_config.read_text() | ||
| else: | ||
| content = "" | ||
| changed = False | ||
| export_opc = None | ||
| export_cc = None | ||
|
|
||
| if "zsh" in shell or "bash" in shell: | ||
| export_opc = f'export CLAUDE_OPC_DIR="{opc_dir}"' | ||
| export_cc = f'export CLAUDE_CC_DIR="{cc_dir}"' | ||
| elif "fish" in shell: | ||
| export_opc = f'set -gx CLAUDE_OPC_DIR "{opc_dir}"' | ||
| export_cc = f'set -gx CLAUDE_CC_DIR "{cc_dir}"' | ||
|
|
||
| if "CLAUDE_OPC_DIR" not in content: | ||
| with open(shell_config, "a") as f: | ||
| f.write(f"\n# Continuous-Claude OPC directory (for skills to find scripts)\n{export_line}\n") | ||
| console.print(f" [green]OK[/green] Added CLAUDE_OPC_DIR to {shell_config.name}") | ||
| f.write(f"\n# Continuous-Claude directories\n{export_opc}\n{export_cc}\n") | ||
| changed = True | ||
| elif "CLAUDE_CC_DIR" not in content: | ||
| with open(shell_config, "a") as f: | ||
| f.write(f"\n# Continuous-Claude root directory\n{export_cc}\n") | ||
| changed = True | ||
|
|
||
| if changed: | ||
| console.print( | ||
| f" [green]OK[/green] Added CLAUDE_OPC_DIR and CLAUDE_CC_DIR to {shell_config.name}" | ||
| ) | ||
| else: | ||
| console.print(f" [dim]CLAUDE_OPC_DIR already in {shell_config.name}[/dim]") | ||
| console.print( | ||
| f" [dim]CLAUDE_OPC_DIR and CLAUDE_CC_DIR already in {shell_config.name}[/dim]" | ||
| ) |
There was a problem hiding this comment.
Rewrite managed exports instead of checking only for presence.
Rerunning the wizard from a different checkout leaves stale CLAUDE_OPC_DIR/CLAUDE_CC_DIR values in the shell config because this only appends when the variable names are missing. The next shell session can still point at the old repo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@opc/scripts/setup/wizard.py` around lines 947 - 979, The current logic only
appends exports when variable names are missing, leaving stale values; update
the block that handles shell_config/export_opc/export_cc so it rewrites any
existing managed exports instead of just checking for presence: detect a managed
marker comment (or lines matching CLAUDE_OPC_DIR/CLAUDE_CC_DIR) in the content
read from shell_config, build the correct export strings for the detected shell
(export_opc/export_cc for bash/zsh or set -gx for fish), remove or replace any
existing managed lines, and write back a single managed block (e.g. wrapped with
"# Continuous-Claude directories - managed start" / " - managed end") containing
the current export_opc and export_cc; ensure changed is set when replacement or
addition occurs and preserve unrelated content.
|
This is pretty much a giant PR with all the other pending PRs in it. Additionally, this is a overly complex PR even if it WAS your own work. I'd never approve it. |
Summary by CodeRabbit
New Features
Improvements
qltyandast-greptoolsCLAUDE_OPC_DIRandCLAUDE_CC_DIRsupportConfiguration Updates