mirror of
https://github.com/alirezarezvani/claude-skills.git
synced 2026-10-08 03:07:51 +00:00
9 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Claude
|
7eb198ff42
|
fix(engineering): require --yes for schedule + close mkdir/chmod race (round-10)
A tenth review pass, after confirming all nine prior rounds of fixes hold up under independent re-reading, found two more low-severity gaps and offered to accept a follow-up -- fixed both now for consistency with how every prior round's findings were handled: 1. schedule had no confirmation gate at the CLI layer. The "confirm with the user before schedule" safeguard (deviation #15) lived only in commands/skillopt-sleep.md's agent-facing instructions -- cmd_schedule() called scheduler.schedule() directly and installed a real crontab entry immediately. Fine for the documented Claude Code agent workflow (which confirms in chat first), but anyone invoking `python -m skillopt_sleep schedule` directly bypassed it entirely. Fixed: schedule now requires --yes; an interactive terminal without it gets a [y/N] prompt, a non-interactive one refuses outright (exit 2) pointing at --yes. commands/skillopt-sleep.md updated so the driving agent passes --yes once it has confirmed with the user in chat -- that's what --yes records, not a redundant re-prompt that would hang forever with no TTY inside a non-interactive Bash tool call. 2. mkdir-then-chmod wasn't atomic in write_staging()/SleepState.save(), leaving a brief window where a freshly-created sensitive directory sat at the process's default umask. Fixed: the os.makedirs() calls creating the state dir, staging leaf dir, and backup dir now pass mode=0o700 directly, on top of (not instead of) the existing post-creation chmod calls, which still matter for intermediate parent dirs and pre-existing directories that mode= doesn't cover. The equivalent race for individual files was judged a larger rewrite (every open() call site would need os.open() with an explicit mode) than this specific low-severity finding warranted -- documented as a known, narrower residual gap rather than silently claimed as fully closed. Verified: non-interactive schedule without --yes refuses with exit 2, with --yes it proceeds to the same scheduler.schedule() call as before; a synthetic run confirms state dir/state.json/staging leaf still land at 0700/0600/0700 after the mode= change. Added as README deviations #22-23 and reconciled the count across all three documents to 23 (6 cosmetic, 17 safety/hardening) across ten review rounds -- cross-checked with grep. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |
||
|
Claude
|
a227b06e6f
|
fix(engineering): anchor scheduler.py's project marker match (round-9 bug)
A ninth review pass found scheduler.py's schedule()/unschedule() both
located "this project's" managed cron line via marker not in ln, a
bare substring test, not an exact-match or delimiter-anchored check.
Failure scenario: two projects scheduled where one path is a literal
prefix of the other (e.g. /home/user/app and /home/user/app-v2) --
"# project=/home/user/app" is itself a substring of
"# project=/home/user/app-v2"'s line. Running schedule() or
unschedule() for /home/user/app would silently drop app-v2's cron
entry too, with no error or warning.
harvest.py's _project_matches() (added in this same PR) already gets
this right via delimiter-anchored comparison; scheduler.py's marker
matching didn't follow the same discipline.
Fixed: added _line_matches_project(), anchored on
ln.rstrip().endswith(marker) since the marker is always the last token
of a generated line -- used at both call sites.
Also fixed the related minor nit: install-cron.sh's printed --backend
value was unquoted next to otherwise-quoted ${RUNNER}/${PROJECT} in
its heredoc (low risk since that script only prints a line for the
user to copy, never executes anything itself, but inconsistent with
the quoting discipline everywhere else).
Verified two ways: a standalone reproduction confirmed the bug before
the fix and its absence after, and a full schedule()/unschedule()
round-trip through the actual public API (crontab -l/crontab - swapped
for an in-memory fake) confirmed scheduling both /home/user/app and
/home/user/app-v2, then unscheduling only app, correctly leaves
app-v2's line intact.
Added as README deviations #20-21 and reconciled the count across all
three documents to 21 (6 cosmetic, 15 safety/hardening) across nine
review rounds -- cross-checked with grep.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS
|
||
|
Claude
|
4d68c542f2
|
fix(engineering): close CLI-output redaction gap (round-8 HIGH finding)
An eighth review pass found that seven rounds of redaction fixes were all file-level (write_staging(), diagnostics.json, state.json's archive) but __main__.py's cmd_run() reads the same in-memory Report object and prints EditRecord.content directly to the console, and _report_payload() serializes it unredacted for --json -- write_staging()'s redaction runs on a copy (report.to_dict()) used only for the on-disk JSON, it never touches report.edits itself. Concretely: scheduler.py's cron entry redirects run's stdout/stderr straight into <project>/.skillopt-sleep/cron.log -- a secret that leaked into a proposed edit's content would land there in plaintext on every scheduled night, in a file that (unlike state.json/staged files) also had no chmod protection. Fixed: - _report_payload() and cmd_run()'s plain-text edit printing now run through redact_secrets(), gated on the same redact_secrets config flag as everywhere else. - cmd_harvest()'s debug output (--json, --output <file>, and the plain-text loop) gets the same treatment -- it prints raw mined TaskRecord.intent text so a human can review it before setting "reviewed": true on a --tasks-file, and redaction only strips secret-shaped substrings, so it doesn't reduce what's reviewable while closing the same leak path. - scheduler.py's generated cron line now chmod 700s the .skillopt-sleep log dir and chmod 600s cron.log itself (best-effort, 2>/dev/null) before each run appends to it -- that file was never covered by the state/staging chmod pass in an earlier round. Verified: a synthetic secret seeded into a task's intent no longer appears in cmd_run's --json payload, plain-text edit output, or cmd_harvest's redacted payload; executing the actual generated cron line end-to-end (not just inspecting the string) produces a 0700 log dir and 0600 log file on disk. Added as README deviation #19 and reconciled the count across all three documents to 19 (5 cosmetic, 14 safety/hardening) across eight review rounds -- cross-checked with grep. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |
||
|
Claude
|
e64c1cdd44
|
fix(engineering): relabel misleading redaction placeholder (round-5 review)
A fifth review pass found staging.py's sk-[A-Za-z0-9_-]{10,} secret
pattern -- which matches OpenAI, Anthropic (sk-ant-...), and other
vendors sharing the sk- prefix convention -- was labeled
[REDACTED_OPENAI_KEY] regardless of which vendor's key shape it
actually matched. Redaction itself was unaffected (the text was
scrubbed either way), just a misleading placeholder if a user reads it
literally. Relabeled to [REDACTED_API_KEY].
Added as README deviation #17 (cosmetic) and reconciled the count
across all three documents (README.md's numbered list, plugin.json,
CLAUDE.md) to 17 (4 cosmetic, 13 safety/hardening) across five review
rounds -- cross-checked with grep, not just eyeballed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS
|
||
|
Claude
|
e729f157ee
|
fix(engineering): close round-4 review findings on vendored skillopt-sleep
A fourth automated review pass on PR #907 found the deviation count had drifted out of sync across the three places that document it, plus two more real gaps in the vendored plugin: 1. Deviation count inconsistency: plugin.json and README.md both said 13 (after round 3), but CLAUDE.md's v2.11.2 section said "8 deviations" with an itemized list that didn't map onto the real 13-item README list -- it named a "dead cross-reference to a non-vendored design doc" as a cosmetic item that was never actually added as a numbered README deviation (it was fixed in round 3's commit but never itemized). Fixed: added it as README deviation #14, updated plugin.json's derivation_note with a note that README.md's numbered list is the single source of truth if any summary disagrees again, and rewrote CLAUDE.md's bullet to match. 2. commands/skillopt-sleep.md's action table listed `schedule` as an ordinary action alongside safe previews (`status`/`dry-run`/`run`), while its own "Safety reminders" section separately said to point users at the print-only install-cron.sh instead -- two uncoordinated stories about the same action. scheduler.schedule() writes directly to the user's real crontab the moment it runs, with no confirmation step. Fixed (README deviation #15): "Steps to follow" now has an explicit step 1 telling the agent to confirm with the user before running `schedule`; "Safety reminders" no longer contradicts the action table. 3. state.json (the cross-night task archive) and .skillopt-sleep/staging/<ts>/'s proposal/report/diagnostics files contain real harvested session content in plaintext, created via plain os.makedirs/open(...,"w") -- world-readable-by-default on a typical multi-user box. Fixed (README deviation #16): state.py and staging.py now chmod every directory they create to 0700 and every file they write to 0600 (best-effort). Live CLAUDE.md/SKILL.md files are intentionally left alone -- those are the user's own, often-committed files, not new output this plugin introduces. All three documents (README.md's numbered list, plugin.json's derivation_note, CLAUDE.md's v2.11.2 section) now agree on 16 deviations (3 cosmetic, 13 safety/hardening) -- verified by grep. Verified: py_compile clean, mock-backend dry-run still exits 0, a synthetic test confirms state dir/state.json/staging dir/staging files land at 0700/0600/0700/0600 respectively after this fix (previously default umask permissions), all 4 repo CI gates pass locally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |
||
|
Claude
|
32446887e6
|
fix(engineering): close round-3 review findings on vendored skillopt-sleep
A third automated review pass on PR #907 found 2 HIGH and 5 further gaps in the vendored skillopt-sleep plugin, all in the same family as rounds 1-2 (a safety claim in the docs the code didn't fully back up): HIGH: 1. state.py's add_to_archive() persisted raw TaskRecord content (intent/context_excerpt/attempted_solution -- real harvested prompt/response text) to ~/.skillopt-sleep/state.json indefinitely, entirely outside the staging dir a user is ever told to review. Fixed: cycle.py now redacts each task dict before archiving, using the same redact_enabled flag as everything else. 2. report.md / report.json were never redacted, despite being the two files a human is told to read FIRST (the SKILL.md's own workflow: "show the user the exact proposed edits"). EditRecord.content/ .rationale come from the optimizer's reflect() output over real failing task responses. Fixed: write_staging() now redacts the rendered report_md string and report.to_dict() before writing. MEDIUM/LOW: 3. replay_mode: "fresh" (worktree replay) was declared in config but never implemented anywhere -- only fed a cosmetic report label. Implementing real worktree isolation was judged too invasive for a vendored copy; instead cycle.py now warns loudly when it's set to anything but "mock" rather than silently implying isolation that isn't happening. 4. backend.py shipped an AzureOpenAIBackend/AzureResponsesBackend pair with 5 internal-looking Azure endpoint hostnames and a hardcoded Managed Identity client ID, commented as sourced from "the intern's avail_api.md" -- reads like leaked internal Microsoft dev infra. Already unreachable from this plugin's documented mock/claude/ codex/copilot --backend choices and requires deps this repo doesn't vendor. Removed entirely (classes, constants, get_backend()/ build_backend() dispatch branches, the now-unused azure_endpoint param); get_backend("azure") now safely falls back to MockBackend. 5. attempt_with_tools() (all three CLI backends) used a task's tool name both as a shim filename and interpolated unescaped into the shim's generated shell body. Tool names originate from a hand-authored --tasks-file's judge.checks[].arg, never validated -- not reachable via harvest/mine today, but a documented user-facing input. Fixed: a shared _sanitize_tool_names() helper filters to a safe-identifier allowlist before any name is used as a filename or shell text. 6. SKILL.md pointed to a design doc path (docs/superpowers/specs/...) that was deliberately not vendored. Fixed: points to the real upstream guide URL instead, with a note on why the local path is absent. 7. CLAUDE.md's "Current Scope" line claimed v2.11.2 but no dated version section existed for it. Added one matching the repo's established per-release convention. All 13 deviations (2 cosmetic, 11 safety/hardening) cross-documented in plugin.json's derivation_note and README.md's "Deviations from upstream" + "Safety model" + "What was and wasn't vendored" sections. Verified: py_compile clean, mock-backend dry-run still exits 0, all 4 repo CI gates pass, and a synthetic test with a real API-key-shaped secret confirms it appears in NONE of state.json's task archive, report.md, or report.json after this fix (all three fired positive signal for the leak before it). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |
||
|
Claude
|
316982fa9f
|
fix(engineering): address round-2 review findings on vendored skillopt-sleep
A second automated review pass on PR #907 confirmed the prior 3 safety patches and found 3 more small gaps: 1. plugin.json's attribution.derivation_note still asserted "no logic modified" after the round-1 patches changed staging.py/scheduler.py/ cycle.py/backend.py. Updated to match README.md's deviation log. 2. redact_secrets was declared in config.py's DEFAULTS but never read anywhere -- write_staging() redacted unconditionally regardless of the flag (safe direction, but a dead config knob). Wired cfg.get("redact_secrets", True) through write_staging() and the diagnostics.json fields; disabling it is honored but never silently -- fires a loud report note. 3. scheduler.py's _runner_cmd shlex.quote()-d project/logdir/log/repo root (round-1 fix) but left the `extra` flags parameter raw. Not exploitable today (it's only ever a hardcoded literal), but closes the same class of gap defensively via shlex.split + per-token shlex.quote so a future multi-flag `extra` can't reopen it. Also applied the reviewer's non-blocking hardening suggestion: adopt() now re-runs redact_secrets() on staged content before writing to the live path (read+redact+write instead of a raw shutil.copy2), covering the case where a staged proposal is hand-edited between `stage` and `adopt` -- exactly the workflow staging exists to allow. All 6 deviations now cross-documented in plugin.json's derivation_note and README.md's "Deviations from upstream" + "Safety model" sections so re-vendoring can't silently drop them. Verified: py_compile clean, mock-backend dry-run still exits 0, synthetic tests confirm both the empty/populated extra-quoting paths and the redact_secrets on/off report-note behavior, all four repo CI gates (smoke_scripts, check_plugin_json, check_paths, derive_counters) pass locally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |
||
|
Claude
|
186c0f6d11
|
fix(engineering): close 3 safety gaps found by PR review in vendored skillopt-sleep
Automated review on PR #907 read the actual module code (not just the surface docs) and found the vendored plugin's own safety claims didn't fully match its behavior. Patches applied directly to our vendored copy (documented as deviations in the plugin README for re-vendor): 1. staging.py: redact_secrets() was applied to diagnostics.json but not to proposed_SKILL.md/proposed_CLAUDE.md -- the exact files adopt() copies over the live CLAUDE.md/SKILL.md (with --auto-adopt, with no human in the loop). A secret pasted into a real debugging session could have landed in live memory unredacted. Now redacted before write_staging() persists either file. 2. scheduler.py: the generated crontab line interpolated an arbitrary project path via unescaped f-string into a command cron runs through sh -c on every fire. A path containing shell metacharacters could break out of the quoting. Now shlex.quote()-d. 3. cycle.py: max_tokens_per_night was declared in config.py's DEFAULTS and budget.py already had a Budget/plan_depth heuristic built for it, but nothing in the production run_sleep_cycle() path ever read it -- a real-backend night had no actual token ceiling. Now a Budget starts right after backend construction (harvest/mine spend counts too), sizes dream_rollouts down via plan_depth() when remaining budget is tight, and the report notes when it caps rollouts or the budget is exhausted -- no silent truncation. This caps rollout depth per task, not a hard mid-call abort; documented as a residual limitation in the README. Also dropped a leftover hardcoded nvm path in backend.py's resolve_codex_path() (the generic scan a few lines below already covers it) and added a one-line acknowledgment to CLAUDE.md's Anti-Patterns list that this plugin's non-mock backends are a documented, opt-in exception to "no LLM calls in scripts" -- not precedent for adding LLM calls to analysis/reference skills. Verified: py_compile clean, mock-backend dry-run still exits 0, synthetic test confirms dream_rollouts capping actually engages under a tight budget and is a no-op under the default budget, all repo CI gates (smoke_scripts, check_plugin_json, check_paths, derive_counters) still pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |
||
|
Claude
|
cf6ca763ec
|
feat(engineering): vendor SkillOpt-Sleep from microsoft/SkillOpt
Verbatim copy of the stdlib-only skillopt_sleep engine + Claude Code plugin surface (skills/hooks/commands/scripts) into engineering/skillopt-sleep/. Gives a local agent a nightly gated self-improvement cycle: read-only harvest of past Claude Code session transcripts -> mine recurring tasks -> offline replay -> held-out-gated CLAUDE.md/SKILL.md edits -> staged for explicit /skillopt-sleep adopt. Nothing live changes without that explicit step. The heavier skillopt training package (needs numpy/openai/azure-* + hand-labeled benchmarks per task) was deliberately not vendored, since it optimizes one narrow scoreable task at a time and doesn't fit this repo's broad domain-expertise skills or no-ML-in-scripts convention. Attribution preserved in plugin.json + LICENSE + README.md (MIT, Microsoft Corporation / Yifan Yang), following the same verbatim-vendor pattern already used for loop-library/. Registered as its own marketplace plugin; headline counters in README.md/CLAUDE.md/ marketplace.json trued up via scripts/derive_counters.py --check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX374i2YGrjNV4Yi3AmaKS |