fix(cron): preserve silent NO_REPLY completions by bde1 · Pull Request #41287 · openclaw/openclaw

bde1 · 2026-03-09T17:03:38Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: isolated cron jobs can treat an intentional NO_REPLY completion as an interim acknowledgement and inject a second "complete the task now" prompt.
Why it matters: that creates duplicate work, transcript noise, and false not-delivered state for cron jobs that are supposed to end silently.
What changed: runEmbeddedPiAgent() now returns a narrow silentReply signal for exact final NO_REPLY completions with no payloads, isolated cron skips the retry when that signal is present, and cron delivery reuses the existing NO_REPLY silent-success path so delivery state stays correct.
What did NOT change (scope boundary): this does not change general empty-output handling, interim-ack heuristics for real acknowledgements, or any non-exact NO_REPLY text.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Isolated cron jobs that intentionally finish with exact NO_REPLY no longer get a forced follow-up prompt.
Delivery-requested cron jobs now treat those silent completions as intentional success instead of surfacing not-delivered.

Security Impact (required)

New permissions/capabilities? (Yes/No) No
Secrets/tokens handling changed? (Yes/No) No
New/changed network calls? (Yes/No) No
Command/tool execution surface changed? (Yes/No) No
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS 15.x
Runtime/container: local pnpm workspace
Model/provider: mocked embedded runner + cron isolated agent suites
Integration/channel (if any): isolated cron / Telegram-shaped delivery paths in tests
Relevant config (redacted): none

Steps

Configure an isolated cron job whose successful path ends with exact NO_REPLY.
Run the job under 2026.3.8 logic.
Observe the cron runner inject Your previous response was only an acknowledgement... and continue the run.

Expected

Exact NO_REPLY should end the run silently with no forced retry and no false not-delivered state.

Actual

The silent completion is stripped out of payloads, then misread as interim / empty and retried.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
- pnpm exec vitest run src/cron/isolated-agent/run.interim-retry.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/agents/pi-embedded-runner/usage-reporting.test.ts
- pnpm exec vitest run --config vitest.e2e.config.ts src/agents/pi-embedded-runner.e2e.test.ts
- pnpm build
- pnpm test (873 files passed, 7101 tests passed, 2 skipped)
- git diff --check
Edge cases checked:
- mixed assistant text containing NO_REPLY is not classified as silent
- silent cron completions still count as intentional delivery success
- descendant/interim retry heuristics still apply to real acknowledgement text
What you did not verify:
- pnpm check is still failing on untouched upstream formatting drift in CONTRIBUTING.md on current main

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes/No) Yes
Config/env changes? (Yes/No) No
Migration needed? (Yes/No) No
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert commit 5bd09eb70
Files/config to restore: src/agents/pi-embedded-runner/run.ts, src/cron/isolated-agent/run.ts
Known bad symptoms reviewers should watch for: legitimate empty non-NO_REPLY cron completions being treated as delivered

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk: over-classifying silent completions could hide real empty-output failures.
- Mitigation: the new signal only sets on exact final NO_REPLY when payload generation already produced nothing.
Risk: cron delivery state could drift from retry classification.
- Mitigation: the cron path now reuses the existing NO_REPLY silent-delivery handling, and both sides have dedicated regression coverage.

AI-assisted: Yes. Session summary: issue triage on latest origin/main, tests-first implementation, audit pass for delivery-state and default-test-gate coverage, then submission.

greptile-apps · 2026-03-09T17:07:11Z

Greptile Summary

This PR fixes a bug where isolated cron jobs would incorrectly retry an intentional NO_REPLY completion by misreading it as an interim acknowledgement, causing duplicate work and false not-delivered delivery states. The fix is well-scoped: a new resolveSilentReply() helper in the embedded runner detects exact final NO_REPLY completions and surfaces them as a silentReply signal; the cron isolated-agent runner skips its interim-ack retry when that signal is present, and synthesizes SILENT_REPLY_TOKEN as the delivery text so the pre-existing silent-success path in dispatchCronDelivery fires correctly.

resolveSilentReply logic is sound — guards on payloadCount > 0 (no payloads present) and stopReason === "error", then walks back through assistantTexts to find the last non-empty text and verifies it is an exact NO_REPLY via isSilentReplyText.
Cron retry guard (!interimRunResult.silentReply) is positioned correctly among the other shouldRetryInterimAck conditions.
Delivery synthesis (if (finalRunResult.silentReply && !synthesizedText)) correctly covers both the delivery-requested path (routes to the existing NO_REPLY silent-success handler) and the no-delivery path (run resolves as status: "ok" without side effects).
Test coverage is thorough: unit tests for exact vs. mixed-content classification, a full E2E run with a mock-silent model, interim-retry skip regression, and double-announce delivery guard.
Minor concern (style): resolveSilentReply excludes stopReason === "error" but not stopReason === "max_tokens". A model truncated at its token budget whose output happens to be exactly "NO_REPLY" would be silently classified as intentional. This is very unlikely due to the exact-match requirement but could be made explicit for safety.

Confidence Score: 4/5

This PR is safe to merge — the fix is narrowly scoped to exact NO_REPLY final completions and does not touch general empty-output handling, interim-ack heuristics for real acknowledgements, or any non-exact NO_REPLY text.
The core logic is correct and well-tested (unit, E2E, delivery-dispatch, and interim-retry regression tests all pass). The only minor gap is that resolveSilentReply does not explicitly exclude stopReason === "max_tokens", leaving a theoretical (but extremely unlikely) misclassification path for token-budget-truncated responses whose last text happens to be exactly "NO_REPLY".
No files require special attention. src/agents/pi-embedded-runner/run.ts carries the one style-level concern around max_tokens handling in resolveSilentReply.

_{Last reviewed commit: 5bd09eb}

src/agents/pi-embedded-runner/run.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5bd09eb707

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/cron/isolated-agent/run.ts

fix(cron): preserve silent NO_REPLY completions

5bd09eb

greptile-apps bot reviewed Mar 9, 2026

View reviewed changes

src/agents/pi-embedded-runner/run.ts Outdated Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Mar 9, 2026

View reviewed changes

src/cron/isolated-agent/run.ts Outdated Show resolved Hide resolved

openclaw-barnacle bot added agents Agent runtime and tooling size: S labels Mar 9, 2026

fix(cron): tighten silent reply guards

47af057

openclaw-barnacle bot added size: M and removed size: S labels Mar 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cron): preserve silent NO_REPLY completions#41287

fix(cron): preserve silent NO_REPLY completions#41287
bde1 wants to merge 2 commits intoopenclaw:mainfrom
bde1:codex/fix-cron-no-reply-retry

bde1 commented Mar 9, 2026

Uh oh!

greptile-apps bot commented Mar 9, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

bde1 commented Mar 9, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps bot commented Mar 9, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant