docs(adr): goal-driven cronjob (disable_on_success)#816
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
OpenAB PR ScreeningThis is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Screening report## IntentPR #816 proposes an ADR for extending The operator-visible problem: recurring cron prompts keep firing even after the task’s goal is complete. This creates noise, duplicated work, and stale scheduled agent activity. The ADR aims to support “goal-driven” scheduled runs without introducing a full goal-runner system yet. FeatThis is a docs / architecture decision PR. Behavior described by the ADR:
Who It ServesPrimary beneficiaries:
Secondary beneficiary:
Rewritten PromptImplement the ADR for goal-driven usercron jobs by adding Before each scheduled run, if Do not introduce a separate goal-runner system or model-based judging in this phase. Add tests for success match, failed command, missing sentinel, notification behavior, and persisted disablement. Merge PitchThis is worth advancing because it documents a small, practical step toward goal-driven scheduled agents without committing OpenAB to a larger orchestration system too early. Risk profile: moderate for future implementation, low for this ADR. The main reviewer concern will be whether mutating Best-Practice ComparisonRelevant OpenClaw principles:
Relevant Hermes Agent principles:
Not relevant yet:
Those belong to the ADR’s stated Phase 2. Implementation OptionsOption 1: Conservative ADR-only merge Merge the ADR as design documentation, but require a follow-up implementation issue before code changes. Add reviewer notes about locking, atomic writes, exact sentinel parsing, and notification routing. Option 2: Balanced usercron implementation Implement Option 3: Ambitious goal-runner foundation Introduce a goal-runner abstraction behind usercron with structured state, run logs, disable conditions, retry metadata, and future hooks for stuck detection or LLM judging. Expose Comparison Table
RecommendationAdvance the ADR, but steer follow-up toward Option 2: balanced usercron implementation. It matches the PR’s stated philosophy: extend the existing scheduler, keep the success condition explicit, and avoid building a full goal-runner too early. For merge discussion, the important acceptance bar should be: file locking, atomic TOML writes, exact sentinel matching, stable delivery routing, and tests around non-match cases. Split Phase 2 explicitly into a later design item: run logs, stuck detection, escalation, state deltas, and LLM judging. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
chaodu-agent
left a comment
There was a problem hiding this comment.
I would hold this ADR until #818 is adjusted. Two contract details currently need alignment: the timeout mitigation depends on actually killing/reaping the command on timeout, and the ADR says missing id with disable_on_success is a startup error while the implementation currently warns and skips invalid usercron entries. Once those semantics are settled, the ADR should match the code exactly.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
F1: clarify validation timing — startup error on missing id/match,
not deferred to cron fire
F2: fix flow diagram orphan character
F3: define match semantics as substring (case-sensitive) with
guidance to use unique markers
chaodu-agent
left a comment
There was a problem hiding this comment.
Group Review — 超渡法師 (Kiro)
✅ LGTM — Ready to merge
All three findings from 普渡法師's review have been addressed in commit 687bfec:
| Finding | Status | Verification |
|---|---|---|
| F1: Validation timing | ✅ Fixed | Step 1 now validates at load time (startup error); step 2 no longer duplicates validation |
| F2: Flow diagram orphan | ✅ Fixed | Diagram rewritten with clean dual-branch structure |
| F3: Match semantics | ✅ Fixed | Explicitly defined as case-sensitive substring match with unique marker guidance |
Design direction is sound — minimal extension of existing cron infra with clear Phase 1/Phase 2 boundary. ADR is ready to merge.
This comment has been minimized.
This comment has been minimized.
|
LGTM ✅ — Well-structured ADR that correctly documents the design implemented in PR #818. 🟢 INFO: Review notes
No issues found. |
Summary
ADR for extending usercron
[[jobs]]withdisable_on_success— enabling goal-driven "escape room" mode.Key Decisions
disable_on_success— command evaluates the goal before sending the regular promptdisable_on_success_matchbefore OpenAB treats the goal as achievedenabled = falsedirectly to$HOME/.openab/cronjob.toml, no separate state file✅ Goal achievedbefore disablingPhase 1 vs Phase 2
Context
Design discussion: https://discord.com/channels/1491295327620169908/1504239931940409587
cc @pahud