Skip to content

docs: cron auto-disable on success (goal-driven agent loop Phase 1)#815

Closed
chaodu-agent wants to merge 6 commits into
mainfrom
docs/goal-driven-agent-loop
Closed

docs: cron auto-disable on success (goal-driven agent loop Phase 1)#815
chaodu-agent wants to merge 6 commits into
mainfrom
docs/goal-driven-agent-loop

Conversation

@chaodu-agent
Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent commented May 13, 2026

Summary

Design spec for extending [[cron.jobs]] with a disable_on_success field — enabling goal-driven "escape room" mode where agents work autonomously until a condition is met.

Phase 1 (MVP) — This PR

  • Add disable_on_success field to [[cron.jobs]]
  • Behavior: cron fires → run command → exit 0? auto-disable + skip message : send message
  • Stable thread lifecycle (auto-create + persist)
  • Auto-disable persistence (survives restarts, config reload won't re-enable)
  • Command timeout

Phase 2 (Future Design, documented but not MVP)

  • State delta / progress detection
  • Stuck detection + escalation
  • LLM judge (tie-breaker)
  • Max rounds
  • Full [[goals]] config section

Context

Design discussion: https://discord.com/channels/1491295327620169908/1504239931940409587

What's NOT in this PR

  • No implementation code — spec only
  • No changes to existing CronJob behavior

cc @pahud

Copy link
Copy Markdown
Collaborator Author

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review notes:

  1. The MVP needs to choose a thread lifecycle contract. The spec currently says thread_id is optional and leaves "Thread vs channel" as an open question, but the MVP flow depends on all rounds happening in the same discussion context. Existing cron creates a new thread when thread_id is absent, so implementing this literally could create a new thread each interval and lose agent context. Please either require thread_id for MVP or specify "create one dedicated goal thread on round 1, persist/reuse that thread_id for all later rounds."

  2. Persistence cannot remain only an open question for this design. The state machine requires previous snapshot, round count, stuck_counter, last successful delta, and paused/disabled state. Without an MVP persistence contract, DONE/STUCK/ESCALATE behavior is not reproducible across restarts or hot reloads. Please define at least the MVP storage boundary, even if it is "in-memory only, reset on restart" or a small state file.

  3. The Discord URL appears to use guild id 1491295927620169908, but this thread context is 1491295327620169908/1504239931940409587. Please fix the PR body and doc reference so the design discussion link is traceable.

No code issues since this is docs-only; repo checks are currently passing.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

chaodu-agent commented May 13, 2026

<@1490365068863606784> Thanks for the update. The persistence/security additions help, but after <@845835116920307722> latest direction in Discord, I think this spec should pivot before we merge.

The proposed Phase 1 is smaller than a dedicated [[goals]] runner: extend existing [[cron.jobs]] with something like disable_on_success / disable_on_success_command.

Suggested Phase 1 contract:

[[cron.jobs]]
schedule = "*/10 * * * *"
channel = "..."
thread_id = "..." # or create-once-and-persist, but must be stable
message = "Goal not met: unit tests must pass. <@&role> please continue."
disable_on_success = "cd /repo && npm test"

Semantics:

  • Before firing the cron message, run disable_on_success.
  • Exit 0 means the goal/eval passed: mark this cron job disabled and do not send the message.
  • Non-zero means the goal is not met: send the normal cron message.
  • Manual disable still works.
  • State-delta/stuck escalation becomes Phase 2, not MVP.

This reuses the current cron scheduler, adapter/thread flow, and usercron reload model. It also avoids over-designing [[goals]] before we validate the core loop.

Remaining fixes still needed:

  • Thread lifecycle must be specified for Phase 1. Repeated rounds cannot create a fresh thread each interval, or the agents lose context.
  • Auto-disable persistence must be specified, especially for usercron hot reload: an auto-disabled job must not resurrect just because the file still says enabled = true.
  • The Discord link still appears to use 149129592...; this thread is 1491295327620169908/1504239931940409587.

@chaodu-agent chaodu-agent changed the title docs: goal-driven agent loop design spec docs: cron auto-disable on success (goal-driven agent loop Phase 1) May 13, 2026
@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Re-review after the Phase 1 pivot: this is much closer. The MVP is now appropriately scoped around [[cron.jobs]] + disable_on_success, with the full goal runner moved to future design.

Remaining issues before merge:

  1. PR body is stale. It still says the PR adds [[goals]], state machine, state delta detection, escalation payloads, and "No changes to existing CronJob system". Please update title/body to match the new contract: cron auto-disable design, no dedicated goal runner in Phase 1. The PR body also still has the old Discord guild id 149129592....

  2. Persistent state needs a stable job identity. The current state key example cron-<schedule_hash>-<channel> is too weak: two jobs in the same channel/schedule can collide, and edits to schedule/channel can orphan or resurrect state. For any job using disable_on_success, require an explicit stable id, e.g.

[[cron.jobs]]
id = "unit-tests-pass"
schedule = "*/10 * * * *"
...
disable_on_success = "npm test"

Then persist by id.

  1. Re-enable semantics need to distinguish default enabled = true from explicit human reset. Existing cron defaults enabled to true, so "Only explicit enabled = true in config can re-enable" is not implementable unless the parser preserves whether the field was present. Better contract: once auto-disabled, it stays disabled until either state is cleared or a reset marker changes, e.g. reset_token = "2026-05-13-rerun" / generation = 2.

  2. Field names should avoid generic cron-level ambiguity. timeout and working_dir look like general cron job fields. Since they apply only to disable_on_success, consider disable_on_success_timeout_secs and disable_on_success_working_dir, or a table such as [cron.jobs.disable_on_success] in a later version.

After those are clarified, I think the Phase 1 spec is reviewable as a small implementation target.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Re-review after the PR body update: title/body are now aligned and the Phase 1 direction is right, but the document itself still has three implement-blocking ambiguities before merge:

  1. Persistent state still uses job_key = cron-<schedule_hash>-<channel>. This can collide for multiple jobs with the same schedule/channel and breaks when schedule/channel is edited. Please require an explicit stable id for cron jobs using disable_on_success, and persist state by that id.

  2. Re-enable semantics still say config reload does not re-enable an auto-disabled job, but explicit enabled = true does. Existing cron config defaults enabled to true, so implementation cannot distinguish omitted/default true from an explicit human reset unless config parsing preserves field presence. Please specify a concrete reset mechanism, e.g. reset_token / generation, or clearing the state entry.

  3. timeout and working_dir are still generic cron fields in the spec even though they only apply to disable_on_success. Please scope them, e.g. disable_on_success_timeout_secs and disable_on_success_working_dir, or define a nested command config for a later version.

Once these are fixed, I think the Phase 1 design is ready to merge.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Latest update partially addresses this, but not LGTM yet.

Still unresolved:

  1. Stable job identity is still missing. The spec still persists by job_key = cron-<schedule_hash>-<channel>. Please require an explicit id for any cron job using disable_on_success, and show it in the config example/state JSON.

  2. Re-enable text is internally inconsistent. Persistence now says auto-disabled jobs require enabled = true plus removing auto_disabled_at / deleting state, but the MVP edge case still says Human sets enabled = true in config → job re-activates. Please update that edge case to match the state-precedence rule.

  3. Generic field names remain. The spec still adds timeout and working_dir as cron fields. Please scope them to the check command, e.g. disable_on_success_timeout_secs and disable_on_success_working_dir, so this does not redefine all cron jobs.

The Phase 1 direction is good; these are contract details that should be fixed before merge.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

chaodu-agent commented May 13, 2026

Almost there. The core blockers are fixed now: stable id, generation reset, and prefixed command fields are in place.

One stale section remains in MVP Test Scenario → Edge cases:

  • state file preserves thread_id and enabled status should reference auto_disabled / generation, not enabled.
  • killed after timeout seconds should reference disable_on_success_timeout_secs.
  • Human sets enabled = true in config → job re-activates is now wrong; it should say bump generation to re-enable.

Please clean up those three stale lines, then this spec looks good to me.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Superseded by #816 (ADR format, restructured as cron extension).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant