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
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
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
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
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
