Repository navigation
fix(schedule-crons): no inline-fire on registration + per-cron queue gate - #1437
Merged
Merged
Conversation
…gate
Two bootstrap-storm mitigations bundled (M1 + M2 from the 2026-06-03
diagnostic on the 16:52Z 12-prompt avalanche).
**M1 — SKILL.md step 3 ambiguity.** The line "If prompt_skill is set,
invoke it as /skill-name" reads as "fire the skill immediately." When a
running session re-invoked /schedule-crons at 16:52Z, the prior session
inline-fired /morning-briefing + 5 cron-body prompts at one instant,
dropping 8 spurious prompts atop legit watcher TASK_FILE events. The
long-running session (9.5h old) drowned and ended at 16:54 without
processing two queued owner DMs.
Step 3 now explicitly says: pass `prompt: "/skill-name"` to CronCreate;
never invoke the skill or run the prompt body inline. Crons fire at
their scheduled cron expression, never on registration. Past-bug
reference embedded so future readers see the consequence.
**M2 — per-cron task-queue gate (defense in depth).** New helper
`scripts/cron-gate.sh <reason> <command...>` defers when any
`task-*.txt` is pending in `<workspace>/tasks/` (top-level only;
`archive/` and `processed/` subdirs don't count). Workspace resolves via
the M0 helper if present, else legacy `${SUTANDO_WORKSPACE:-...}` so
the script works on both main and the staging-workspace-revamp branch.
crons.example.json wraps every non-loop entry (morning-briefing,
daily-insight, pending-questions, sync-memory, obsidian-dream,
learned-skills-scan) with cron-gate. main-loop (/proactive-loop) is
exempt — it IS the owner-task handler; gating it would deadlock.
Tests: tests/cron-gate.test.sh — 7 cases (empty-queue runs / queued
defers / missing dir / archive+processed don't count / non-task files
don't count / usage error exits 2 / wrapped exit code propagates).
All 7 pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ub-daily Per Lucy's non-blocking review on #1437 (Discord #dev, 17:37:48Z): wrapping the daily crons (morning-briefing 06:57, daily-insight 06:50, obsidian-dream 03:37, learned-skills-scan 07:30) means a single deferral skips the function for 24h — a missed briefing day, missed dream pass, etc. — because there's no near-term retry tick. For sub-daily crons (`*/30` pending-questions and sync-memory), the next natural fire covers a skipped one within 30 min. M1's no-inline-fire rule already kills the avalanche on registration. M2's queue-gate is defense-in-depth and only earns its keep on high-frequency crons where the avalanche-risk × queue-collision probability is meaningful. Changes: - `crons.example.json`: unwrap `cron-gate.sh` from the 4 daily entries (morning-briefing, daily-insight, obsidian-dream, learned-skills-scan). Keep wrap on the 2 `*/30` entries (sync-memory, pending-questions). - `SKILL.md` "When to gate" decision rule: NEVER for main-loop, YES for sub-daily (`*/N`/hourly), NO for daily-or-less-frequent. Cites Lucy's catch so future cron additions know the rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
@cla-assistant check |
qingyun-wu
added a commit
that referenced
this pull request
Jun 3, 2026
Owner directive 2026-06-03: this file is the operational source of truth for v0.8; the contract-v0.8.md adjunct stays as design rationale. * Title → "Workspace Contract — v0.8" with a Version line beneath. * §1.1 final wording per owner: workspace is **always** at <repo>/workspace/ by default; specified via config file; per-clone overrides via sutando.config.local.json. No env override. * ⚠ callout listing 5 cwd-anchored features that degrade if override resolves outside repo root (CLAUDE.md auto-load, skills discovery, project-slug transcripts, hook scope, sync-workspace defaults). * §1.2 Layout adds relay/, skills/, state/auth/. * §1.4 Confidentiality expanded with "Mechanisms enforcing this:" — 9 bullets covering: gitignore blanket, pre-commit + CI guards, access tiers, TOFU + allowlist, sandboxed delegation, result-body markers, memory split, state/auth/ per-host scoping, channel-confidentiality routing rule. * §3 Migration with owner's 3-tier framing: auto (default) / ask-sutando / manual. * §7 FAQ — 5 Q&A: override-mindfully, why-not-env, how-to-migrate, weird-place-recovery, external-SSD-symlink. * §8 Shipping history table extended with #1431, #1432, #1437, and #1440 in flight. Diff stat: +189/-86 lines on top of the prior #1424 baseline.
qingyun-wu
added a commit
that referenced
this pull request
Jun 4, 2026
… warnings [DRAFT] (#1440) * fix(workspace-v0.8): strip $SUTANDO_WORKSPACE from resolver + colored warnings Step 1 of Path B (single-PR strip per docs/workspace-contract-v0.8.md target). The library-level resolver in src/sutando_config.{py,ts} no longer honors $SUTANDO_WORKSPACE — workspace resolves only via sutando.config.{json,local.json} or the {repoRoot}/workspace baked-in default. If $SUTANDO_WORKSPACE is set in the environment, the resolver fires a ONE-TIME bold-red ANSI warning pointing at scripts/sutando-migrate.sh for relocation of any data still living at the env-pointed path. Same for stale .env declarations. Both warnings: - Bold-red on TTY (operators notice + migrate); plain text on log captures (script / tee / journald / GitHub Actions) so escape codes don't pollute. NO_COLOR=1 honored per no-color.org convention. - Omit the env value to avoid the path-shaped-warning trap (2026-06-02 rogue-folder bug — bash tokenization on stderr re-capture). - Direct the operator at `bash scripts/sutando-migrate.sh --dry-run` + `--commit` for relocation. Next commits on this branch will add: - src/startup.sh + src/init.sh auto-migration on bootstrap when $SUTANDO_WORKSPACE is set (per follow-up directive — fragile to expect operators to manually run sutando-migrate). - scripts/sutando-migrate.sh --auto flag (non-interactive mode for bootstrap-triggered migration). - ~141 SUTANDO_WORKSPACE refs in tests migrated to use sutando.config.local.json fixtures. - Strip $SUTANDO_WORKSPACE fallback from init.sh, startup.sh, install-credential-proxy-launchd.sh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workspace-v0.8): bootstrap auto-migration + strip env fallbacks Step 2 of Path B: bootstrap-side env handling. **Auto-migration in src/startup.sh** (per owner directive 2026-06-03 18:31Z: "we need to automatically trigger a migration for the user"): After .env is sourced + validated, before init.sh runs: - If $SUTANDO_WORKSPACE is set with data at the env-pointed path, invoke `bash scripts/sutando-migrate.sh --commit` to relocate into the v0.8 default (<repo>/workspace/). - Sentinel `state/auth/migrated-from-env.txt` guards against re-running. - Bold-red TTY banner so the operator notices the one-shot relocation. - On migrate failure: abort startup (refuse to proceed with split state), point operator at `scripts/sutando-migrate.sh --dry-run` to diagnose. **Compress-in-place after migration** (per owner directive 2026-06-03 18:36Z: "compress original folder in place — (1) reserve data, (2) avoid old processes silently using it"): After successful migrate-commit: - `tar -czf <legacy>-pre-v0.8-<ts>.tar.gz` the original folder. - `rm -rf` the original directory. - Result: stale processes still pointing at the env-path now hit ENOENT instead of writing to a divergent directory; data is recoverable via `tar -xzf`. **Belt-and-suspenders**: `unset SUTANDO_WORKSPACE` after migration so child processes (init.sh, bridges, voice-agent, etc.) get v0.8 directly and the resolver's deprecation nag stops firing on every subprocess spawn. **Strip env fallbacks** from: - src/init.sh — was `elif [ -n "${SUTANDO_WORKSPACE:-}" ]`; now hard-fail if workspace_resolve.sh is unreachable. .env-drift nag updated to v0.8 framing (env no longer honored, delete the .env line + move to sutando.config.local.json). - src/workspace_resolve.sh — shared helper sourced by init.sh + several install-launchd scripts. Same hard-fail. - src/install-credential-proxy-launchd.sh — same. - src/startup.sh — workspace resolution falls back to $REPO/workspace (baked-in default) when the helper isn't present, no env. Init.sh .env-drift warning now uses bold-red ANSI on TTY (matches the resolver-side warnings in `529007d`); plain text in log captures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workspace-v0.8): SUTANDO_TEST_MODE escape hatch + test fixture updates CI was red after the resolver-strip commits (529007d, 6cf3e9d) because ~14 test files set `$SUTANDO_WORKSPACE` to redirect workspace to a per-test tmp dir — and v0.8 ignores the env. Path B test-fixture migration would be invasive (141 refs across 23+ files). Cleaner path: add a private, test-only escape hatch in the resolver that re-honors `$SUTANDO_WORKSPACE` silently when `SUTANDO_TEST_MODE=1` is also set. The end-user v0.8 contract is unchanged: production warns + ignores. Tests opt in explicitly. Changes: * src/sutando_config.{py,ts} — early-return when env+SUTANDO_TEST_MODE both set; warning + ignore otherwise (existing v0.8 behavior). * tests/_helpers/temp-workspace.ts — helper sets both env vars; cleanup() clears both so subsequent in-process tests can exercise the production code path. * tests/sutando-config.test.{py,ts} — invert the two env-precedence tests to assert v0.8 behavior (env ignored, local.json wins). * tests/workspace-default.test.py — module-level setUpModule sets SUTANDO_TEST_MODE=1 (migration test classes redirect via env); resolve()-canonical equality on macOS /private symlink paths; inverted test confirms production-path env-ignore. * tests/workspace-default-relative-env.test.py — per-test opt-in to SUTANDO_TEST_MODE. * tests/{outbox-log,single-instance,bridges-sending-orphan-recovery}.test.py — setUp opt-in to SUTANDO_TEST_MODE. * tests/{agent-state-endpoint,init-script,sutando-config-shell}.test.ts — spawn-env opt-in to SUTANDO_TEST_MODE. Local test run: all 17 prior failures clear. Single pre-existing flake unrelated to env-strip (`init.sh --preflight optional=2/8` asserts host env state; passes on clean CI). * docs(workspace-contract): mark v0.8 + add §1.4 mechanisms expansion Owner directive 2026-06-03: this file is the operational source of truth for v0.8; the contract-v0.8.md adjunct stays as design rationale. * Title → "Workspace Contract — v0.8" with a Version line beneath. * §1.1 final wording per owner: workspace is **always** at <repo>/workspace/ by default; specified via config file; per-clone overrides via sutando.config.local.json. No env override. * ⚠ callout listing 5 cwd-anchored features that degrade if override resolves outside repo root (CLAUDE.md auto-load, skills discovery, project-slug transcripts, hook scope, sync-workspace defaults). * §1.2 Layout adds relay/, skills/, state/auth/. * §1.4 Confidentiality expanded with "Mechanisms enforcing this:" — 9 bullets covering: gitignore blanket, pre-commit + CI guards, access tiers, TOFU + allowlist, sandboxed delegation, result-body markers, memory split, state/auth/ per-host scoping, channel-confidentiality routing rule. * §3 Migration with owner's 3-tier framing: auto (default) / ask-sutando / manual. * §7 FAQ — 5 Q&A: override-mindfully, why-not-env, how-to-migrate, weird-place-recovery, external-SSD-symlink. * §8 Shipping history table extended with #1431, #1432, #1437, and #1440 in flight. Diff stat: +189/-86 lines on top of the prior #1424 baseline. * fix(workspace-v0.8): opt task-bridge stop-at-delimiter test into SUTANDO_TEST_MODE Missed in 3f02c80 — this test uses dynamic `await import(...)` instead of the setupTempWorkspace helper because resolveWorkspace() captures SUTANDO_WORKSPACE at module-load time and ES `import` is hoisted. The escape-hatch opt-in needs to land alongside the SUTANDO_WORKSPACE set. Local: 5/5 pass. * fix(workspace-v0.8): opt remaining py tests into SUTANDO_TEST_MODE CI was still red after d005762 because `quota-burn-rate.test.py` loads `read-quota.py`, which exits 1 when `quota-state.json` is missing. The test seeds quota-state.json into its tmp workspace, but without SUTANDO_TEST_MODE the env was ignored and `read-quota.py` looked in `<repo>/workspace/state/` — empty on CI clean checkout, hence exit 1 × 11 tests. Locally those tests passed by accident — host has a real `<repo>/workspace/state/quota-state.json`. CI clean checkout has no workspace state, exposing the env-ignore as the cause. Also patched 7 other py tests that set SUTANDO_WORKSPACE but don't strictly depend on env-honor for correctness — adds the opt-in for consistency + defense against future CI flakes. Local: all 8 patched files pass. * fix(workspace-v0.8): PR #1440 review — auto-migration safety hardening Addresses 4 blockers Mini raised in #design 2026-06-04 02:09Z on the v0.8 env-strip PR. The auto-migration block in src/startup.sh ran sutando-migrate.sh --commit, then tar -czf'd the source dir, then rm -rf'd it — a sequence that could delete the active workspace if SUTANDO_WORKSPACE was symlink-equivalent to the resolved path, archive the wrong (post-migration) state, or rm -rf an unsafe path on a malformed env var. Fixed by adding 4 layered guards. ## Changes src/migration_safety_helpers.sh (new — 4 sourceable functions): - _realpath: cross-platform (GNU realpath / BSD readlink -f / python fallback) - _same_inode: stat -L -f %i (BSD) || -L -c %i (GNU); follows symlinks via -L - _is_unsafe_for_migration: deny-list — /, /usr|/etc|/var|/bin|/sbin|/System| /Library|/Applications, $HOME + top-level subdirs, $REPO + anywhere under it, /private/etc|/private/var (macOS symlink targets), paths with surviving '..'. - _color_warn: bold-red on TTY *only* when NO_COLOR is unset src/startup.sh (sources the helpers and restructures the migration block): - B1: realpath equality + _same_inode check before any migrate/compress/delete. Skip the entire block when env points at the resolved workspace. - B2: tar -czf BEFORE sutando-migrate.sh --commit. The tarball is now a true pre-migration recovery snapshot; if archive fails, abort before the destructive step. If migrate fails AFTER archive succeeds, the archive is preserved (caller can roll back). - B3: _is_unsafe_for_migration deny-list applied at TWO points — once up-front (refuse the whole block), again after successful migrate before rm -rf (catches state mutation between checks). Failed re-check preserves archive. - B4: _color_warn replaces the inline `[ -t 2 ]` check; honors NO_COLOR. - Sentinel now records the archive path for downstream tooling. src/init.sh: - B4: honor NO_COLOR in the .env-deprecation warning (parallel structure to startup.sh — same `[ -t 2 ] && [ -z NO_COLOR ]` gate). - Drop literal `$_env_val` and `$WORKSPACE` interpolations from the warning text (parity with Python's c58270d safety pass — embedded /-bearing values were the trigger for the caller-side `mkdir -p "$captured"` rogue-folder regression). src/sutando_config.ts (TS resolver): - B4: drop literal `${envVal}` from the SUTANDO_WORKSPACE-set warning at L283-294 and `${dotenvVal}` + `${resolved}` from the .env-drift warning at L312-325. Wording rephrased so it reads naturally without the path values. The existing NO_COLOR handling in _colorWarn (L188) already worked — only the path-leak side needed fixing. ## Verification - bash tests/migration-safety-helpers.test.sh: 24/24 pass (unit-tests the 4 guard functions: realpath against symlinks, same_inode follows symlinks, is_unsafe_for_migration covers /, /etc, /var, /System, $HOME and subdirs, $REPO and subdirs, non-existent, sneaky-symlink-to-$HOME, plus _color_warn TTY/NO_COLOR matrix). - npx tsx --test tests/sutando-config.prod-path.test.ts: 7/7 pass (env-set production path: NO_COLOR=1 + non-TTY emit zero ANSI; env warning + .env warning contain no literal path values; warning is one-shot per process; resolver returns config-driven path or {repoRoot}/workspace default). - tests/sutando-config.test.ts: 21/21 pass — no regression. - tests/sutando-config.test.py: 30/30 pass — no regression. - bash -n on edited shells: syntax clean. ## Known follow-up Mini also asked for a full startup-migration integration test (same-path, symlink path, empty legacy dir, migrate failure, archive contents, no-delete on unsafe paths). The helpers-level test + TS prod-path test cover the highest-risk units; a driver-level integration test that orchestrates the full migration block (mocking sutando-migrate.sh --commit + tar) is a separate follow-up — non-trivial setup, lower marginal value than the unit coverage shipped here. Tracking in PR body. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workspace-v0.8): coverage for PR #1440 safety helpers + prod path Two new test files, both passing locally, both pinned to the safety hardening shipped in the parent commit. ## tests/migration-safety-helpers.test.sh (24 assertions) Unit-tests src/migration_safety_helpers.sh — the 4 guard functions extracted from src/startup.sh in the parent commit: - _realpath against an existing file, a symlink, a non-existent path. - _same_inode: file == itself, symlink == target (follows via -L), different files != each other. - _is_unsafe_for_migration: positive coverage over /, /usr, /etc, /var, /System, $HOME, $HOME/{Documents,Desktop,Downloads}, $REPO, $REPO/src, non-existent paths, plus a sneaky `ln -s $HOME /tmp/...` that realpath resolves through; negative coverage on /tmp/<random>/* as safe. - _color_warn: non-TTY → plain; NO_COLOR=1 → plain even on TTY (via `script` for emulation); TTY + NO_COLOR unset → ANSI. Test fixture uses /tmp/<pid>/ explicitly (not `mktemp -d` which on macOS lands under /var/folders/ → /private/var/... which the deny-list correctly catches as a /var subpath). ## tests/sutando-config.prod-path.test.ts (7 assertions) Mini called out that the existing TS suite leans on SUTANDO_TEST_MODE=1, leaving the production path under-covered. New file exercises the env-set + no-TEST_MODE branch directly: - NO_COLOR=1 → deprecation stderr has zero ANSI escapes. - non-TTY stderr → zero ANSI escapes regardless of NO_COLOR. - env-set warning text does NOT contain the literal SUTANDO_WORKSPACE value (B4 path-leak parity with c58270d on Python side). - .env-drift warning text does NOT contain the literal .env value. - Deprecation warning fires exactly once across 3 resolveWorkspace calls (one-shot guard via _legacyEnvWarnPrinted). - env set + sutando.config.local.json present → resolver returns the config-driven path (config wins). - env set + no config → resolver returns the baked-in {repoRoot}/workspace. Stderr capture uses a process.stderr.write monkey-patch with a restorer in the afterEach hook so tests don't pollute each other's captures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workspace-v0.8): PR #1440 v1 review — Mini's sub-blockers + Python prod-path twin Addresses Mini's #design follow-up review (2026-06-04 02:30Z) on the v1 of the safety hardening pass. Three sub-blockers + a Python coverage gap. ## B3 — deny-list expansion (Mini: "still a blocker") `_is_unsafe_for_migration` now denies the additional cases Mini called out, where the prior pass left descendant paths or common dotfile targets uncovered: - `/tmp` + `/private/tmp` exact (subdirs like mktemp targets remain safe — the prior pass only denied the macOS-resolved /private/etc + /private/var) - `$HOME/Documents/*`, `$HOME/Desktop/*`, `$HOME/Downloads/*` descendants (prior: only the exact dirs; `~/Documents/some-repo` was fair game) - `$HOME/.sutando` exact + subpaths, EXCEPT `$HOME/.sutando/workspace` (and subpaths) — the workspace subpath IS the known legacy default we WANT to be able to migrate; the allow-exception is checked first via a case statement so the subsequent .sutando deny doesn't shadow it - `$HOME/.claude` exact + subpaths (Claude Code's install location) - `$HOME/.config` exact + subpaths (XDG install location) ## B1 — device:inode (Mini: "edge: same inode across unrelated fs") `_same_inode` now uses `stat -L -f '%d:%i'` (BSD) / `-c '%d:%i'` (GNU) instead of `%i` alone. Comparing inode numbers without the device id false-positives when two filesystems happen to reuse the same inode number — `device:inode` is what makes the pair globally unique. ## Test coverage expansion `tests/migration-safety-helpers.test.sh` gains 14 new assertions: - `/tmp` exact deny + `/tmp/<subdir>` safe (using mkdir'd test fixture) - `/private/tmp` exact deny - `$HOME/Documents|Desktop|Downloads/foo` descendant deny (3) - `$HOME/.sutando` exact deny + non-workspace subpath deny + workspace subpath ALLOW (incl. nested) (4) — uses a fake $HOME under TEST_DIR with mkdir'd dir tree to test the allow-exception without touching the user's real ~/.sutando/ tree. Realpath's the fake-home so the $HOME case pattern matches the realpath'd /private/tmp/... paths (test-fixture-side workaround; real systems with $HOME=/Users/... don't hit this aliasing). - `$HOME/.claude` + subpath deny (2) - `$HOME/.config` + subpath deny (2) Total: 38/38 pass (was 24/24 on v1). ## tests/sutando-config.prod-path.test.py (NEW — Python twin of v1 TS test) Mini: "Python prod-path coverage is still thin." Mirrors the TS prod- path test added in df70545; 7 assertions covering the env-set, no- TEST_MODE branch: - NO_COLOR=1 → zero ANSI in deprecation stderr - non-TTY stderr (StringIO under unittest) → zero ANSI - env-set warning has no literal env-var value (c58270d regression guard) - .env-drift warning has no literal dotenv value - warning is one-shot across 3 resolve_workspace() calls - env set + sutando.config.local.json → resolver returns config-driven path - env set + no config → resolver returns {repoRoot}/workspace (realpath'd to handle macOS /var → /private/var) Uses contextmanager-style capture_stderr() with restorer in tearDown so tests don't pollute each other's captures. Mirrors the TS test's beforeEach/afterEach env-snapshot pattern. ## Verification - bash tests/migration-safety-helpers.test.sh: 38/38 pass (was 24/24) - python3 tests/sutando-config.prod-path.test.py: 7/7 pass (new) - npx tsx --test tests/sutando-config.prod-path.test.ts: 7/7 pass (no regression) - npx tsx --test tests/sutando-config.test.ts: 21/21 pass (no regression) - python3 tests/sutando-config.test.py: 30/30 pass (no regression) ## Known follow-up (carried from v1) - Full startup-migration integration test (Mini's "Test gaps" list — driver- level mock-migrate + tar end-to-end). Still a separate PR; the helpers unit + TS+Python prod-path tests cover the highest-risk units. - Test-fixture migration (~141 SUTANDO_WORKSPACE refs across 23 tests). The SUTANDO_TEST_MODE escape hatch is transitional; full migration to sutando.config.local.json fixtures is tracked under Path B.1b. ## Symlink legacy dir as archive edge (Mini flagged) If SUTANDO_WORKSPACE itself is a symlink to a real dir, `tar -czf` handles this case by archiving the symlink target's content (BSD + GNU tar both follow per `-h` default off, but `tar -C "$(dirname "$_ws_legacy")" "$(basename "$_ws_legacy")"` operates on the entry as named — symlink entry is preserved in the archive with `-h` off, archive-with-followed-content with `-h` on). The B1 realpath guard already converts the symlink target to its real path for the same-path / inode comparisons, so a symlink→resolved-workspace case is caught up-front and the destructive block is skipped. A broken symlink fails the `[ -d "$_ws_legacy" ]` precondition and is also skipped. Documented as a known-limitation note in the PR body. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workspace-v0.8): add migration_safety_helpers + shell tests to lint allowlist CI failure on `0c496ef` — `refuse new direct workspace-path resolution` flagged legitimate `$HOME/.sutando/workspace` references in: - `src/migration_safety_helpers.sh` (the deny-list allow-exception logic — the very point of the helper is to recognize the legacy default as an intended migration source) - `tests/migration-safety-helpers.test.sh` (the test asserting that the allow-exception works) The lint regex correctly catches new code that DOES workspace resolution inline (the anti-pattern it was designed to block), but here the references are in: (1) a NEW safety-helper file whose purpose IS to know about the legacy default — the literal path appears as a CASE PATTERN in the deny-list, not as resolver code; and (2) a SHELL test (the existing tests/ allowlist matched only .py and .ts; generalizing to also include .sh covers any future shell-test that needs to assert against literal paths). Two-part fix: - Add `src/migration_safety_helpers\.sh` to the ALLOWED files regex. - Extend `tests/[^/]+\.(test\.)?(py|ts)$` → `(py|ts|sh)$` so shell tests in tests/ get the same exemption Python and TS tests already have. Verified locally: `BASE_REF=origin/staging-workspace-revamp bash scripts/lint-workspace-resolution.sh --diff` → `✓ no new workspace-resolution anti-patterns in this diff.` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 5, 2026
Merged
john-the-dev
added a commit
that referenced
this pull request
Aug 27, 2026
…digest (#3393) * chore(daily-insight): remove the feature — nothing invoked it and its output duplicated the digest Owner decision (2026-08-25). Removed rather than repaired, on three measurements: no scheduler no daily-insight cron on the live host, and its only caller (morning-briefing.py:780) has no cron entry either last output 2026-08-12 — 13 days, unnoticed what it emitted "landed 2 of 7 commits from the last 24h (tests/, src/)" i.e. commit statistics the PR digest already carries Deletes src/daily-insight.py and its five dedicated suites; excises get_daily_insight(), its call site and the `insight` parameter from morning-briefing; drops the job from crons.example.json and the step from the morning-briefing skill; updates two docstrings that referenced it. Deliberately NOT changed: - schedule-crons/SKILL.md:208 records a dated 2026-06-03 incident (PR #1437) that names daily-insight among the daily crons of that day. Editing a historical record to remove a since-deleted job would falsify it. - health-check-daily-punctuality.test.py uses "daily-insight" as an arbitrary fixture job name for _interpret_daily_punctuality. That test is about the probe, not the feature, and renaming its fixtures would bundle an unrelated refactor into a removal. Evidence: 13 affected suites (morning-briefing-*, *punctuality*, telemetry) run on /usr/bin/python3 — the 3.9 CI uses — pass=13 failed=0 at HEAD. Stand: Echo Act IV Mini * fix(remove-daily-insight): surgical JSON edit + regenerate src-map Two CI failures, both mine and both mechanical: `refuse docs/src-map.md stale vs src/` — deleting src/daily-insight.py without regenerating the map. Fixed by running scripts/gen-src-map.py. `hardcoded-paths` — I had rewritten crons.example.json with json.dumps, which reformatted the WHOLE file and pulled a pre-existing `~/.sutando/memory-sync/` literal (line 20, the sync-workspace prompt, not my content) into my diff. The scanner reads the diff, so a reformat makes someone else's line mine to answer for. Restored the file and removed only the daily-insight object, leaving every other byte untouched: the diff drops from 15 changed lines to 7, and the memory-sync line is no longer in it. Not adding a REVIEW.md allow for that literal — it is a real pre-existing violation in a line this PR has no business touching, and an allow would hide it from whoever does fix it. Stand: Echo Act IV Mini * docs(morning-briefing): the delivery section still gathered a removed feature john-the-dev's review: the numbered list renumbered correctly but the delivery section below it still referenced insight — "steps 1-4" when three remain, an `Insight:` line in the echo block, and "GWS/insight augmentation" in the manual-invocation note. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: live docs still named the removed daily-insight component qingyun-wu's P2 at e9d601f: three live claims survived the removal and pointed config help and telemetry architecture at deleted production code. docs/sutando-config.schema.json `stand` "Read by ... src/daily-insight.py" hooks/skill-usage-telemetry.py "the two hand-instrumented scripts (…, daily-insight)" TELEMETRY.md:63 same claim, second copy A fourth the review did not list, same class: TELEMETRY.md's `feature_used` row used `daily_insight` as its live example. Re-pointed at `skill:<name>`, which is what the hook actually emits. Test fixtures under tests/ keep the string deliberately — they exercise the punctuality probe's handling of a named job and are not claims about the tree. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The prior Sutando session (
eeb8b348-..., 9.5h-old) drowned at 16:52Z and ended at 16:54Z without processing two queued owner DMs (PR-#1428 review asks). JSONL forensics showed a 12-prompt avalanche in 22 seconds:5 of the 6 "Run ..." prompts shouldn't have matched the 16:52 schedule. Root cause was in
skills/schedule-crons/SKILL.mdstep 3:This reads as "fire the skill immediately." When the prior session re-invoked
/schedule-cronsat 16:52, it inline-fired/morning-briefingplus the prompt-strings — even though their schedule didn't match. 8 spurious prompts on top of 2 legit TASK_FILE events drowned the session.Fix — two layers
M1 — Tighten step 3 (the bug)
Explicit "do NOT invoke inline" + show the canonical
CronCreate(cron, prompt: "/skill-name")form. Past-bug reference embedded so future readers see the consequence:3. For each job in the config: - Skip if a job with matching prompt/name already exists - - If `prompt_skill` is set, invoke it as `/skill-name` - - Call CronCreate with the cron expression and prompt + - Call `CronCreate` with the cron expression and prompt: + - If `prompt_skill` is set, pass `prompt: "/skill-name"` ... + - Otherwise pass `prompt: <prompt-string-from-config>`. + - **Do NOT invoke the skill or run the prompt body inline ...** + (Past bug 2026-06-03T16:52Z: ...)M2 — Per-cron task-queue gate (defense in depth)
New helper
scripts/cron-gate.sh <reason> <command...>defers when anytask-*.txtis pending in<workspace>/tasks/(top-level only;archive/andprocessed/subdirs don't count). Workspace resolves via the M0 helper if present, else legacy${SUTANDO_WORKSPACE:-...}so the script works on both main and the staging-workspace-revamp branch.crons.example.jsonwraps every non-loop entry.main-loop(/proactive-loop) is exempt — it IS the owner-task handler; gating would deadlock.Even if M1 doesn't fully prevent inline-fires under some future ambiguity, M2 ensures owner work always wins by deferring cron noise on a non-empty queue.
Tests
tests/cron-gate.test.sh— 7 cases, all passing:Cross-PR linkage
/schedule-cronsskips entries whose[ -f <path> ]-guarded prompt body references a non-existent script). Smaller separate PR.cron-gate.shworks with both the M0 helper and the legacy fallback.Test plan
bash tests/cron-gate.test.sh— all 7 cases pass locallypython3 -c "import json; json.load(open('skills/schedule-crons/crons.example.json'))"— JSON validcron-gate: owner tasks queued — deferring <reason>and owner tasks process firstVerification (added 2026-06-03 after #design discussion)
Per @susanliu_'s rigor critique that "did anyone actually reproduce the before-state vulnerability?", two independent verifications landed in #design:
Lucy's isolated-naive-agent repro (proves M1 sufficiency)
Lucy ran two isolated naive agents, same step-3 text, only variable = OLD vs NEW wording, with NO real CronCreate/harness and clock pinned to 14:00 (no schedule matches). Outcome:
invoke it as /skill-name): agent inline-ran/morning-briefingimmediately — verbatim "Ran the skill immediately (not scheduled, but as required by the procedure)" — then scheduled.CronCreate'd both, "not invoked right now."Conclusion: the SKILL.md ambiguity is sufficient to cause inline-fire in a naive agent, independent of harness behavior. M1 closes this vector.
My deeper JSONL forensic (confirms a second, harness-side mechanism on top)
Looked one level into the prior session's JSONL: each 16:52:0X prompt arrives as
type="queue-operation"operation="enqueue"from the harness side, NOT as an assistant message emitting tool/text inline. Right before the visible avalanche, 10 millisecond-clustered queue-operations at 16:51:58.8 — consistent with the harness draining a backlog of pending cron fires into the session's user-prompt queue on resume from compact/idle.So both mechanisms can be live in the 16:52 incident:
Where this leaves the PR
cron-gate.sh) — even more load-bearing than originally framed: the harness backlog-drain is something M1 cannot prevent, butcron-gate.shdefers individual cron prompts when owner work is queued, which is the right shape to protect against that resume-spike.skills/scope; not addressed in this PR.Thanks @susanliu_ for the verification push — caught a single-cause framing in the original PR description.