feat: add save skill and improve kaizen plugin - #44
Conversation
📝 WalkthroughWalkthroughAdds a manual "save session as skill" feature (design doc and SKILL.md), introduces a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Client
participant PluginLogic as "Kaizen Plugin\nAnalyzer/Generator"
participant FS as "Filesystem\n(~/.claude/skills)"
User->>Client: invoke /kaizen:save (or interactive save)
Client->>PluginLogic: send session trajectory & context
PluginLogic->>PluginLogic: analyze trajectory, detect pattern, identify params
PluginLogic->>PluginLogic: generate SKILL.md and optional helper scripts
PluginLogic->>FS: write `~/.claude/skills/{skill-name}/SKILL.md` (+scripts)
PluginLogic->>Client: return summary, usage instructions, conflict warnings
Client->>User: display save result and next steps
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@plugins/kaizen/README.md`:
- Around line 84-91: The README mentions the /guidelines:save skill but the
plugin structure tree omits it; add an entry for skills/save/SKILL.md under the
plugin structure (alongside skills/learn and skills/recall) or clarify where the
save SKILL lives if it’s intentionally located elsewhere, ensuring the tree
includes "skills/save/SKILL.md" so readers can find the generated SKILL and
helper scripts.
In `@plugins/kaizen/scripts/retrieve_guidelines.py`:
- Around line 95-98: The current logging prints the entire input_data JSON (via
log(f" Full content: {json.dumps(input_data, indent=2)}")), which may expose
sensitive user content; change this so only non-sensitive metadata is logged by
default (keep log(f" Keys: {list(input_data.keys())}") and remove or disable
the full JSON dump). If verbose debugging is needed, gate the full dump behind
an opt-in environment variable (e.g., GUIDELINES_VERBOSE_LOG) and only log
json.dumps(input_data, ...) when that var is true. Also ensure the plugin log
file creation/rotation code sets restrictive permissions (owner-read/write only,
e.g., 0o600 via open/os.chmod or secure os.open flags) for
/tmp/guidelines-plugin.log so persisted data isn’t world-readable. Target the
calls to log and the code paths that open/write /tmp/guidelines-plugin.log and
the input_data usage.
In `@plugins/kaizen/skills/learn/SKILL.md`:
- Line 103: Update the SKILL.md line that claims guidelines are "automatically
saved when a conversation ends via the Stop hook" to reflect the current
behavior now that the Stop hook was removed; locate the sentence in SKILL.md and
either remove the clause referencing the Stop hook or replace it with the
accurate mechanism (e.g., state that guidelines are saved on conversation end
via the End/Close hook or via an explicit save action), and ensure any mention
of "Stop hook" is removed or corrected to reference the actual hook name used in
plugins/kaizen/hooks/hooks.json.
In `@SAVE_SKILL_DESIGN.md`:
- Around line 221-244: The document has duplicate "Step 7" headings: "Step 7:
Save Skill" and "Step 7: Provide Summary"; rename the second heading ("Step 7:
Provide Summary") to "Step 8: Provide Summary" and update any internal
references or numbering that point to it so the sequence is continuous; ensure
the table of contents or any cross-references (if present) reflect the new "Step
8" heading.
🧹 Nitpick comments (2)
SAVE_SKILL_DESIGN.md (1)
248-266: Example output contains hardcoded user-specific path.Line 251 uses
/Users/vinod/data/digitallabor/src/agents/kaizen/skills/which is specific to a developer's machine. Consider using a generic placeholder like~/.claude/skills/to be consistent with the documented storage location.📝 Proposed fix
Skill Name: {skill-name} -Location: /Users/vinod/data/digitallabor/src/agents/kaizen/skills/{skill-name}/SKILL.md +Location: ~/.claude/skills/{skill-name}/SKILL.mdplugins/kaizen/README.md (1)
73-90: Vary the repeated “Manually invoke …” phrasing for readability.Three successive sections start with the same phrase, which reads a bit monotonous. Consider rephrasing one or two headings/lines.
✍️ Example rewording
- Manually invoke to retrieve and display stored guidelines. + Use this skill to retrieve and display stored guidelines.
| log("=== Input Data ===") | ||
| log(f" Keys: {list(input_data.keys())}") | ||
| log(f" Full content: {json.dumps(input_data, indent=2)}") | ||
| log("=== End Input Data ===") |
There was a problem hiding this comment.
Logging full prompt content may expose sensitive user data.
The Full content log at line 97 writes the entire JSON input (including user prompts) to /tmp/guidelines-plugin.log. If users include sensitive information in their prompts (credentials, PII, API keys in commands), this could be persisted in a world-readable location.
Consider:
- Limiting log verbosity to non-sensitive fields (e.g., just the keys)
- Ensuring the log file has restrictive permissions
- Making verbose logging opt-in via an environment variable
🛡️ Proposed fix to reduce exposure
try:
input_data = json.load(sys.stdin)
log("=== Input Data ===")
log(f" Keys: {list(input_data.keys())}")
- log(f" Full content: {json.dumps(input_data, indent=2)}")
+ # Only log full content in debug mode to avoid exposing sensitive prompt data
+ if os.environ.get("KAIZEN_DEBUG"):
+ log(f" Full content: {json.dumps(input_data, indent=2)}")
log("=== End Input Data ===")🤖 Prompt for AI Agents
In `@plugins/kaizen/scripts/retrieve_guidelines.py` around lines 95 - 98, The
current logging prints the entire input_data JSON (via log(f" Full content:
{json.dumps(input_data, indent=2)}")), which may expose sensitive user content;
change this so only non-sensitive metadata is logged by default (keep log(f"
Keys: {list(input_data.keys())}") and remove or disable the full JSON dump). If
verbose debugging is needed, gate the full dump behind an opt-in environment
variable (e.g., GUIDELINES_VERBOSE_LOG) and only log json.dumps(input_data, ...)
when that var is true. Also ensure the plugin log file creation/rotation code
sets restrictive permissions (owner-read/write only, e.g., 0o600 via
open/os.chmod or secure os.open flags) for /tmp/guidelines-plugin.log so
persisted data isn’t world-readable. Target the calls to log and the code paths
that open/write /tmp/guidelines-plugin.log and the input_data usage.
There was a problem hiding this comment.
Good catch. We're only running this locally for now, so this isn't a problem.
There was a problem hiding this comment.
@vinodmut Thanks for the context! That makes sense for local development. Just a heads-up that if the plugin is eventually shared or deployed more broadly, it would be worth revisiting this logging to avoid any unintended exposure.
4834170 to
b6c8efb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/kaizen/README.md`:
- Around line 68-73: Add a language identifier to the fenced code block
containing the dialogue example (the block starting with "User: [Complete a
successful task]" and lines with User/Assistant prompts) so Markdownlint
recognizes it; change the opening fence from ``` to ```text (or another
appropriate language) to mark it as plain text, leaving the block contents
unchanged.
🧹 Nitpick comments (1)
plugins/kaizen/README.md (1)
50-62: Reduce repetitive “Manually invoke…” phrasing.Three consecutive sentences start the same way; consider rewording one for readability.
✍️ Example tweak
-Manually invoke to retrieve and display stored guidelines. +Use this skill to retrieve and display stored guidelines.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@SAVE_SKILL_DESIGN.md`:
- Line 251: Update the example output in SAVE_SKILL_DESIGN.md so the saved-skill
path matches the implemented/designated path (~/.claude/skills/) instead of the
hardcoded /Users/vinod/data/digitallabor/...; locate the "example output" block
that shows Location:
/Users/vinod/data/digitallabor/src/agents/kaizen/skills/{skill-name}/SKILL.md
and change it to Location: ~/.claude/skills/{skill-name}/SKILL.md to align with
the design decision and implementation.
- Line 295: The "Primary Location" entry currently lists the ambiguous path
`/skills/{skill-name}/SKILL.md`; update that entry to the home-prefixed
canonical path `~/.claude/skills/{skill-name}/SKILL.md` so it matches the
previously stated design decision using the home directory prefix, and replace
any other occurrences of the old path in SAVE_SKILL_DESIGN.md's "Primary
Location" references to ensure consistency.
🧹 Nitpick comments (1)
SAVE_SKILL_DESIGN.md (1)
365-368: Consider adding language identifiers to code blocks.The tool call example code blocks could benefit from language identifiers (e.g.,
json) for better syntax highlighting and clarity.♻️ Example enhancement
For instance, at lines 365-368:
-**Example**: -``` -mcp__filesystem__read_text_file({ +**Example**: +```json +mcp__filesystem__read_text_file({Apply similar changes to other tool call examples throughout the document.
Also applies to: 377-379, 389-393, 402-406
- Add save skill for capturing session workflows as reusable skills - Remove deprecated Stop hook from README - Fix duplicate step numbering in SAVE_SKILL_DESIGN.md - Remove user-specific paths from documentation and test data Co-Authored-By: Claude (aws/claude-opus-4-5) <noreply@anthropic.com>
5110e18 to
706bbf7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/kaizen/README.md`:
- Around line 67-73: The Quick Start example is inconsistent with the documented
invocation: the text specifies the fully qualified command `/kaizen:save` but
the interaction example uses `save`; update the example to use `/kaizen:save`
(or explicitly show both forms and clarify which is required) so the documented
skill name and the sample dialogue match (search for the example block that
contains "User: \"save\"" and change it to "User: \"/kaizen:save\"" or add a
clarifying note).
🧹 Nitpick comments (3)
SAVE_SKILL_DESIGN.md (1)
232-238: Consider adding a language specifier to the fenced code block.The directory structure code block lacks a language specifier. While not critical, adding one improves rendering consistency.
📝 Proposed fix
-``` +```text ~/.claude/skills/{skill-name}/ ├── SKILL.md └── scripts/ (if applicable) ├── script1.py └── script2.py ```plugins/kaizen/skills/learn/SKILL.md (1)
66-68: Consider adding a language specifier to the fenced code block.The code block at line 66 for "Method 2: From File" lacks a language specifier per the static analysis hint.
📝 Proposed fix
-``` +```bash cat guidelines.json | python3 ${CLAUDE_PLUGIN_ROOT}/scripts/save_guidelines.py</details> </blockquote></details> <details> <summary>plugins/kaizen/skills/save/SKILL.md (1)</summary><blockquote> `51-60`: **Consider adding language specifiers to unlabeled code blocks.** Several code blocks throughout the document lack language specifiers (e.g., lines 51-60, 79-83, 104-108). Adding `text` or a more specific language improves rendering and accessibility. This is a minor documentation hygiene issue. <details> <summary>📝 Example fix for one block</summary> ````diff -``` +```text User Goal: "Read a file and display its contents" Workflow Pattern: 1. Attempt to read file at expected location ...</details> Also applies to: 79-83, 104-108 </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| **Quick Start:** | ||
| ``` | ||
| User: [Complete a successful task] | ||
| User: "save" | ||
| Assistant: "What would you like to name this skill?" | ||
| User: "my-workflow-name" | ||
| ``` |
There was a problem hiding this comment.
Align Quick Start command with documented skill name.
The example uses save, but the text above specifies /kaizen:save. If the invocation must be fully qualified, this could confuse users. Consider making the example consistent.
✏️ Proposed doc tweak
-User: "save"
+User: "/kaizen:save"🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 71-71: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In `@plugins/kaizen/README.md` around lines 67 - 73, The Quick Start example is
inconsistent with the documented invocation: the text specifies the fully
qualified command `/kaizen:save` but the interaction example uses `save`; update
the example to use `/kaizen:save` (or explicitly show both forms and clarify
which is required) so the documented skill name and the sample dialogue match
(search for the example block that contains "User: \"save\"" and change it to
"User: \"/kaizen:save\"" or add a clarifying note).
- Add save skill for capturing session workflows as reusable skills - Remove deprecated Stop hook from README - Fix duplicate step numbering in SAVE_SKILL_DESIGN.md - Remove user-specific paths from documentation and test data
2 participants
For #36
Summary by CodeRabbit
New Features
Workflow Updates
Documentation
Maintenance
✏️ Tip: You can customize this high-level summary in your review settings.