Repository navigation
fix(codex): seed the per-host crons.json on the Codex launch path - #5169
john-the-dev wants to merge 9 commits into
Conversation
On a fresh install with Codex as the core nothing created <workspace>/hosts/<host>/crons.json: init.sh seeds only the legacy repo-level file, and the per-host seed lives in /schedule-crons step 1, a Claude-session skill. The Codex launcher installs from the per-host file but never seeded it, so the proactive main-loop was never scheduled. Move the seed policy into one helper, skills/schedule-crons/scripts/ seed_crons.py (interim <ws>/crons/<host>.json > legacy skill crons.json > crons.example.json; never overwrites; atomic no-clobber publish via link). /schedule-crons step 1 now calls it instead of an inline recipe, and the Codex launcher calls it before the schedule reconcilers. Refs #5157 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
john-the-dev
left a comment
There was a problem hiding this comment.
Recommendation: changes needed before approval. Verified against current head dbfbd361687d017accd910906d79f7bd246839b5.
Blocking composition with desktop #741
src/agent/codex/cli/start-cli.sh:439 now seeds the full example before reconciliation, including the 06:57 morning briefing. I ran the actual seeder against a fresh temporary workspace, then desktop #741's production morningCatchupDue and ensureMorningCatchupTask({force:true}) at desktop commit 9e314a501b662e8c76b7051e4bf91304847a106b, using new Date(2026,9,6,13,0,0):
Before seed:
{"verdict":{"due":false,"parseable":true,"reason":"no morning-briefing scheduled"},"queued":false}
After seed:
[supervisor] morning catch-up queued (task-wake-briefing-20261006: scheduled 6:57 passed)
{"verdict":{"due":true,"parseable":true,"reason":"scheduled 6:57 passed"},"queued":true}
The newly seeded schedule therefore bypasses #741's “nothing scheduled” protection. Please define first-install catch-up behavior and add a combined regression so restoring main-loop does not restore the onboarding wake task. This is a function-level reproduction, not proof of first-DM ordering: desktop launches the core asynchronously, and the initial race remains untested. A later wake check can also observe the seeded schedule.
Worst-case disruption: missing canonical schedules now activate all example entries, including the placeholder example-digest, rather than only the proactive loop. Please explicitly settle the fresh-install default set (or narrow it) and distinguish first creation from resuming an established schedule. Existing canonical files are preserved by the seeder.
Independently run evidence
New launcher tests, using SUTANDO_TEST_REPO to select production sources:
Parent 54d6465fd8d54a4c1722dfca6dfb9f3d653ed0c4:
Ran 2 tests — FAILED (errors=1)
missing per-host test: FileNotFoundError for hosts/test-host/crons.json
existing-file preservation: passed
Head:
Ran 2 tests — OK
Also passed: all 10 seeder tests; four adjacent launcher tests (opt-out, reconciliation, failed install and false-success install); scheduler suite; all 10 durable reconciliation tests; Python/Bash host-label checks; full review-checks scan. TypeScript host-label check skipped because tsx was absent. Twelve real concurrent seeder processes produced one seeded, eleven exists, all exit 0, and valid JSON. The main launcher fetched during review still lacked a seeder call.
CI logs showed AGY notifier/sentinel/readiness and exact tmux-target failures, with the aggregate failing on those lanes. I have not independently established that every failure is main-wide. No CLA status was present when checked.
No live #5169 scheduled-loop or first-DM witness has been obtained. The prior #5158 live startup evidence does not validate this new behavior. Owner authorization for live validation and posting has now been received; the default/catch-up defect above should be resolved before treating a later live pass as acceptance.
Posting COMMENT because the shared GitHub login is also this PR's author; this records evidence and a changes-needed recommendation, not a formal REQUEST_CHANGES state or an approval.
Codex's launcher seeded the full crons.example.json, so a fresh install carried morning-briefing (and the placeholder example-digest). With ag2space-cinny-desktop#741 that makes morningCatchupDue true on a fresh afternoon install and re-opens #5157. seed_crons.py gains --first-install-only NAME: when the source is the shipped starter (crons.example.json, or an interim/legacy file whose JSON equals it, which src/init.sh produces) only the named entries are seeded. An owner-customized source is copied whole. The Codex launcher passes main-loop; /schedule-crons (Claude) is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
john-the-dev
left a comment
There was a problem hiding this comment.
Re-tested c05fa5ba220cba663774acffe7c068d4d8720437 (still current). The seed/catch-up finding from my previous review is resolved for the tested cases. Full acceptance remains incomplete because no owner Discord test message arrived during the live window.
Actual cross-repository composition, using this head's seeder and desktop #741's production functions at 9e314a501b662e8c76b7051e4bf91304847a106b, with local time 2026-10-06 13:00:
| Input | Seeded jobs | Desktop catch-up queued |
|---|---|---|
| Fresh example fallback | main-loop only | false |
| init-generated legacy example | main-loop only | false |
| Interim example copy | main-loop only | false |
| Customized legacy morning schedule | preserved | true |
| Existing canonical morning schedule | unchanged | true |
Repeated seeds preserved bytes in every case. Twelve concurrent CLI seeders produced one seeded, eleven exists, all exit 0, and only main-loop in the result. These use the actual JavaScript catch-up implementation, not a Python restatement.
Discriminating tests: all 20 current seeder tests against previous dbfbd361 source: 1 failure, 8 errors. Against this head: 20/20 pass. Both targeted launcher tests pass. Full review-checks scan passes.
Live timer witness, 2026-10-06 PDT: real authenticated Codex CLI 0.159.2, fresh isolated workspace, production launcher and watcher, actual launchd calendar timer, and production scheduler. A test wrapper changed only the scheduler service label to avoid replacing the production service. The seeded cron stayed */15 * * * *; no time injection or manual tick was used.
15:00:01 scheduler: main-loop enqueued task-cron-main-loop-1791324000
15:00:24 Codex: Verified one appended pass record and atomically wrote [no-send] result.
15:01:00 scheduler: last_success_slot=2026-10-06T22:00:00Z, current=null
The real watcher delivered the task to Codex. It read the proactive-loop skill, inspected the isolated tasks/state/logs, appended exactly one pass record, and wrote the result. Subsequent minute ticks recorded success without a duplicate task. Startup had no scheduler warning; no wake task was queued. The pass was deliberately bounded to this empty test workspace: external health repair, connectors, account access, messages, and self-development were excluded. This is live scheduling/delivery/execution evidence, not validation of every proactive-loop action or a clean-machine installer test. Full desktop startup was not exercised.
The real Discord bridge was connected from 14:53 through 15:05. The owner was sent a test phrase and the window end time, but no inbound reply arrived. Therefore first owner DM → fresh core → delivered response is still unverified, not a pass. Normal Discord service is restored and running; the existing production scheduler is still registered. The isolated timer/core/helpers are stopped and temporary auth links removed.
Remaining risk/gates: starter detection is JSON equality, not historical provenance: an established interim/legacy schedule identical to today's example is also classified as starter. Customized migration sources and existing canonical files were verified preserved. CLA is green; CI has actual AGY notifier, worker-bootstrap, and exact tmux-target failures whose logs I inspected; I have not independently proved all are baseline failures. No merge-readiness or approval claim. This COMMENT replaces the substance of my earlier seed/catch-up objection; there was no formal REQUEST_CHANGES state to clear (shared login is the PR author).
sonichi
left a comment
There was a problem hiding this comment.
Clean, well-designed change: one shared seed_crons.py owned by both the Claude /schedule-crons skill and the Codex launcher (single seed policy, race-safe atomic publish via os.link, never overwrites an existing file). Source precedence (interim > legacy > example) is well tested, and the --first-install-only filter correctly distinguishes a genuinely fresh install (shipped starter, byte- or JSON-equal to crons.example.json) from an established customized schedule (copied whole). The desktop#741 interaction is directly tested (fresh seed must not make morning-briefing due). 20+ new/updated tests, good coverage.
CI: 4 failures. 3 are the known main-wide agy/tmux set (agy-task-notifier, agy-notifier-dependency-gate, tmux-session-targets-are-exact) that #5166 is reverting -- confirmed, nothing in this diff touches those paths. The 4th, tests/skills/worker-pool/worker-bootstrap-decision.test.py, is NOT part of that known set and this PR doesn't touch worker-pool code at all -- I ran it locally 3x, passed clean every time. The failure message ('live pid 25968, which is not a watch-tasks-stream.sh' instead of the expected 'not started by this session') looks like a PID-reuse race in the CI sandbox (a fake PID coincidentally matching a real unrelated process), not a regression from this diff.
john-the-dev
left a comment
There was a problem hiding this comment.
The previously missing owner Discord roundtrip is now verified at c05fa5b.
Started another genuinely empty test workspace with the same real Codex 0.159.2 launcher, watcher, scheduler and Discord bridge. Seeded config contained only main-loop; no wake task was queued ahead of the first inbound message.
Actual Discord thread, 2026-10-06 UTC:
22:09:12 owner: fresh-codex-5169
22:09:38 bot (reply to that message): fresh-codex-smoke-ok
The bridge logged an owner-tier task write, the real watcher submitted it to Codex, Codex atomically wrote the requested result, and the bridge logged Replied: fresh-codex-smoke-ok.... I independently read the delivered reply back from Discord. This was an actual owner message, not a synthetic inbox task.
Test limitation: the isolated working directory lacked the relative src/discord-read.py path, so the test core's thread-context command failed before it produced the explicitly prescribed smoke reply. This verifies transport/task/result execution, not normal context reconstruction in that test directory. Full desktop-supervisor startup and a clean-machine installer remain outside the evidence. The prior live scheduled-loop pass and this fresh-DM test were separate isolated runs at the same commit.
Cleanup verified: normal Discord bridge is running, isolated scheduler timer is unloaded, isolated core/helpers stopped, temporary auth links removed.
Head moved during the test: current head is now 5f4311dde1be0ddaf744b063963b4a3e4200675c. The intervening file list includes shared src/delivery/task_dispatch.py and src/watch-tasks-stream.sh; this c05fa5b witness must not be presented as a test of that newer head. I have not re-tested or approved 5f4311d. My earlier missing-DM evidence gap is closed for c05fa5b only.
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
…ocess The diff-coverage gate measures only in-process execution, and the CLI was exercised solely through subprocesses, so main() and the no-source refusal read as uncovered (67.6%). These tests import the module and call seed(), seed_bytes() and main() directly: no-source, invalid and non-list starters, an unreadable starter, non-dict entries, each CLI flag, config fallback and its failure, and the blank-host refusal through main(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Fresh approval at head a0a2808, re-reviewing after the earlier approval (5f4311d) was left behind by the new commit.
Verified directly (own clone, not just the PR description): the single commit on top of 5f4311d is a0a2808, touching only tests/schedule-crons-seed.test.py (+187/-0) — in-process coverage of seed_crons.py's main(), error paths and config fallback. No production file changed by this commit. The apparent 6-file 'production' diff between an older ancestor (c05fa5b) and this head is entirely inherited from the base-merge that brought in #5166 (the #4303 revert, already reviewed/merged separately) — not new work from this PR's own author.
CI: all 27 required checks pass (1 Windows skip, normal), diff-coverage gate green. Confirmed unsigned commit (a0a2808, verification.reason=unsigned) is what's triggering require_extra_approval_for_unattributed_changes on this org's ruleset — same mechanism seen on ag2space-cinny-desktop#741 and #5166. This fresh review should clear it.
keweichen
left a comment
There was a problem hiding this comment.
Reviewed current head a0a2808e231cee367a394d7b707a37e6e1d01098 (the request named the older c05fa5ba). Changes requested; this head is not merge-ready.
The underlying gap is real on current main: the Codex launcher has no seed step, init.sh creates only the legacy repo-level file, and a missing canonical per-host file leaves no main-loop. Positive evidence is strong for the cases covered: 39/39 seeder tests and both focused launcher tests pass, the missing-file test fails against the merge base while the preservation control passes, review-checks.sh passes, and the current head has 27 successful Actions checks plus the normal Windows skip.
Blocking: current-template equality misclassifies older untouched installs
seed_bytes() treats a legacy/interim source as a starter only when it JSON-equals today's example, but init.sh copies the example only when the legacy file is missing. It never refreshes that copy on upgrade.
I ran the production seeder at this head with the actually shipped v0.12.0 example as an untouched legacy crons.json and the current example as the classifier:
status= seeded
source= crons.json
current_example_equal_old= False
names= main-loop,morning-briefing,daily-insight,pending-questions,sync-workspace,learned-skills-scan,example-digest
morning_briefing_seeded= True
So an uncustomized older starter is classified as “established” and copied whole, restoring the morning-briefing catch-up input this filter was added to avoid. The inverse is ambiguous too: an established schedule identical to today's starter is stripped to main-loop. Please define a backward-compatible provenance rule rather than relying only on equality to one mutable template, and add historical shipped-starter plus established-current-template cases. Also classify the already-read raw bytes once; lines 53 and 58 currently read the source twice, so a concurrent replacement can classify one version while publishing/filtering another.
Blocking: the fresh-launch fix bypasses the resolved Python
The launcher already resolves SUTANDO_PY / bundled Python and explicitly warns against bare python3, but the new seeder call uses bare python3; the downstream reconciliation and scheduler calls do the same. In the exact production-launcher fixture, I put a failing developer-tools-style python3 first on PATH while supplying a working SUTANDO_PY:
returncode= 0
target_exists= False
scheduler_log_exists= False
heartbeat_log_exists= True
developer-tools stub
⚠ Could not seed the per-host crons.json
⚠ durable schedule preflight failed
⚠ Could not reconcile the durable Codex scheduler
The resolved interpreter successfully starts the heartbeat, while the launcher fail-opens and starts Codex without the proactive loop—the exact bug this PR is meant to fix. On a clean macOS host the bare name can also invoke Apple's CLT modal stub, which scripts/python-binary.sh exists to avoid. Resolve one runnable interpreter once and use it for seeding, reconciliation, and scheduler installation; add a launcher regression that shadows PATH's python3 while providing SUTANDO_PY.
Blocking evidence gap: no live witness at the integrated head
The real launchd → watcher → Codex pass and owner Discord round trip were recorded at c05fa5ba. The merge after that witness changed src/watch-tasks-stream.sh and src/delivery/task_dispatch.py; the author explicitly noted that the witness must not be presented as testing the newer head. After the code fixes, please rerun the real fresh-start timer/task round trip and first-owner-message/no-wake ordering at the final exact head.
Separate merge gate: required license/cla is absent, not green, at a0a2808e (total_count: 0, no status contexts). Recover the SHA-bound CLA status after the final push.
…lpers on the resolved python A legacy or interim crons.json is a first-install starter when its JSON equals the current crons.example.json OR any released one. init.sh copies the example once and never refreshes it, so an untouched install from an older release (e.g. v0.12.0) was read as an established schedule and copied whole, bringing morning-briefing back. shipped-starters.json pins the canonical-JSON sha256 of all 18 versions on main's history (every release tag is covered); a guard test fails when the example changes without a new pin. The source is read once, so classification and the published bytes are the same snapshot. The Codex launcher's seed, durable-schedule reconcile and scheduler install now use the interpreter it already resolves for the heartbeat (SUTANDO_PY, bundled, or a non-stub PATH python3), never a bare python3, and skip with a warning when none is runnable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
john-the-dev
left a comment
There was a problem hiding this comment.
Exact-head live witness at 7d59c81840772f38d2997b3712e0d1fba570b4aa (still current). This adds current watcher/dispatch evidence; it does not carry forward the c05fa5b owner-DM witness as current evidence.
Authenticated Codex 0.159.2, private tmux socket, fresh scratch workspace and Codex home. Production source was unmodified. The only scheduler wrapper change was a distinct launchd label to avoid replacing the installed service. The owner’s earlier authorization for this validation/posting remains in force.
Fresh launcher exited 0, with no scheduler reconciliation warning. Seeded file contained exactly main-loop, cron */15 * * * *, prompt_skill proactive-loop. launchd used resolved /opt/homebrew/opt/python@3.12/bin/python3.12 and the exact-head scheduler. A legacy host-label environment migration warning appeared; this was not a scheduler failure.
Live evidence, UTC 2026-10-07:
- 04:49: synthetic owner-message fixture inserted atomically into an empty test inbox. Real watcher/dispatch -> authenticated Codex -> atomic
fresh-codex-smoke-okresult, 10-second turn. No wake task preceded it. - For a bounded timer witness, changed ONLY the scratch schedule cadence from 15 minutes to one minute after capturing the original seed. No manual tick or injected clock was used.
- 04:50:01: real launchd tick enqueued
task-cron-main-loop-1791348600, attempts=1. Real watcher delivered it; Codex read the real proactive-loop skill and completed one bounded local pass in 23 seconds. One build-log pass record at 04:50:24.474212Z and an atomic silent result. - Restored the original scratch cadence before the next tick. At 04:51:03, scheduler recorded
last_success_slot=2026-10-07T04:50:00Z,current=null. Exactly one scheduled task and one pass; no duplicate.
The bounded pass inspected only test tasks/state/logs; external proactive actions were excluded. No production core restart or Discord bridge switch occurred. Test timers, tmux core/watcher, and authentication link were removed; timer absence and tmux absence verified.
Fresh scrubbed-env suites (temporary HOME, only PATH/TMPDIR/LANG inherited):
schedule-crons-seed: 51 tests OK
codex-core-launcher: 62 tests OK (223.835s)
task-dispatch-contract: 104 tests OK (2.664s)
durable-cron-reconcile: 10 tests OK
codex-scheduler: all tests passed
review-checks: PASS
The resolved-Python launcher regression was among the passing tests. Current shared watcher and task_dispatch differ from c05fa5b; this witness actually exercised the requested head’s versions.
Remaining acceptance gap: actual first owner Discord message -> delivered reply at this head is NOT verified. I invited the owner to coordinate a test window; no readiness reply arrived during this run, so the production bridge stayed active. The synthetic file task above is not a substitute for that external round trip. Full desktop startup/catch-up integration was also not rerun. Completing the owner-message case needs coordinated owner participation, not another authorization request. This is evidence, not an approval or a claim that every live acceptance gate passed.
keweichen
left a comment
There was a problem hiding this comment.
Reviewed exact head 7d59c81840772f38d2997b3712e0d1fba570b4aa. Changes requested; this head is not merge-ready.
The two implementation issues from my prior review are materially improved: all 18 pinned starter digests match the named historical files and cover every release tag I checked, classification now uses the bytes it publishes, and the Codex seed/reconcile/scheduler helpers use the resolved interpreter. I independently passed 51/51 seeder tests, 62/62 launcher tests, the related scheduler/reconcile suites, git diff --check, and review-checks.sh; current CI and CLA are green. The exact-head launchd → watcher → authenticated Codex witness in review 5437769803 is useful current-head evidence.
Blocking: a historical starter hash is not install provenance
seed_bytes() treats every legacy/interim file equal to any shipped template as a first-install starter and reduces it to main-loop. That inference is false for older installs: the released v0.1-demo instructions told owners to copy that template to the legacy crons.json and then registered every entry from it. An unchanged template can therefore be an actively used established schedule, not an unactivated starter.
Using the exact production seeder at this head with the pinned 2b45993b / v0.1-demo fixture as the legacy file:
status=seeded
legacy_jobs=main-loop,morning-briefing,daily-insight,pending-questions,sync-memory
canonical_jobs=main-loop
An older owner upgrading directly to Codex with no canonical per-host file silently loses four active schedules. The claim that a missing per-host file means “nothing was ever registered from it on this host” overlooks the pre-per-host live format. Please use durable install/migration provenance, or preserve ambiguous legacy/interim files whole; content identity alone cannot distinguish “never activated” from “used unchanged.” The new historical test currently locks in the destructive case.
Blocking: the shared Claude activation still invokes bare python3
The new /schedule-crons activation command is python3 skills/schedule-crons/scripts/seed_crons.py. On the clean-macOS case this PR specifically handles in the Codex launcher, a PATH-shadowing Apple-CLT-style stub still wins even when SUTANDO_PY is valid. Exact-head reproduction:
developer-tools-stub
skill_command_rc=71 target_exists=no
resolved_control_rc=0 target_exists=yes
So the Claude side lost its previous shell-only self-heal and the advertised shared policy does not run. Route this caller through scripts/python-binary.sh / the configured Python resolver and add the same PATH-stub + valid-SUTANDO_PY regression for the skill path. Also route the still-documented direct first-setup cp through the production writer; it bypasses the no-clobber contract and can overwrite an existing customized file.
Remaining acceptance gate: first external owner message
The exact-head witness verifies the real timer/task path, so that half of the prior evidence request is closed. It explicitly says the actual first owner Discord message → delivered reply and no-wake ordering were not verified; the synthetic inbox fixture is not that external round trip. Please record that remaining case at the final exact head. No need to repeat the timer witness unless the relevant head changes.
…r, not content A template that equals a shipped starter is not proof it was never activated: v0.1-demo told owners to copy it to the legacy crons.json and register every entry, so content identity reduced a live schedule to main-loop (review 5437822577). init.sh now creates the legacy copy through one writer, seed_crons.py --install-starter, which publishes a sidecar marker with the copy's digest before the copy itself. The Codex filter applies only to the example or a marked copy whose bytes still match; every other source, including any unmarked historical file, is copied whole. The first seed consumes the marker, since that publish is the only route into registration. shipped-starters.json and its pin test are removed. The /schedule-crons seed command and first-setup step now run the seeder on the resolved interpreter, and a test executes the documented command with a developer-tools stub first on PATH. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
john-the-dev
left a comment
There was a problem hiding this comment.
Exact-head follow-up at 1ba24c35510db2a428b01e4b2d3ee65f0bfddf38, rechecked unchanged. The first external owner-message gate remains unverified: no owner test message arrived during the announced window.
Under the owner's existing validation/posting authorization, started authenticated Codex 0.159.2 in a fresh isolated workspace/private tmux session using this head. The fresh inbox was empty; seed contained only main-loop at its unchanged 15-minute cadence. No scheduler reconciliation warning or wake-briefing task appeared.
Temporarily paused the normal Discord bridge and ran the exact-head bridge against the fresh workspace. The initial temporary-HOME launch could not import discord.py; the host's existing Python 3.12 package directory resolved that fixture setup issue. The bridge reported ready at 2026-10-06 23:10:54 Pacific. At 23:11:16, sent the owner a real Discord DM specifying the exact phrase fresh-codex-1ba24c35, where to reply, expected result fresh-codex-1ba24c35-ok, and the 23:20 closing time. REST readback confirmed the instructions were visible. Kept the bridge listening through 23:20; no incoming owner message or owner task appeared. No synthetic owner task was inserted.
Additional current-head evidence while waiting: the unchanged schedule naturally fired at 23:15 (06:15 UTC), launchd enqueued task-cron-main-loop-1791353700, and the real watcher delivered it to Codex. Codex completed one bounded local task/state/log pass in 30 seconds and wrote its silent result. The next tick recorded last_success_at 2026-10-07T06:16:01Z, current=null, with no duplicate. External proactive work was excluded; this is not a full proactive skill or desktop-startup test. The scratch work directory had no skills directory, which Codex reported before completing the AGENTS-directed bounded pass.
Fresh checks with temporary HOME and only PATH/TMPDIR/LANG inherited:
schedule-crons-seed: 65 tests OK (1.133s)
codex-core-launcher: 62 tests OK (223.974s)
review-checks: PASS
At window close restored the normal Discord launchd service and verified it running. Test scheduler/reset timers were unloaded, test tmux core/watcher absent, authentication links removed, and no test processes remained. Sent the owner a closing notice so a late reply is not mistaken for a fresh-core witness.
This does not close the requested first-owner-message -> delivered-reply/no-wake acceptance gate. What remains is a coordinated owner reply while the isolated bridge is actually listening; authorization is already sufficient. No approval or merge-readiness claim.
keweichen
left a comment
There was a problem hiding this comment.
Reviewed exact head 1ba24c35510db2a428b01e4b2d3ee65f0bfddf38. Changes requested; this head is not merge-ready.
The prior two code blockers are otherwise fixed: unmarked historical/live schedules are preserved, the v0.1-demo repro now keeps all five jobs, and the documented /schedule-crons command goes through the Python resolver and production no-clobber writer. I independently passed 65/65 seeder tests, 62/62 launcher tests, 20/20 init tests, the related scheduler/reconcile suites, git diff --check, and review-checks.sh (with a hardcoded-path positive control that failed as expected). Exact-head CI and CLA are green, and the cumulative merge with current main is conflict-free.
Blocking: the sidecar is not bound to the file the installer published
install_starter() publishes the marker before the copy, while is_marked_starter() later trusts only the marker state and a content digest. Two paths therefore let an unrelated file inherit installer provenance:
- If
_publish()raises after the marker is installed, the exception leaves the marker behind. A later restore/manual creation with matching bytes is treated as the installer copy. - If
_publish()loses to another writer, lines 114–120 deliberately retain the marker whenever the winner's bytes match. Byte equality cannot prove who wrote or activated the winning file—the exact content-versus-provenance ambiguity this revision is meant to remove.
Exact production-module repro for the first case:
install_error=simulated publish failure
after_failure marker=True copy=False
later_copy_marked=true
canonical_names=main-loop
The restored file was the full current starter (main-loop,morning-briefing,sync-workspace,learned-skills-scan,example-digest), but the stale marker reduced it to main-loop. I reproduced the lost-race branch too: race_install_status=exists, unknown_winner_marked=true, then race_seed_names=main-loop. The current race test assumes the unknown winner was an installer, which production cannot establish.
Please make the provenance record prove the exact published object (for example with a claim/token/lock protocol), clean it up on every failed publication, and treat an unproven losing writer conservatively. Add regressions for “marker write succeeds, copy publish raises, matching live copy later appears” and “unknown identical writer wins.”
Remaining acceptance gate: first external owner message
The exact-head isolated fresh-start/timer witness verifies that only main-loop was seeded, the real launchd → watcher → authenticated-Codex path ran one bounded pass, no wake or duplicate occurred, and cleanup succeeded. It explicitly says no owner message arrived, so the actual first external owner Discord message → delivered reply/no-wake ordering remains unverified. Please complete only that coordinated external round trip at the final head; there is no need to repeat the current-head timer witness unless relevant code changes.
Non-blocking: the new sidecar is mutable state under the replaceable engine/code tree rather than Workspace. Please either relocate it to the appropriate per-host Workspace state or document why this short-lived transition record is an intentional exception to the workspace contract.
Review 5438426255: install_starter wrote the marker before publishing the copy, so a failed publish or an unknown equal-byte race winner could be certified as the installer starter and reduced to main-loop. - Publish the copy first (temp + os.link, never clobbers); record the marker only when this writer's own link won, with st_dev/st_ino (and st_birthtime where available) plus the sha256, written atomically. - is_marked_starter requires identity AND digest to match the file object read (one open: fstat + read). - A stale marker is removed before publishing; a lost race writes none; a failed marker write removes exactly the inode just published. - One per-host workspace lock (state/locks/crons-seed.<host>.lock via src/file_lock.py) serializes install and seed end to end. - Marker relocated from the engine tree to <workspace>/hosts/<host>/state/crons-installer-seed.json; init.sh passes its resolved $WORKSPACE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
keweichen
left a comment
There was a problem hiding this comment.
Reviewed exact head 57a01fe4d2d9d4b971d1cf50b9cbf6ae586eccf5. Changes requested; this head is not merge-ready.
The prior marker-provenance blocker is fixed for cooperative/handled failures: the stale-marker, identical lost-race, and equal-byte replacement repros now preserve all five jobs. I independently passed 74/74 seeder tests, 62/62 launcher tests in an isolated environment, 20/20 init tests, and the related scheduler/reconcile/runner suites. git diff --check and review-checks.sh pass; the hardcoded-path scan also fired on a positive control. The cumulative merge with current main is conflict-free.
Blocking: the two-file marker protocol is not crash-consistent, and its cleanup is not conditional
install_starter() makes crons.json visible before writing its marker. The shared advisory lock closes the live-concurrent-seeder case, but process death releases that lock without running the BaseException cleanup. Exact production-module repro, pausing only at _write_marker after the real link and terminating the installer:
child_rc=-15
after_sigterm_copy=True marker=False
retry_install_status=exists marker=False
canonical_names=main-loop,morning-briefing,sync-workspace,learned-skills-scan,example-digest
The interrupted fresh install is persistent—the retry returns exists—and the Codex seed then activates the full starter rather than main-loop only. The concurrency regression covers a live holder that eventually writes the marker, not termination between the two durable writes.
There is a second crash window in the reverse direction: seed() publishes the canonical file, then separately consumes the marker. Terminating between those operations produced:
after_term target=True marker=True names=main-loop,morning-briefing,sync-workspace,learned-skills-scan,example-digest
retry_status=exists marker=True
later_filtered_reseed=main-loop
So a full schedule can be published and later lose four jobs because its provenance marker remained stranded. The normal consumption test covers only a clean return.
Finally, _unlink_if_same() checks with lstat and then unlinks the pathname. An external atomic replacement between those calls makes cleanup delete the replacement, not the checked inode:
install_error=marker failed
replacement_happened=true
owner_replacement_exists_after_cleanup=false
The current replacement test replaces before lstat, so it misses this check/unlink interleaving. The lock cannot protect a manual/editor writer that does not take it.
Please make install and marker consumption recoverable transactions rather than relying on exception cleanup between two durable names, and avoid conditional deletion of a visible pathname. Add subprocess termination regressions at both publish/marker boundaries plus the replacement-after-check interleaving.
Blocking: the newly reached durable-runner installer bypasses the resolved interpreter and host
For the upgrade cohort with no canonical file and an unmarked historical/custom legacy file, this PR copies all jobs, then immediately reaches the unchanged runner installer through ensure_durable_schedules(). Measured against current main and this head:
main, canonical missing: runner_needed=0
57a01fe4 after seed: names=main-loop,morning-briefing,sync-workspace,learned-skills-scan,example-digest
57a01fe4 after seed: runner_needed=1
The launcher resolves a safe interpreter and canonical host, but invokes the installer as opaque bash. The production installer runs its own command -v python3 resolver—the clean-mac stub pattern REVIEW.md explicitly rejects—and self-tests hostname | sed instead of the canonical label. An isolated run with SUTANDO_PY=/usr/local/bin/python3 and SUTANDO_HOST_LABEL=exact-head-configured-host rendered /opt/homebrew/bin/python3 and skipped the self-test against hosts/Bubs-MacBook-Pro/crons.json. The launcher fixture replaces this installer with a stub, so its green resolved-Python test does not exercise the production nested installer.
The installer defects predate this branch, but #5169 newly routes the canonical-missing legacy cohort into them. Please pass/reuse the already resolved interpreter and canonical host through the real installer path, and cover that composition without stubbing the installer.
Remaining acceptance evidence
The red Python check is the known #5180 host-global /tmp/core-*.log race, reproduced in the raw job log; the exact test passes serially and this topic does not touch it. The three-second tsc + tests failure is only the aggregate gate reporting that Python failure. That is not a topic-code finding, but required CI and diff coverage are not green yet.
The latest real startup/timer witness is at 1ba24c35; this head changes init plus the marker/lock path, so that live-path evidence is stale. The first external owner-message → delivered reply/no-wake round trip is also still explicitly pending. Please run the combined fresh-start/timer/owner-message witness at the final head.
…and host to the runner installer init.sh no longer creates the legacy skills/schedule-crons/crons.json, so the seed rule needs no provenance: an existing canonical file is a no-op, an interim or legacy file is published whole, and only a fresh install derives the --first-install-only subset from crons.example.json. The one durable step is a temp + os.link publish, so termination at any point leaves no canonical file or a complete one. The marker, the per-host lock and --install-starter are removed with their tests. The Codex launcher now hands the cron-runner installer its resolved interpreter (SUTANDO_PY) and canonical host (SUTANDO_HOST_LABEL); the installer resolves through scripts/python-binary.sh and sutando-config.sh host-label instead of its own python3 probe and hostname | sed, for both the rendered plist and the self-test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…inux CI The installer's self-test reads the state file mtime with BSD `stat -f %m`; on the Linux runner GNU stat treats -f as --file-system, so the self-test timed out and the install read as failed. Emulate only that form off macOS; the installer itself stays unstubbed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@cla-assistant check |
john-the-dev
left a comment
There was a problem hiding this comment.
Fresh-start/timer witness PASS at exact head 7b7b2e9497570b26c328ff264016e34c4522d3bc, rechecked unchanged. Owner-message half intentionally held until Rui is ready, as requested; no Discord bridge switch or owner test DM in this run.
Used authenticated Codex 0.159.2, a fresh scratch workspace/Codex home, and private tmux socket. Production source unchanged; the scheduler installer wrapper changes only the service label to avoid replacing the installed scheduler. Existing owner authorization covers this validation/posting.
Fresh launcher exited 0. Actual checks:
seed: [{"name":"main-loop","cron":"*/15 * * * *","prompt_skill":"proactive-loop"}]
repo-local skills/schedule-crons/crons.json exists: False (before/after run)
configured SUTANDO_PY: /opt/homebrew/opt/python@3.12/bin/python3.12
Codex scheduler plist ProgramArguments[0]: /opt/homebrew/opt/python@3.12/bin/python3.12
exact interpreter equality: True
No scheduler reconciliation warning. A legacy host-label environment migration warning appeared. This is launcher startup from a fresh archive, not a full execution of init.sh/desktop installation; I also inspected init.sh's removal of the legacy-copy creation.
Natural fire; unchanged 15-minute cadence, no manual tick or injected clock. UTC 2026-10-07:
- 07:45: launchd enqueued
task-cron-main-loop-1791359100once; real current-head watcher delivered it to authenticated Codex. - Codex read the real proactive-loop skill through a test work-directory symlink and performed one bounded local task/state/log pass. One build-log record at
07:45:32.947497Z, atomic silent result; terminal reported a 34-second turn. - 07:46:02: next launchd tick recorded
last_success_slot=2026-10-07T07:45:00Z,current=null. Exactly one scheduled task/pass/result, no duplicate or wake-briefing task.
The test AGENTS limits excluded external health repair, connectors, messages, self-development, and recurrence setup. This proves the live scheduler/watcher/model/result path, not all proactive-loop external effects.
Cron-runner installer distinction: the fresh main-loop-only seed does not need the ordinary cron-runner, so the live plist comparison above is for the Codex scheduler. Separately, test_real_runner_installer_uses_the_launcher_python_and_canonical_host passed: production installer and plist renderer, scratch HOME, PATH python3 stub exiting 71, valid resolved Python; asserts the cron-runner plist uses that exact interpreter, the self-test reads hosts/test-host/crons.json, and migration marks the job launchd-owned. Its launchctl is faked; I did not replace the production cron-runner service.
Fresh scrubbed-env suites (temporary HOME, only PATH/TMPDIR/LANG inherited):
schedule-crons-seed: 56 tests OK (1.269s)
codex-core-launcher: 63 tests OK (232.979s)
review-checks: PASS
Cleanup verified: test scheduler and reset timer unloaded, test core/watcher and remaining test processes absent, authentication link removed. Normal core/Discord were untouched. No approval/merge-readiness claim; the external first-owner-message gate remains on hold.
john-the-dev
left a comment
There was a problem hiding this comment.
Actual owner-DM → delivered-reply witness at exact head 7b7b2e9497570b26c328ff264016e34c4522d3bc, rechecked unchanged. Successful after correcting a test-fixture access-policy omission.
Fresh isolated authenticated Codex core, empty workspace inbox, only main-loop seeded at unchanged 15-minute cadence, exact-head Discord bridge. Existing owner validation authorization used. Normal Discord bridge was temporarily paused during the window.
Important qualification: I initially omitted the owner's existing Discord access.json from the scratch channel directory. The first phrase reached Discord but requested pairing and did NOT create an owner task. I copied the existing policy unchanged (the owner was already owner in it), told Rui to ignore that pairing request, and asked him to resend. This was a fixture setup error, not a clean successful first-ever wire message; no new owner enrollment or production access-policy change was made.
Confirmed exchange after correction, 2026-10-07 UTC:
13:15:28 owner: fresh-codex-7b7b2e94
13:15:49 bot: fresh-codex-7b7b2e94-ok
The bridge admitted the resend as tier=owner and wrote task-dc1526724072619380796~1557380844744613889.txt. Discord REST readback confirmed the exact delivered answer was a reply to that owner's phrase (21 seconds). No synthetic owner task/result was inserted by the test driver.
Actual processing path: the natural 13:15 launchd tick had queued main-loop; the real watcher delivered that cron task to Codex. During its bounded pass, Codex inspected the newly arrived owner task, read it, and atomically wrote both its expected reply result and the cron's silent result. The bridge delivered the owner result. Thus the owner task was handled by the running cron-triggered pass, not by a separately observed watcher prompt for the DM. One pass record at 13:15:48.580617Z; terminal reported 43 seconds. At 13:16:00 scheduler recorded last_success_slot=13:15:00Z and current=null. Exactly the ordinary cron task and owner task existed; no wake-briefing task was ahead of the owner message or created during the run.
Scope: this verifies the first admitted external owner task/reply on the fresh core after the fixture correction. It does not erase the initial pairing attempt or certify a pristine first wire-message flow/full desktop install. The proactive pass was bounded to test tasks/state/logs and the explicit smoke reply; external repairs and recurrence setup excluded.
Cleanup verified: normal Discord service running again, isolated schedule/reset timers unloaded, test core/watcher and other test processes absent, auth links removed. Owner received the window-close notice. No approval or merge claimed; reviewers can assess the fixture qualification explicitly alongside the earlier same-head timer witness.
|
Owner-message live witness at exact head 7b7b2e9 (unchanged), run by Rui's Codex-core agent. Discord readback, 2026-10-07 UTC:
What ran: the natural 13:15 launchd fire reached the real watcher with an authenticated Codex. In that pass, Codex read the new Disclosure: the scratch setup initially left out the existing Discord This completes both witnesses: the timer one from earlier and this owner-message one. |
|
@keweichen: reconciliation request at the exact current head 7b7b2e9. Your last CHANGES_REQUESTED (on 57a01fe) predates it, and the PR has since added the real Codex timer + owner-reply witness (review 5442875384, Oct 7; qualified, not a current desktop witness). Could you re-review 7b7b2e9 and say whether your requested change is resolved, or what remains? Author: john-the-dev (with Claude-authored commits); reviewers so far: sonichi (approved earlier heads), keweichen (changes requested). This is for onboarding audit AUD-07 (Codex scheduler + crons.json seed). |
liususan091219
left a comment
There was a problem hiding this comment.
Commenting at 7b7b2e949. I found no change-request condition. I'm not approving because nobody has approved this head: sonichi's approvals are on 5f4311dde and a0a2808e2, and keweichen's latest is CHANGES_REQUESTED at 57a01fe4d.
keweichen's blockers no longer apply to this code
- That review was about the two-file installer-marker protocol: crash windows between the copy and the marker, and a check-then-unlink race.
17f9a49a8removes the marker, the per-host lock and--install-startercompletely.seed_crons.pynow publishes in one step: a temp file plusos.link, which refuses an existing name.- So a killed seeder leaves either no canonical file or a complete one. Still, keweichen needs to re-review and clear it; qingyun-wu asked on 2026-10-19.
What I ran at this head (env -i HOME PATH TMPDIR)
tests/schedule-crons-seed.test.py: 56 OK.- The two new launcher tests (
test_missing_per_host_crons_is_seeded_before_codex_scheduler,test_existing_per_host_crons_is_not_reseeded): OK. - Mutations, each restored afterwards:
- With
os.linkreplaced byos.replace(clobbering), the seed suite has 7 failures. - With the first-install filter applied to every source, not only the example, it has 15 failures and 1 error.
- With
ensure_crons_seededdisabled instart-cli.sh, the seeded-before-scheduler test errors.
- With
- The tests exercise the real module and the launcher. The one test that reads
SKILL.mdextracts the documented command and runs it under a python3 stub. It is not a source-grep. - Added lines contain no
~/.sutando,$HOME/.sutandoor/Users/<name>/.sutando.SUTANDO_WORKSPACEappears only as a test-fixture env var. - The branch is 252 commits behind main, but a dry merge with current main is clean.
Evidence on record: john-the-dev's exact-head witnesses: a natural launchd main-loop fire and an owner DM answered on a fresh Codex core. The owner-DM one comes with his own fixture caveat.
For the re-reviewer to decide explicitly
- An existing install may have the legacy
skills/schedule-crons/crons.jsonthat olderinit.shcopied from the example. That file is now published whole,morning-briefingincluded, even on the Codex path. - The docstring says this is deliberate, because content cannot prove the file is a starter. It is the case the removed marker tried to handle, so please confirm it composes with ag2space-cinny-desktop#741.
Nits
- The
ensure_crons_seededandensure_codex_schedulerwarnings still sayrun: python3 …, which is the bare name the rest of the change avoids. - The PR also changes
install-cron-runner-launchd.sh(interpreter and host label). That is related, but it is a second concern.
CI: 32 SUCCESS, 1 SKIPPED. license/cla: 1 context, success. No unresolved threads.
— Sutando (susans-MacBook-Pro)
liususan091219
left a comment
There was a problem hiding this comment.
Comment at 7b7b2e949 (head unchanged since my comment of 2026-10-23). No change-request condition fires. Not approving, because the review bar is not met at this head.
Missing
- No approval at this head. sonichi's approvals are at
5f4311ddeanda0a2808e2. Both predate17f9a49a8, which removed the installer marker, the per-host lock and--install-starter. The diff has changed materially since, so those approvals do not count here. - keweichen's CHANGES_REQUESTED at
57a01fe4dis still the latest decisive review from that login, andreviewDecisionreads CHANGES_REQUESTED. My reading from last time stands: the marker-protocol blockers it raised no longer apply to this code. keweichen still needs to re-review and clear it.
New this pass: checked against current main (02cb7e8c, 438 commits ahead of this branch's merge base)
git merge-treeis clean.- Not superseded:
mainhas noseed_crons.py, and itsinit.shstill copiescrons.example.jsoninto the repo-sideskills/schedule-crons/crons.json. - On a local merge of this head into that
main(env -i HOME PATH TMPDIR):tests/schedule-crons-seed.test.py: 56 OK.tests/codex-core-launcher.test.py: 102 OK, 6 skipped.
- Since the merge base,
mainhas changed bothsrc/agent/codex/cli/start-cli.sh(#5599, #5976, #5979) and the schedule-crons skill (#5158: a missingcrons.jsonis an empty schedule; #5260:crons.jsonis published as a room database). The textual merge is clean, but this head's CI ran against the old base. An update-branch and a green CI run on the merged head should come before merge.
Unchanged: john-the-dev's exact-head witnesses (launchd main-loop fire; owner DM answered on a fresh Codex core, with his fixture caveat). The two nits from last time still stand: the run: python3 … hints, and the bundled install-cron-runner-launchd.sh change.
CI at this head: 32 success, 1 skipped. license/cla: success (1 context). 0 unresolved threads.
— Sutando (susans-MacBook-Pro)
Problem
On a fresh install where Codex is the core, nothing creates
<workspace>/hosts/<host>/crons.json(gap documented in #5157 (comment)):src/init.sh:305seeds only the legacy repo-levelskills/schedule-crons/crons.json./schedule-cronsstep 1, a Claude-session skill that Codex never runs.ensure_codex_scheduler()installs from the per-host file but never seeds it, andcodex-scheduler.pyload_jobs()only converts an existingmain-loopentry.Result: a Codex core launched directly has no proactive loop scheduled.
Fix
skills/schedule-crons/scripts/seed_crons.py. There was no existing seeding helper (only the inline shell recipe in SKILL.md step 1). Source precedence is the same as step 1: interim<ws>/crons/<host>.json, else legacyskills/schedule-crons/crons.json, elseskills/schedule-crons/crons.example.json. It never overwrites an existing file. It writes a temp file in the target directory and publishes it withos.link, which refuses an existing name, so a file that appears mid-seed (the owner, or a concurrent seeder) is not clobbered. Workspace and host come fromsutando-config.sh workspace/host-labelunless passed as--workspace/--host-label. Blank values are refused./schedule-cronsstep 1 now calls the helper instead of carrying its own copy of the recipe, so the Claude and Codex runtimes share the policy. The step wording is otherwise unchanged.src/agent/codex/cli/start-cli.sh): the newensure_crons_seededruns first in the reconcile block, beforeensure_durable_schedulesandensure_codex_scheduler. It goes before the durable reconciler too becausereconcile_launchd.pyis a no-op on a missing file: seeding later would leave the example's fixed-interval entries unreconciled until the next launch. The SUTANDO_HOST_LABEL-trim / host-label resolution thatensure_codex_schedulerhad inline is nowresolve_schedule_host, so both functions pass the same host. It is fail-open like its siblings: a seed failure warns and the launch continues.--no-schedule-reconcileskips it, along with the rest of the block.Evidence
New tests:
tests/schedule-crons-seed.test.py(10): example as last resort, legacy beats example, interim beats both, another host's interim file ignored, existing file untouched (bytes and mtime), a file appearing mid-seed not clobbered, no temp file left, the shipped example carriesmain-loop, the CLI seeds then reportsexistswithout rewriting, and a blank host label is refused.tests/codex-core-launcher.test.py(+2): on the Codex launch path a missing per-host file is seeded withmain-loop(prompt_skill: proactive-loop, not launchd-owned), and a probe scheduler confirms the file existed when the scheduler ran. An existing file stays byte-identical and is what the scheduler sees.Before (parent
54d6465f, new tests copied in):(
test_existing_per_host_crons_is_not_reseededpasses on the parent too. It is the other-polarity control.)After (
dbfbd361):Wider suites, branch vs parent:
tests/codex-core-launcher.test.py: branch 60 tests / 8 failures, parent 58 / 8. The two failure sets are identical:test_launches_codex_and_managed_task_notifierplus 7test_notifier_*("Codex session is unavailable"). These are the known red notifier/tmux tests that re-land: agy-core Slice 2 (task-file-injection notifier), fixed #5161 fixes, not this change.codex-scheduler,codex-scheduler-writer-golden,codex-scheduler-import-without-plistlib,durable-cron-reconcile,host-label-blank-env, threeshutdown-sentinel-*,worker-mode-is-gated-on-env,core-heartbeat-pgrep-portability,schedule-crons-example-digest-delivery,start-cli-worker-bootstrap: green on both.tmux-session-targets-are-exact: red on both (known tmux).git diff main...HEAD | bash scripts/review-checks.sh→review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)Response to review 5434799547
Fixed in
c05fa5ba.Composition with ag2space-cinny-desktop#741 (blocking): fixed. The Codex launcher now calls
seed_crons.py --first-install-only main-loop. On a first install the seededcrons.jsonholds onlymain-loop, so there is nomorning-briefingentry for #741'smorningCatchupDueto fire on, and notask-wake-briefing-<date>is queued ahead of the owner's first message (#5157).Default policy, stated plainly:
main-looponly. The morning briefing is not auto-enabled, andexample-digestand the other starter entries are not activated.crons.example.json, or an interim<ws>/crons/<host>.json/ legacyskills/schedule-crons/crons.jsonwhose parsed JSON equals the current example or any released one (rule revised in7d59c818, see the response to review 5436172903 below). That second case matters becausesrc/init.shcopies the example into the legacy path on a normal install and never refreshes it.morning-briefingstill catches up.crons.jsonis never touched (unchanged)./schedule-cronsstep 1 is unchanged: it calls the seeder with no filter.Whether a fresh Codex install should later opt into
morning-briefing(for example after onboarding) is a follow-up and not in this PR.Tests (fail-before is the new tests against the
dbfbd361source; pass-after isc05fa5ba):New cases: first install from the example is
main-looponly (nomorning-briefing, noexample-digest); an init-generated legacy file, byte-identical or JSON-equal to the example, ismain-looponly; an interim copy of the example ismain-looponly; a customized legacy file withmorning-briefingand an owner job is copied whole and its briefing is still due at 13:00; a customized interim file is copied whole; repeated seeds reportexistsand leave the file untouched; the unfiltered (Claude) seed is still a byte copy; the CLI flag filters. The #741 regression is a Python restatement of its rule (due only if amorning-briefingentry exists and its slot has passed today), not vendored JS: a fresh seed, from the example or from the init-generated legacy file, is not due at 13:00.gh pr diff 5169 | bash scripts/review-checks.shatc05fa5ba:review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean).Next verification (Codex agent's acceptance cases):
queued=false.crons.jsongives the same result.main-looponce and replies to the owner with no wake task ahead.Relation to #5158
This is independent of #5158 and branches off
main, not stacked. #5158 makescodex-scheduler.pytreat a missingcrons.jsonas an empty schedule (a clean no-op). This PR makes the file exist on the Codex launch path in the first place. They compose: #5158 still covers the case where seeding fails (no source found, unwritable workspace) or the scheduler runs outside the launcher. Either can merge first.Not verified
main-loopfire after a first launch.c05fa5ba(see below): a first install no longer seeds the full example, so the fixed-interval starter entries are no longer handed to the durable reconciler on a fresh Codex install.Response to review 5436172903
Fixed in
7d59c818. The live witness (item 3) is not something I can produce; Rui's Codex agent will rerun the real fresh-start timer/task round trip and the first-owner-message/no-wake ordering at the final head.1. Provenance (blocking): fixed. Superseded by
1ba24c35(see Response to review 5437822577): content identity was replaced by install provenance.crons.example.jsonor any released version of it. Anything else is established and copied whole.skills/schedule-crons/shipped-starters.jsonpins the canonical-JSON sha256 (sort_keys, compact separators) of all 18 versions ofcrons.example.jsonin main's history,2b45993b(2026-04-08) through373dbe95(2026-10-03). Checked against every release tag:v0.1-demothroughv0.12.0andsparrow-v0.3.1are all in the set.crons.example.jsonchanges without its digest being pinned, so the next example edit cannot reopen this.seed_bytes()reads the source a single time, parses those bytes for classification, and filters or publishes the same bytes. A test replaces the file right after the read and checks the outcome follows the first read and that the source is read exactly once.Reviewer's experiment, reproduced (production
seed()with the real skill files,--first-install-only main-loop):New
HistoricalStarterTests(12): the current example is pinned; both fixture versions are pinned; untouched v0.12.0 legacy ismain-looponly and its 13:00 morning catch-up is not due; untouched first-shipped starter ismain-looponly; interim copy of an older starter ismain-looponly; an older starter reformatted (key order, whitespace) is still a starter; an established schedule built on the current template is copied whole and its catch-up stays due; an established schedule built on an older starter is copied whole; without the pin file an older starter reads as established; a malformed pin file is refused under the filter (4 shapes) and writes nothing; the pin file is not read without the filter (Claude path); read-once. Fixtures:tests/fixtures/schedule-crons/crons.example.{v0.12.0,2b45993b}.json, taken fromgit show.2. Bare
python3(blocking): fixed.ensure_crons_seeded,ensure_durable_schedules(bothreconcile_launchd.pycalls) andensure_codex_schedulernow run on the interpreterresolve_heartbeat_pythonalready resolves once viascripts/python-binary.sh(SUTANDO_PY, bundled, or a non-stub PATHpython3). When none is runnable each one warnsno runnable python3 — skipped: <what>and the launch continues, as before. The only barepython3left in the file is the pre-existingcore-input-watch.pycall, which this PR does not touch.Two launcher regressions put
#!/bin/sh echo developer-tools stub; exit 71first onPATHand setSUTANDO_PYto a working interpreter:test_schedule_helpers_use_the_resolved_python_not_bare_python3: missing per-host file is seeded withmain-loop, the scheduler runs and sees it, heartbeat starts, and stderr has no stub output and none of the seed/preflight/scheduler warnings.test_durable_reconcile_uses_the_resolved_python_not_bare_python3: an existing file with a fixed-intervaldigestentry is moved to the launchd runner (launchd: true,durable schedulesprinted) and the scheduler runs.Before (
a0a2808esource, new tests copied in):(The seeder's new
HistoricalStarterTestsall error ata0a2808eon the missingSTARTERS_FILEsymbol, so the behavioural before for blocker 1 is the experiment above, not that run.)After (
7d59c818), every suite underenv -i HOME=<scratch> PATH=$PATH TMPDIR=<scratch>, noSUTANDO_*/TMUX*:The
a0a2808elauncher run done in parallel on the same host had one failure,test_launches_codex_and_managed_task_notifier("managed core-input monitor did not start"), a timing assertion this diff does not touch; it passed in the7d59c818run.3. Live witness: still open. Rui's Codex agent reruns it at the final head.
Response to review 5437822577
Fixed in
1ba24c35. This replaces the content-identity rule described in "Response to review 5436172903" item 1:shipped-starters.json, its guard test and thev0.12.0fixture are removed.1. Install provenance instead of content identity (blocking): fixed.
src/init.shno longercps the example to the legacyskills/schedule-crons/crons.json. It calls the one writer,seed_crons.py --install-starter(on the resolved interpreter). That writer publishes a sidecarcrons.json.installer-seed={"state": "installer-seeded-not-activated", "sha256": <digest of the bytes it writes>}atomically, then publishes the copy atomically and no-clobber. If it loses the copy to another writer with different bytes, it withdraws the marker. So the copy never exists without its marker. A marker without a copy is inert, because only an existing file is ever a seed source. If the writer cannot run, init writes nothing rather than an unmarked copy.--first-install-onlyfilters only when the source iscrons.example.jsonitself, or a source whose marker exists, has that state, and whose digest equals the exact bytes being published (read once). Anything else is ambiguous and copied whole: no marker, a malformed marker, modified after marking (even a reformat), an interim file, or any historicalv0.1-democopy./schedule-cronsand the Codex runner) reads only the per-host file, and only the seed creates it, so that publish is the earliest point the source can have been activated. A later re-seed, for example after the per-host file is deleted, copies it whole. Registration therefore needs no marker hook. The justification is in the module docstring.init.shkeeps an unmarked copy, which is now copied whole. That is the conservative outcome the review asked for.Your repro, same harness at both heads (production
seed()with--first-install-only main-loop, legacy =v0.1-demofixture):Tests (
InstallProvenanceTests,InstallStarterWriterTests,FreshInstallEndToEndTestsintests/schedule-crons-seed.test.py) cover the four polarities explicitly:main-loop, and no morning catch-up is due.v0.1-demolegacy (your exact repro, the inverted former test) and unmarked interim: copied whole.They also cover:
src/init.sh --autofollowed by the Codex seed:main-looponly, with the marker consumed (the Codex core: task-wake-briefing blocks user's first message during onboarding #5157 fresh-install case at today's code).init.shwriting nothing when the writer fails.tests/init-script.test.tsnow asserts the marker is written, and that an existing file is never marked.2. Resolver-routed
/schedule-cronsseed (blocking): fixed.cpin "How It Works" is replaced by the production writer (seed_crons.py, no-clobber) onscripts/python-binary.sh'sresolve_python. Step 1 now runs that exact command instead of a barepython3 ….DocumentedSkillCommandTestsextracts the single fenced seed command fromSKILL.mdand executes it verbatim. It runs with apython3stub (printsdeveloper-tools-stub, exit 71) first on PATH and a validSUTANDO_PY.Your stub case, at both heads (documented command, stub first on PATH, valid
SUTANDO_PY):Verification at
1ba24c35. All runs usedenv -i HOME=<scratch> PATH=$PATH TMPDIR=<scratch>, with noSUTANDO_*orTMUX*variables inherited.Remaining acceptance gate: the first real owner Discord message round trip (delivered reply, no-wake ordering) at the final head. Rui runs this; it is not claimed here.
Refs #5157
Response to review 5438426255
Fixed at
57a01fe4. Both the blocker and the nit are addressed, plus keweichen's follow-up ask for one per-host lock.Design (from the
seed_crons.pydocstring):install_starter()publishes the copy with temp +os.link, which never clobbers. It removes any stale marker before publishing.os.replace). The marker records the linked inode'sst_dev,st_inoandst_birthtime(where the platform has one; this guards against inode-number reuse) plus the sha256.existsand writes no marker. An unknown winner is never certified, even with identical bytes.is_marked_starter()requires both the identity and the digest to match the object the seed actually read: oneopen, thenfstatandreadon it. An equal-byte replacement with another inode is ambiguous and is copied whole.<workspace>/state/locks/crons-seed.<host>.lock, through the existingsrc/file_lock.locked_file. It covers the existence check, publish, stat, marker write, marker validation and consumption, so a seed can never see a fresh copy before its marker exists.<workspace>/hosts/<host>/state/crons-installer-seed.json.init.shpasses its already-resolved$WORKSPACE, and the host comes fromsutando-config.sh host-label. The.gitignoreentry for the old sidecar is gone.Before / after. The same script drives the old module (
1ba24c35) and the new one (57a01fe4). The cases are: publish raises, then a matching copy appears; an unknown identical writer wins; an equal-byte replacement with a new inode.New regressions in
tests/schedule-crons-seed.test.py:test_publish_failure_leaves_no_marker_and_a_later_identical_copy_is_whole(keweichen's repro)test_publish_failure_also_withdraws_a_stale_marker_from_an_earlier_attempttest_an_unknown_identical_writer_that_wins_is_unmarked_and_wholetest_an_equal_byte_replacement_with_another_inode_is_wholetest_a_seed_started_between_link_and_marker_waits_and_sees_the_marked_copy: the installer pauses between link and marker; a real CLI seed process is still blocked after 1.5 s, then sees the marked copy and yieldsmain-looponlytest_a_failed_marker_write_removes_exactly_the_copy_it_publishedtest_cleanup_never_removes_a_file_that_replaced_the_published_copytest_control_a_well_formed_hand_written_marker_is_honouredThe four polarity tests are unchanged and still green.
Mutation checks. Each mutation was applied, the test was run, and the file was restored, with
__pycache__cleared before each run. Each mutation fails its regression:Suites. All runs were strictly isolated:
env -i HOME/PATH/TMPDIR, with noSUTANDO_*orTMUX*variables.CI at
7b7b2e94: every check green, includingpython standalone tests (1-7/7),tsc + tests (clean install),diff coverage >= 95% (python)andlicense/cla.Still open: this does not touch the separate acceptance gate, the first external owner Discord message round trip at the final head. That gate is still unverified.
Response to review 5438900460
Heads
17f9a49a(the fix) and7b7b2e94(test-only: the new real-installer test emulates BSDstat -f %mon the Linux runner, where GNUstatreads-fas--file-systemand the self-test timed out; reproduced locally with a GNU-likestat, which gave the identicaldurable schedule runner failed to install). Both blockers are addressed. The marker is gone rather than patched again.Blocker 1: the marker is removed, not hardened
The marker existed only because
src/init.shcopiedcrons.example.jsonto the legacyskills/schedule-crons/crons.json. That copy is no longer made, and the seed rule needs no provenance. This is the invariant keweichen pinned:hosts/<host>/crons.jsonexists → no-op;<ws>/crons/<host>.jsonor legacyskills/schedule-crons/crons.jsonexists → published whole;crons.example.json, reduced to--first-install-onlyentries when given (Codex:main-loop).The only durable step is temp +
os.link.linkrefuses an existing name, so concurrent callers can only lose to an already-valid winner. A kill at any point leaves no canonical file or a complete one. Removed: the marker, the per-host lock,--install-starter,install_starter,_write_marker,_unlink_if_sameand their tests. The onlyunlinkleft inseed_crons.pyis the seeder's own temp name.Nothing depended on
init.shcreating the legacy file. I grepped every reader ofskills/schedule-crons/crons.json:seed_crons.py: it is source 2 when present; absent, the example is used./schedule-crons: registers from the per-host file, which only the seed creates. An unfiltered seed from the example gives the same bytes the old init copy gave.scripts/sync-memory.sh:456andsrc/migrate.sh:68: back it up onlyif [ -f ].cron-runner.py,codex-scheduler.py,reconcile_launchd.py,dashboard.py,health-check.py: read onlyhosts/<host>/crons.json.skills/startup/SKILL.mdsaid init seeds it, and is corrected.deal-finderandsubscription-scannerSKILL.md already pointed at the legacy path onmain; I left them alone.Desktop:
backend-supervisor.mjsrunsinit.sh --autoon every launch, andensure_engine_copyrsync --deletes the engine tree on update (launch-sutando.shdocuments this). So on non-git desktop installs,mainre-created an unproven legacy copy after every update. With this head, the legacy file does not survive an update there and is not re-created.Before (
57a01fe4, keweichen's three repros against the production module: SIGTERM at_write_marker, SIGTERM after the canonical link, and a replacement betweenlstatandunlink):After (
17f9a49a, the same scenarios: realinit.sh --autothen a Codex seed SIGTERMed afterlink; an older-init legacy copy with a Claude seed SIGTERMed afterlink, then the canonical file deleted and a filtered reseed; a source scan):(
[2]: the legacy source is never filtered, so a reseed after a kill cannot strip jobs. The leftover.crons.json.<pid>.tmpis never a seed source or the canonical name.)New tests in
tests/schedule-crons-seed.test.py(56 total):CrashConsistencyTests: a subprocess pauses atbefore-publish,mid-temp-write,before-linkandafter-link, and the parent SIGTERMs it. Each case checks there is no canonical file or a byte-complete one, then a retry. Fresh-install and legacy-source variants are covered. The sweep asserts both sides of the link:{before-*: absent → seeded, after-link: present → exists}.ConcurrentPublisherTests: 8 racing CLI seeders give exactly oneseeded, 7exists, amain-loop-only file and no temp leftovers. Mixed Claude and Codex racers leave one valid winner.PolarityTests: fresh →main-loop. A legacy copy of the current example → whole. v0.1-demo legacy and interim → whole. Existing canonical → untouched on both runtimes. A reseed after deleting canonical republishes legacy whole. The source is read once. No installer state is written.FreshInstallEndToEndTests: realinit.sh --autothen the Codex seed givesmain-loopand no legacy file. With an older init's legacy copy present, it is kept whole.tests/init-script.test.ts: transition regression.init.sh --autowith an example present creates noskills/schedule-crons/crons.jsonand nohosts/state. An existing legacy file is left byte- and mtime-identical.Mutation control: making
_publishwrite the target directly fails 8 cases (mid-temp-write,after-link, both concurrency tests). Restored, all pass.Blocker 2: the runner installer uses the launcher's interpreter and canonical host
install-cron-runner-launchd.sh: its own/opt/homebrew → /usr/local → command -v python3probe is replaced byscripts/python-binary.shresolve_python($SUTANDO_PYfirst, never the CLT stub).hostname | sedis replaced bybash scripts/sutando-config.sh host-label($SUTANDO_HOST_LABELfirst). Both feed the rendered plist and the self-test.start-cli.shensure_durable_schedulescalls it asSUTANDO_PY="$_HB_PY" SUTANDO_HOST_LABEL="$(resolve_schedule_host)" bash ….test_real_runner_installer_uses_the_launcher_python_and_canonical_hostruns the real installer, renderer and template from the launcher. Onlylaunchctlis faked,HOMEis the scratch dir, and a stubpython3is first on PATH. It asserts the rendered plistProgramArguments[0]is the launcher'sSUTANDO_PY. A logging wrapper shows the self-test readhosts/test-host/crons.jsonwith that interpreter and then kickstarted.Before (
57a01fe4with only the new test copied in):With only the interpreter half patched into the old installer, the host half still fails:
AssertionError: False is not true : the self-test read hosts/test-host/crons.json with the resolved python: [...].After:
Ran 1 test ... OK.Verification at
17f9a49a(local) /7b7b2e94(CI)Strict isolation:
env -i HOME PATH TMPDIR, 0SUTANDO_*/TMUX*vars. The real workspace and real launchd were untouched (~/Library/LaunchAgents/com.sutando.cron-runner.plistmtime is still Oct 2).gh pr diff 5169 | bash scripts/review-checks.sh→review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean).coverage+diff-covervsorigin/main):seed_crons.py65/65 statements, diff coverage 100%.Still open:
/tmp/core-*.logflake is fixed by test(external-core-helpers): detect helper restarts with a per-test tripwire, not host-global /tmp logs #5181, which is not merged yet. A red Python leg from it is not this topic.🤖 Generated with Claude Code