feat: add project context recovery skill for v0.0.50#197
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new ChangesProject Context Recovery Skill
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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 |
|
Updated this PR to follow Claude Agent Skills authoring guidance more closely:
Validation run locally: Results:
Claude Code smoke note: Claude Code detected the project skill, but this local runtime is not logged in ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/codexmate-cross-session-search/scripts/search_sessions.py`:
- Around line 39-52: The candidate_roots function supports prefix-based grouping
where selecting "codexmate-derived" would match both "codexmate-derived-codex"
and "codexmate-derived-claude" (via the startswith check), but the CLI argument
parser has hard-coded choices that only include the full specific names, making
the prefix matching logic unreachable. Add the grouped selector options
(umbrella prefixes like "codexmate-derived") to the CLI argument choices
alongside the existing individual source names. This will align the CLI choices
with the prefix-matching behavior already implemented in candidate_roots and
make all code paths reachable. Check both the candidate_roots function
definition and the CLI argument parser setup (referenced at line 217) to ensure
the grouped options are consistently exposed.
- Around line 220-223: The numeric CLI arguments (--limit, --snippets,
--max-bytes, and --max-files-per-root) in the parser.add_argument calls lack
bounds validation, allowing negative values that cause unexpected behavior.
Define a custom type validation function that ensures these arguments are
positive integers (greater than 0), then apply this validation function via the
type parameter in each of these add_argument calls. This will reject invalid
negative values at parse time rather than allowing them to produce surprising
results during execution.
🪄 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: 00ee102d-8a92-46a8-b0d3-c94b16bcf83a
📒 Files selected for processing (4)
skills/codexmate-cross-session-search/SKILL.mdskills/codexmate-cross-session-search/scripts/search_sessions.pytests/unit/codexmate-cross-session-search-skill.test.mjstests/unit/run.mjs
✅ Files skipped from review due to trivial changes (1)
- skills/codexmate-cross-session-search/SKILL.md
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.43.0)
skills/codexmate-cross-session-search/scripts/search_sessions.py
[info] 195-195: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"hits": [hit.dict for hit in hits]}, ensure_ascii=False, indent=2)
Note: Security best practice.
(use-jsonify)
🪛 Ruff (0.15.15)
skills/codexmate-cross-session-search/scripts/search_sessions.py
[warning] 113-113: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (2)
tests/unit/codexmate-cross-session-search-skill.test.mjs (1)
1-62: LGTM!tests/unit/run.mjs (1)
23-23: LGTM!
| def candidate_roots(selected: str) -> list[tuple[str, Path]]: | ||
| h = home() | ||
| roots = [ | ||
| ("codex", h / ".codex" / "sessions"), | ||
| ("claude", h / ".claude" / "projects"), | ||
| ("codexmate-derived-codex", h / ".codexmate" / "sessions" / "derived" / "codex"), | ||
| ("codexmate-derived-claude", h / ".codexmate" / "sessions" / "derived" / "claude"), | ||
| ("gemini", h / ".gemini"), | ||
| ("codebuddy", h / ".codebuddy"), | ||
| ] | ||
| if selected == "all": | ||
| return roots | ||
| return [(name, path) for name, path in roots if name == selected or name.startswith(f"{selected}-")] | ||
|
|
There was a problem hiding this comment.
Align --source choices with candidate_roots prefix behavior.
candidate_roots supports grouped prefix selection, but current CLI choices prevent using umbrella selectors (making part of that logic unreachable). Either expose grouped source choices or remove the prefix branch.
Suggested fix (expose grouped selectors)
- parser.add_argument("--source", default="all", choices=["all", "codex", "claude", "gemini", "codebuddy", "codexmate-derived-codex", "codexmate-derived-claude"], help="Session source to search")
+ parser.add_argument(
+ "--source",
+ default="all",
+ choices=[
+ "all",
+ "codex",
+ "claude",
+ "gemini",
+ "codebuddy",
+ "codexmate",
+ "codexmate-derived",
+ "codexmate-derived-codex",
+ "codexmate-derived-claude",
+ ],
+ help="Session source to search",
+ )Also applies to: 217-217
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/codexmate-cross-session-search/scripts/search_sessions.py` around
lines 39 - 52, The candidate_roots function supports prefix-based grouping where
selecting "codexmate-derived" would match both "codexmate-derived-codex" and
"codexmate-derived-claude" (via the startswith check), but the CLI argument
parser has hard-coded choices that only include the full specific names, making
the prefix matching logic unreachable. Add the grouped selector options
(umbrella prefixes like "codexmate-derived") to the CLI argument choices
alongside the existing individual source names. This will align the CLI choices
with the prefix-matching behavior already implemented in candidate_roots and
make all code paths reachable. Check both the candidate_roots function
definition and the CLI argument parser setup (referenced at line 217) to ensure
the grouped options are consistently exposed.
| parser.add_argument("--limit", type=int, default=DEFAULT_LIMIT, help="Maximum hits to print") | ||
| parser.add_argument("--snippets", type=int, default=2, help="Maximum snippets per hit") | ||
| parser.add_argument("--max-bytes", type=int, default=DEFAULT_MAX_BYTES, help="Tail bytes to scan per session file") | ||
| parser.add_argument("--max-files-per-root", type=int, default=5000, help="Maximum files to scan per root") |
There was a problem hiding this comment.
Validate numeric CLI bounds at parse time.
Negative values currently produce surprising behavior (for example, --limit -1 slices results unexpectedly, and negative scan limits can short-circuit traversal). Add explicit integer bounds in argument parsing.
Suggested fix
+def positive_int(value: str) -> int:
+ ivalue = int(value)
+ if ivalue <= 0:
+ raise argparse.ArgumentTypeError("must be > 0")
+ return ivalue
+
def main() -> int:
@@
- parser.add_argument("--limit", type=int, default=DEFAULT_LIMIT, help="Maximum hits to print")
- parser.add_argument("--snippets", type=int, default=2, help="Maximum snippets per hit")
- parser.add_argument("--max-bytes", type=int, default=DEFAULT_MAX_BYTES, help="Tail bytes to scan per session file")
- parser.add_argument("--max-files-per-root", type=int, default=5000, help="Maximum files to scan per root")
+ parser.add_argument("--limit", type=positive_int, default=DEFAULT_LIMIT, help="Maximum hits to print")
+ parser.add_argument("--snippets", type=positive_int, default=2, help="Maximum snippets per hit")
+ parser.add_argument("--max-bytes", type=positive_int, default=DEFAULT_MAX_BYTES, help="Tail bytes to scan per session file")
+ parser.add_argument("--max-files-per-root", type=positive_int, default=5000, help="Maximum files to scan per root")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/codexmate-cross-session-search/scripts/search_sessions.py` around
lines 220 - 223, The numeric CLI arguments (--limit, --snippets, --max-bytes,
and --max-files-per-root) in the parser.add_argument calls lack bounds
validation, allowing negative values that cause unexpected behavior. Define a
custom type validation function that ensures these arguments are positive
integers (greater than 0), then apply this validation function via the type
parameter in each of these add_argument calls. This will reject invalid negative
values at parse time rather than allowing them to produce surprising results
during execution.
|
Follow-up after skepticism/negative testing: This skill should be described as a best-effort local session evidence locator, not a semantic memory system. I found and fixed a false-positive case in the search script: a hyphenated unknown query such as Additional validation: Results:
Scope clarified in |
awsl233777
changed the title
|
Implemented the proposed “方案 B” direction. The PR no longer positions this as a strong semantic search/memory feature. It is now a project context recovery / handoff brief skill:
Validation run locally: Results:
Remaining honest limitation: the brief extraction is still heuristic. It improves the handoff value beyond raw grep, but it is not a full semantic memory/indexing system. Mutable facts still require live checks. |
|
Addressed the two actionable CodeRabbit findings on the current implementation:
Regression coverage added:
Validation after the fix: Results:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
skills/codexmate-project-context-recovery/scripts/search_sessions.py (1)
254-258: 💤 Low valueConsider catching a more specific exception type.
The bare
except Exceptioncatch is overly broad. Sincejson.loadsraisesjson.JSONDecodeError(subclass ofValueError), catching that specifically would be more precise. However, given this is a defensive parser for unknown session formats, the broad catch is acceptable if intentional.♻️ Optional fix for more specific exception handling
if m: try: return json.loads('"' + m.group(1) + '"') - except Exception: + except (json.JSONDecodeError, UnicodeDecodeError): return m.group(1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/codexmate-project-context-recovery/scripts/search_sessions.py` around lines 254 - 258, Replace the overly broad `except Exception` clause in the json.loads error handling block with a more specific exception type. Since json.loads raises json.JSONDecodeError when it fails to parse the JSON string, change the except clause to catch json.JSONDecodeError specifically instead of the generic Exception class. This makes the error handling more precise and clearly documents what exceptions are actually expected from the json.loads call.Source: Linters/SAST tools
tests/unit/codexmate-project-context-recovery-skill.test.mjs (2)
31-32: ⚡ Quick winConsider emitting a skip message when python3 is unavailable.
The test silently returns when
python3is not found. While this prevents spurious failures, it also hides that the test didn't run.💡 Suggested enhancement
test('project context recovery skill script finds evidence and builds a handoff brief', () => { - if (!hasPython3()) return; + if (!hasPython3()) { + console.log('⊘ Skipping project-context-recovery skill test: python3 not found'); + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/codexmate-project-context-recovery-skill.test.mjs` around lines 31 - 32, The test for "project context recovery skill script finds evidence and builds a handoff brief" silently returns when python3 is unavailable via the hasPython3() check, making it unclear that the test was skipped rather than passed. Replace the silent return with the test framework's skip mechanism (such as test.skip()) to emit a skip message that makes it visible when the test is not executed due to missing python3.
82-89: Brief mode assertions depend on internal script scoring and term matching.Lines 83 and 88 assert specific confidence levels and evidence content that depend on:
- The script's internal score calculation reaching >= 20 for
confidence='high'(Line 83)- The script's
VALIDATION_TERMSmatching"npm run lint"text (Line 88)These assertions are valid for the current implementation but may be fragile if the script's scoring logic or term lists change. Consider this when maintaining the test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/codexmate-project-context-recovery-skill.test.mjs` around lines 82 - 89, The test assertions at lines 83 and 88 are fragile because they depend on internal implementation details of the script: the confidence level assertion depends on the script's internal score calculation logic, and the validation term assertion depends on specific strings in the script's VALIDATION_TERMS. To make the test more maintainable, either refactor these assertions to test the behavior rather than specific implementation values, or add explanatory comments documenting exactly which internal script components these assertions depend on and under what conditions they might need updating.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/codexmate-project-context-recovery/SKILL.md`:
- Around line 18-26: The quick-start command examples in SKILL.md reference
`python3 scripts/search_sessions.py` without clarifying the actual location of
this script after npm installation. Add a clarification note in the
documentation that explains either the full path to the script within the
installed node_modules directory
(node_modules/codexmate/skills/codexmate-project-context-recovery/scripts/search_sessions.py)
or documents an alternative invocation pattern (such as a CLI entrypoint if
available). This will help users understand how to properly locate and invoke
the script after installation rather than assuming they are running from within
the skills directory.
- Around line 39-43: The SKILL.md file contains mixed-language section headers
with Chinese characters (`当前对象`, `历史证据`, `接手简报`, `实时核查`) that are inconsistent
with the rest of the document which is written in English. Replace each of these
four Chinese headers with their English equivalents to maintain language
consistency throughout the document. Ensure all section headers in the file use
the same language to improve readability for all audiences.
- Line 33: The confidence level guidance at line 33 in the SKILL.md file is
incomplete. Currently it documents `high` and `medium/weak` confidence levels,
but omits the `none` confidence level that the brief builder can emit when no
search hits are found. Update the documentation to include all three confidence
levels by adding a case for `none` confidence, explaining that it indicates no
hits were found and guidance on how to proceed when encountering this level.
In `@tests/unit/run.mjs`:
- Line 23: Add a missing import statement for the cross-session search skill
test file in the test runner. In run.mjs, after the existing import statement
for codexmate-project-context-recovery-skill.test.mjs, add a new import
statement that follows the same pattern to dynamically import
codexmate-cross-session-search-skill.test.mjs using pathToFileURL and path.join
with __dirname. This ensures both skill test files are registered in the
runner's dynamic import sequence.
---
Nitpick comments:
In `@skills/codexmate-project-context-recovery/scripts/search_sessions.py`:
- Around line 254-258: Replace the overly broad `except Exception` clause in the
json.loads error handling block with a more specific exception type. Since
json.loads raises json.JSONDecodeError when it fails to parse the JSON string,
change the except clause to catch json.JSONDecodeError specifically instead of
the generic Exception class. This makes the error handling more precise and
clearly documents what exceptions are actually expected from the json.loads
call.
In `@tests/unit/codexmate-project-context-recovery-skill.test.mjs`:
- Around line 31-32: The test for "project context recovery skill script finds
evidence and builds a handoff brief" silently returns when python3 is
unavailable via the hasPython3() check, making it unclear that the test was
skipped rather than passed. Replace the silent return with the test framework's
skip mechanism (such as test.skip()) to emit a skip message that makes it
visible when the test is not executed due to missing python3.
- Around line 82-89: The test assertions at lines 83 and 88 are fragile because
they depend on internal implementation details of the script: the confidence
level assertion depends on the script's internal score calculation logic, and
the validation term assertion depends on specific strings in the script's
VALIDATION_TERMS. To make the test more maintainable, either refactor these
assertions to test the behavior rather than specific implementation values, or
add explanatory comments documenting exactly which internal script components
these assertions depend on and under what conditions they might need updating.
🪄 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: 8ba8cf07-63df-427c-9437-654659e9bf16
📒 Files selected for processing (5)
skills/codexmate-project-context-recovery/SKILL.mdskills/codexmate-project-context-recovery/scripts/search_sessions.pytests/unit/codexmate-cross-session-search-skill.test.mjstests/unit/codexmate-project-context-recovery-skill.test.mjstests/unit/run.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/codexmate-cross-session-search-skill.test.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.43.0)
skills/codexmate-project-context-recovery/scripts/search_sessions.py
[info] 455-455: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"hits": [hit.dict for hit in hits]}, ensure_ascii=False, indent=2)
Note: Security best practice.
(use-jsonify)
[info] 528-528: use jsonify instead of json.dumps for JSON output
Context: json.dumps(brief, ensure_ascii=False, indent=2)
Note: Security best practice.
(use-jsonify)
🪛 Ruff (0.15.15)
skills/codexmate-project-context-recovery/scripts/search_sessions.py
[warning] 256-256: Do not catch blind exception: Exception
(BLE001)
[warning] 335-335: String contains ambiguous ！ (FULLWIDTH EXCLAMATION MARK). Did you mean ! (EXCLAMATION MARK)?
(RUF001)
[warning] 335-335: String contains ambiguous ？ (FULLWIDTH QUESTION MARK). Did you mean ? (QUESTION MARK)?
(RUF001)
🔇 Additional comments (10)
skills/codexmate-project-context-recovery/scripts/search_sessions.py (7)
1-56: LGTM!
58-68: LGTM!
71-115: LGTM!
118-155: LGTM!
261-346: LGTM!
349-391: LGTM!
394-539: LGTM!tests/unit/codexmate-project-context-recovery-skill.test.mjs (3)
1-21: LGTM!
23-29: LGTM!
91-100: LGTM!
| await import(pathToFileURL(path.join(__dirname, 'task-codex-exec-windows.test.mjs'))); | ||
| await import(pathToFileURL(path.join(__dirname, 'skills-modal-ui.test.mjs'))); | ||
| await import(pathToFileURL(path.join(__dirname, 'skills-market-runtime.test.mjs'))); | ||
| await import(pathToFileURL(path.join(__dirname, 'codexmate-project-context-recovery-skill.test.mjs'))); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if cross-session-search skill test exists and is registered in runner
echo "=== Checking if test file exists ==="
fd -t f 'codexmate-cross-session-search-skill.test.mjs' tests/
echo ""
echo "=== Checking if it's imported in run.mjs ==="
rg -n 'codexmate-cross-session-search-skill' tests/unit/run.mjsRepository: SakuraByteCore/codexmate
Length of output: 208
Add import for codexmate-cross-session-search-skill.test.mjs to the test runner.
The test file codexmate-cross-session-search-skill.test.mjs exists but is not imported in run.mjs. The layer description indicates both the cross-session search skill test and the project-context recovery skill test should be registered in the runner's dynamic import sequence. Add the missing import alongside the existing codexmate-project-context-recovery-skill.test.mjs import.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/run.mjs` at line 23, Add a missing import statement for the
cross-session search skill test file in the test runner. In run.mjs, after the
existing import statement for codexmate-project-context-recovery-skill.test.mjs,
add a new import statement that follows the same pattern to dynamically import
codexmate-cross-session-search-skill.test.mjs using pathToFileURL and path.join
with __dirname. This ensures both skill test files are registered in the
runner's dynamic import sequence.
|
Addressed the current
I did not re-add Validation: Results:
|
|
Updated this PR to bump the package version to Changes:
Validation before committing: Results:
Note: I used |
awsl233777
changed the title
2 participants
Summary
0.0.49to0.0.50inpackage.jsonandpackage-lock.jsonwithout creating a git tagcodexmate-project-context-recovery, a project handoff/context recovery AgentSkill for local agent session historyscripts/search_sessions.pyhelper with two modes:--mode search: raw local session evidence search--mode brief: structured project context brief with confidence, timeline, repo/branch/PR/file signals, commands, validations, risks, and handoff guidance--path-filter,--source codexmate-derived, stricter hyphen/slash matching, and positive integer validation for numeric CLI optionscodexmate-cross-session-searchskill/test with the project context recovery skill/testValidation
npm run lintnpm run test:unitpython3 ~/.npm-global/lib/node_modules/openclaw/skills/skill-creator/scripts/package_skill.py skills/codexmate-project-context-recovery /tmp/codexmate-skill-distnpm pack --dry-run --jsonpython3 skills/codexmate-project-context-recovery/scripts/search_sessions.py 'feat/task-orchestration-tab' --mode brief --source all --path-filter codexmate --match all --limit 2 --format textpython3 skills/codexmate-project-context-recovery/scripts/search_sessions.py 'completely-nonexistent-token-zzq-197' --source all --path-filter codexmate --match all --limit 2 --format textValidation Results
skills/codexmate-project-context-recovery/SKILL.mdskills/codexmate-project-context-recovery/scripts/search_sessions.py--source codexmate-derivedand negative numeric argument validation are covered by unit testsNotes
npm version 0.0.50 --no-git-tag-version; no local or remote git tag was created.Not logged in · Please run /login), so model-level end-to-end behavior could not be completed here.Summary by CodeRabbit
New Features
Tests