Skip to content

feat(cron): disable_on_success for goal-driven usercron#817

Closed
chaodu-agent wants to merge 4 commits into
mainfrom
feat/disable-on-success
Closed

feat(cron): disable_on_success for goal-driven usercron#817
chaodu-agent wants to merge 4 commits into
mainfrom
feat/disable-on-success

Conversation

@chaodu-agent
Copy link
Copy Markdown
Collaborator

Summary

Implements Phase 1 of the goal-driven agent loop ("escape room" mode) per ADR #816.

Adds disable_on_success support to usercron [[jobs]]: before firing a cron job, run a shell command — if it exits 0 (and optional match string is found in stdout), the goal is achieved, the job auto-disables, and a success notification is posted.

Changes

src/config.rs

  • Add 4 new optional fields to CronJobConfig:
    • disable_on_success: Option<String> — shell command to evaluate
    • disable_on_success_match: Option<String> — required stdout substring
    • disable_on_success_timeout_secs: Option<u64> — command timeout (default 60s)
    • disable_on_success_working_dir: Option<String> — working directory

src/cron.rs

  • evaluate_disable_on_success() — runs command with timeout, checks exit code + optional match
  • disable_job_in_usercron() — writes enabled = false back to usercron TOML file
  • Integration in run_scheduler loop: check runs before fire_cronjob, posts ✅ notification on success, triggers hot-reload

Tests (9 new)

  • No command → returns false
  • Exit 0 → returns true
  • Exit non-zero → returns false
  • Match present → returns true
  • Match absent → returns false
  • Timeout → returns false
  • Working directory respected
  • Usercron write-back sets enabled=false
  • TOML field parsing

Configuration Example

# $HOME/.openab/cronjob.toml
[[jobs]]
schedule = "*/10 * * * *"
channel = "123456789"
message = "Goal not met: tests must pass. Keep working."
disable_on_success = "npm test"
disable_on_success_match = "SUCCESS"
disable_on_success_timeout_secs = 60
disable_on_success_working_dir = "/repo"

Related

cc @pahud

Implements the Phase 1 'escape room' mode from ADR #816:
- Add disable_on_success field to CronJobConfig (shell command)
- Add disable_on_success_match (optional stdout match string)
- Add disable_on_success_timeout_secs (default 60s)
- Add disable_on_success_working_dir (optional cwd)
- Evaluate command before firing: exit 0 + match = goal achieved
- On success: post notification, write enabled=false to usercron, trigger reload
- On failure/timeout: fire cron job normally (send message)
- 9 unit tests covering all paths
超渡法師 added 2 commits May 13, 2026 23:10
- 🔴 F1: Replace blocking thread::sleep with async tokio::process::Command + tokio::time::timeout
- 🟡 F2: Rewrite disable_job_in_usercron with line-based editing (preserves comments)
- 🟡 F3: Only evaluate disable_on_success for usercron jobs (skip baseline)
- 🟡 F4: Match jobs by id field instead of schedule+channel+message
- Add id field to CronJobConfig (required for disable_on_success per ADR)
- Add test for comment preservation
- Convert evaluate tests to #[tokio::test] async
…ments

- 🔴 Remove duplicate id: fields caused by sed (compile error)
- 🟡 Add validation: disable_on_success without id is rejected at load time
- 🟡 Fix id line parser to strip inline comments before matching
@chaodu-agent chaodu-agent force-pushed the feat/disable-on-success branch from 0b1fe69 to 91738c1 Compare May 13, 2026 23:13
@shaun-agent
Copy link
Copy Markdown
Contributor

OpenAB PR Screening

This is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Click 👍 if you find this useful. Human review will be done within 24 hours. We appreciate your support and contribution 🙏

Screening report ## Intent

PR #817 adds a way for scheduled usercron jobs to stop themselves once an external goal condition is met.

The operator-visible problem: goal-driven cron loops currently keep firing even after the task is done. This creates noisy follow-up messages and requires manual disabling. The PR proposes a pre-run success check that can auto-disable the job and notify the channel when the goal is achieved.

Feat

Feature.

It adds disable_on_success fields to usercron job config. Before a cron job fires, OpenAB runs a configured shell command. If the command exits successfully, and optionally contains a required stdout match string, OpenAB marks the job as disabled in the usercron TOML file and posts a success notification.

Who It Serves

Primary beneficiary: agent runtime operators and deployers running goal-driven scheduled tasks.

Secondary beneficiaries: Discord users and maintainers, because completed loops should stop creating noise and require less manual cleanup.

Rewritten Prompt

Implement Phase 1 of goal-driven usercron auto-disable behavior.

Add optional usercron job fields:

  • disable_on_success
  • disable_on_success_match
  • disable_on_success_timeout_secs
  • disable_on_success_working_dir

Before dispatching a scheduled job, evaluate the configured command with a bounded timeout and optional working directory. Treat success as: exit code 0, plus matching stdout substring when configured. On success, persist enabled = false for that job in the usercron TOML, skip normal dispatch, post a clear success notification to the configured delivery target, and trigger scheduler reload.

Cover config parsing, success/failure/match/timeout behavior, working directory handling, and TOML write-back. Avoid overlapping evaluations and avoid corrupting usercron state on write failure.

Merge Pitch

This is worth advancing because it turns usercron from a repeated reminder mechanism into a basic goal-driven agent loop. That is directly useful for “keep working until condition X is true” workflows.

Risk profile is medium. The core concern is not the config shape; it is execution and persistence safety. Reviewers should focus on shell-command execution boundaries, timeout behavior, file write atomicity, concurrent scheduler ticks, and whether auto-editing user config is acceptable without stronger locking.

Best-Practice Comparison

Relevant OpenClaw principles:

  • Gateway-owned scheduling: relevant. The scheduler should own the decision to dispatch or suppress a job.
  • Durable job persistence: relevant. Auto-disabling must persist reliably across restarts.
  • Explicit delivery routing: relevant. Success notification should use the same routing model as the scheduled job.
  • Retry/backoff and run logs: partially relevant. This PR likely does not need full retry machinery, but failed success-check evaluations should be observable.
  • Isolated executions: relevant but probably underdeveloped. Shell checks should have clear execution boundaries.

Relevant Hermes Agent principles:

  • Gateway daemon tick model: relevant. This fits naturally into the scheduler tick before dispatch.
  • File locking to prevent overlap: highly relevant. Concurrent scheduler passes or reloads could race on TOML write-back.
  • Atomic writes for persisted state: highly relevant. Editing usercron in place is risky if not atomic.
  • Fresh session per scheduled run: less relevant for this phase unless the cron job launches agent sessions.
  • Self-contained prompts for scheduled tasks: adjacent but not central. This PR is about stopping criteria, not prompt design.

Implementation Options

Option 1: Conservative config-only gate
Add the fields and evaluate the command before dispatch, but do not write back to the TOML file. Instead, skip the current run and emit a success notification. Operators still disable the job manually.

Option 2: Balanced auto-disable with safe persistence
Keep the proposed behavior, but require atomic TOML write-back, scoped file locking, bounded command execution, structured logs, and tests around failed writes/concurrent evaluations. This is the best Phase 1 shape.

Option 3: Ambitious durable goal-runner model
Promote goal-driven cron into a first-class persisted job/run system with run records, status, logs, retry policy, explicit delivery routing, and isolated execution contexts. Usercron becomes a config source rather than the mutable state store.

Comparison Table

Option Speed to ship Complexity Reliability Maintainability User impact Fit for OpenAB right now
Conservative config-only gate High Low Medium High Medium Good if write-back risk is unacceptable
Balanced auto-disable with safe persistence Medium Medium High Medium-High High Best fit
Durable goal-runner model Low High Very high High long-term Very high Too large for this PR

Recommendation

Advance the balanced option.

The feature is directionally right, but merge discussion should center on hardening the state mutation path: atomic writes, locking, clear failure behavior, and observability. If those are not already solid in PR #817, split the work: merge config parsing and success evaluation first, then land TOML auto-disable once persistence safety is reviewed.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

CHANGES REQUESTED ⚠️ — Superseded by #818

What This PR Does

Implements disable_on_success for usercron jobs — same feature as #818 but with a simpler (and less robust) approach.

Findings

# Severity Finding Location
1 🟡 Line-based TOML editing is fragile — breaks on non-trivial formatting src/cron.rs:disable_job_in_usercron
2 🟡 disable_on_success_match is optional — risk of false-positive auto-disable on any exit-0 command src/cron.rs:evaluate_disable_on_success
3 🟡 No thread_id persistence — each fire creates a new thread instead of reusing src/cron.rs
4 🟡 Option<u64> for timeout with .unwrap_or(60) — less ergonomic than serde default src/config.rs
Baseline Check

Recommendation: Close this PR in favor of #818 which uses toml_edit for proper TOML manipulation, requires disable_on_success_match, persists thread_id, and has more comprehensive validation.

@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Closing in favor of #818 which is a strict superset: uses toml_edit for proper TOML manipulation, requires disable_on_success_match, persists thread_id, and has more comprehensive validation.

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.

3 participants