Skip to content

fix(harness): retain classified stop cause before host cleanup - #608

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/571-retain-stop-cause
Nov 11, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/571-retain-stop-cause

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

Summary

Supersedes #571 by @AmirrezaFarnamTaheri: their commit is cherry-picked onto current main with their authorship kept. Thanks for the fix. #571 was a draft 1,400+ commits behind main, so we're landing it from a fresh branch.

A host stop-latch adapter can record a classified halt cause after the loop last looked at it. FailureScopeMiddleware::after_agent then calls finish_run, which is allowed to clear that latch, so the cause was lost. This happens on exits that don't go through the loop's own pause copy, such as a deferred batch or a StopWithFinal/JumpTo(End) after the tools. after_agent now copies the policy's current_halt_cause into run.failure_cause before finish_run, and only when nothing has recorded a cause yet. A cause set by the loop or another middleware stays authoritative.

API Or Behavior Changes

AgentRun::failure_cause is now filled from the host policy's current halt cause at the end of a run when it would otherwise be None. The finish_run contract is unchanged; its doc now says the cause is captured first.

Tests

Run on our Linux CI runner (2026-11-11) on 7a41be9, parent main 813a2e1. The branch also merges cleanly onto today's main.

  • cargo fmt --all -- --check
  • cargo clippy -p tinyagents-harness --all-targets -- -D warnings (0 items)
  • cargo test -p tinyagents-harness --lib: 3311 passed, 0 failed, including the 3 finish tests from the original PR
  • Red control: with the capture in after_agent removed, classified_cause_survives_host_latch_cleanup fails (None instead of the uncertain_side_effect cause). finish_preserves_an_already_recorded_cause and unclassified_finish_does_not_invent_a_cause still pass, as they should.
  • N/A: --all-features builds and the full workspace cargo test were not run here; the change is in tinyagents-harness only and is covered by CI.

Documentation

middleware/library/README.md (from the original commit) describes the capture.

Capture a host's current halt cause during after_agent before finish_run
clears its per-run latch. Preserve an existing AgentRun cause and keep
cleanup behavior unchanged. Cover classified, existing, and unclassified
finalization paths with owner-level regressions.
@coderabbitai

coderabbitai Bot commented Nov 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

Or wait 14 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 456817fa-20f0-4f3a-a4ba-63e39feea13f
Commits

Reviewing files that changed from the base of the PR and between a030aa0 and 7a41be9.

Files selected for processing: 3

  • crates/tinyagents-harness/src/middleware/library/README.md
  • crates/tinyagents-harness/src/middleware/library/failure_scope.rs
  • crates/tinyagents-harness/src/middleware/library/failure_scope_finish_tests.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@tinysweeper

tinysweeper Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 7a41be9a4fb153f68d263385783f36e12e65d97a

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 3 of 3 files.
Pull request purpose (title): fix(harness): retain…

Diagnostics and supporting details

Full summary

Pull request purpose (title): fix(harness): retain classified stop cause before host cleanup. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 1 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

Features

  • Modified — Retain host-latched classified halt cause across finish_run cleanup: `after_agent` captures the policy's current halt cause into `AgentRun::failure_cause` before `finish_run` is allowed to clear the host latch, so classified stop causes latched after the loop's last observation are no longer lost at teardown, while an already-recorded cause remains authoritative. (crates/tinyagents-harness/src/middleware/library/failure_scope.rs impl<Ctx: Send + Sync> Middleware<(), Ctx> for FailureScopeMiddleware<Ctx> {, crates/tinyagents-harness/src/middleware/library/failure_scope.rs pub trait FailureScopePolicy<Ctx: Send + Sync>: FailurePolicy {)
  • Modified — Documentation of the capture ordering for finish_run and the library README: The `finish_run` doc comment and the `failure_scope` README entry now state that the driver copies any current classified cause into the run before calling the hook, so host adapter authors know the latch may be cleared after the cause is preserved. (crates/tinyagents-harness/src/middleware/library/README.md Grouped by extension shape:, crates/tinyagents-harness/src/middleware/library/failure_scope.rs pub trait FailureScopePolicy<Ctx: Send + Sync>: FailurePolicy {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 1 finding. _The code index is behind this pull request (indexed at `a2eb22b400c3`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane is answered with 0 findings across the 2 code files it reviewed; the README was excluded as prose/tabular data rather than flagged as a risk.
  • Lane summary: Reviewed 2 files; 0 findings. 1 file was not security-reviewed: crates/tinyagents-harness/src/middleware/library/README.md (prose or tabular data). _The code index is behind this pull request (indexed at `a2eb22b400c3`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane is answered with no findings: the new module covers latched-but-unrecorded, already-recorded, and absent-cause paths, follows the repository's sibling-test conventions, and the README note matches the behavior.
  • Lane summary: The change makes `FailureScopeMiddleware::after_agent` copy a host-latched classified cause onto `AgentRun::failure_cause` before `finish_run` clears the latch, and the new `failure_scope_finish_tests.rs` exercises all three branches: latched-but-unrecorded (would fail if the copy were dropped or ordered after `finish_run`), already-recorded (would fail if the copy overwrote the loop's cause), and neither present. The test file follows the repository's sibling-test conventions and the README note matches the behaviour, so the change is covered and safe to merge. _The code index is behind this pull request (indexed at `a2eb22b400c3`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane is answered with no sensitive content reported in the committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `a2eb22b400c3`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The description matches the diff: `FailureScopeMiddleware::after_agent` now copies `current_halt_cause` into `run.failure_cause` when it is `None` before calling `finish_run`, the `finish_run` doc and library README are updated to say so, and three focused tests cover the retained, already-recorded and absent-cause cases in a sibling `failure_scope_finish_tests.rs` declared per the repository's test-file rules. The change looks sound and the description covers everything significant in the diff. _The code index is behind this pull request (indexed at `a2eb22b400c3`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.018690
  • Tokens: 592274 input · 65825 output · 258432 cached · 297 embedding
  • Updated: 2026-11-11T12:01:31Z
Head State Pass summary
7a41be9a4fb1 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-11-11T12:01:31Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0187

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Nov 11, 2026
@M3gA-Mind
M3gA-Mind merged commit 5fd8030 into tinyhumansai:main Nov 11, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants