fix(automation): poll artifact_glob after wait_for_idle settles (fixes #120)

[bketelsen]

Closes #120.

What

Replaces the single-shot sleep(2s) + capture_artifact() in the wait_for_idle = true post-step block with a bounded polling loop (1s cadence, window = max(wait_timeout_secs / 30, 5s)..60s).

Why

wait_for_agent_idle returns completed as soon as the runtime adapter detects a completion signal (Claude Code emits this at end-of-turn). That can fire before the agent's last file write is flushed to disk — or before the agent has even started writing the file referenced in its final response. Two seconds of slack isn't always enough.

The artifact-polling-mode branch (wait_for_idle = false) already implements this pattern at src/automation/mod.rs:1080-1255. This PR aligns the wait_for_idle = true path with that proven approach.

Test

• cargo check — green
• cargo test --bin tt automation::tests — 34/34 pass
• Manual: was reproducing wait_for_idle = true + artifact_glob race: post-step capture is single-shot, fails when agent finishes faster than the filesystem flushes #120 reliably in a sandbox before the fix; the patched binary picks up the artifact correctly when the agent writes DESIGN.md shortly after the completion signal.

Backward compatibility

Pure widening: anything that succeeded under the old 2s sleep still succeeds (the loop's first iteration is the same check). Anything that failed under the 2s sleep now gets up to 60s to materialize.

Repro details and root-cause writeup in #120.

Summary by CodeRabbit

• Bug Fixes

  • Improved artifact capture reliability by replacing a single fixed delay with bounded polling that retries until success or timeout; the polling window now adapts to step timeouts (clamped to sensible limits) and is reused across post-step capture paths, reducing timing-related failures.

• Tests

  • Added unit tests covering late artifact availability and polling-window clamping behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d8719aaf-631b-42b8-941b-cbe320225be9

📥 Commits

Reviewing files that changed from the base of the PR and between 24e0424 and 770d9c0.

📒 Files selected for processing (1)

• src/automation/mod.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/automation/mod.rs

---

📝 Walkthrough

Walkthrough

Prompt-step post-idle artifact capture was changed from a fixed 2s sleep + single capture attempt to a bounded polling retry. A per-step poll window (clamped to 5–60s) is computed once and used across early-success and normal post-step capture paths; new tests cover polling and clamp behavior.

Changes

Post-idle artifact polling

Layer / File(s)	Summary
Compute poll window src/automation/mod.rs	Compute post_idle_poll_secs once per prompt step from step_wait_timeout via post_idle_artifact_poll_secs(wait_timeout_secs) (clamped 5–60s).
Introduce poll helper src/automation/mod.rs	Add capture_artifact_with_poll(pre_snap, pattern, name, deadline_secs, poll_interval_secs) which repeatedly calls capture_artifact until success or deadline, sleeping between attempts.
Early-success (implement_code) src/automation/mod.rs	Replace sleep+single capture in implement_code early-success exit with capture_artifact_with_poll(post_idle_poll_secs, 1s); on failure record step failure and abort.
Retry early-success (implement_code retry) src/automation/mod.rs	Apply same polling capture in the retry early-success path.
General post-step capture src/automation/mod.rs	Replace the 2s settle sleep + single capture_artifact in the post-step capture block with capture_artifact_with_poll(post_idle_poll_secs, 1s); store artifact on success or fail step on error.
Tests src/automation/mod.rs (tests)	Add unit tests for capture_artifact_with_poll waiting for late artifact materialization and for post_idle_artifact_poll_secs clamping behavior.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Agent
  participant Automation
  participant Filesystem
  note over Agent,Filesystem: Agent may emit completion before file is flushed
  Agent->>Automation: runtime completion signal (wait_for_idle satisfied)
  Automation->>Automation: compute post_idle_poll_secs
  Automation->>Filesystem: attempt capture_artifact (via capture_artifact_with_poll)
  alt artifact not present
    Automation->>Filesystem: sleep 1s, retry capture
    loop until deadline
      Automation->>Filesystem: attempt capture_artifact
    end
  end
  Filesystem-->>Automation: artifact path (on success) / error (on deadline)
  Automation->>Automation: store artifact or mark step failed

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: artifact pipeline — typed artifact flow between workflow stages (v0.7.0) #111: Modifies src/automation/mod.rs artifact-capture logic; likely directly related to this polling-based fix.

Poem

I nibble on code, hop through the night,
Polling for crumbs until they delight.
A patience of seconds, a bounded small race,
Files land at last — I celebrate with a pace! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: replacing a single-shot artifact capture with polling after wait_for_idle completes, and correctly references the issue being fixed.
Description check	✅ Passed	The description covers the key aspects: what changed (polling instead of single-shot), why (filesystem flush race), testing results, and backward compatibility. The versioning section is incomplete (no version selected), but other critical sections are present.
Linked Issues check	✅ Passed	The PR addresses all coding requirements from issue #120: implements polling window for post-step artifact capture with 1s cadence and max(wait_timeout_secs/30, 5s..60s) bounds, aligns with existing polling-mode approach, and includes unit tests verifying late artifact materialization and clamping behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the #120 race condition: implementing capture_artifact_with_poll, computing post_idle_poll_secs per prompt step, applying polling in three artifact-capture paths, and adding related unit tests. No extraneous changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/automation/mod.rs`:
- Around line 1714-1745: The polling loop that retries capture_artifact with a
bounded deadline should be extracted into a small helper (e.g.,
poll_capture_artifact or try_capture_with_poll) that accepts (pre_snap,
expanded_pattern, art_name, post_idle_poll_secs, optionally poll_interval) and
returns Result<PathBuf, Error>; implement its logic using
step_wait_timeout-derived post_idle_poll_secs, a 1s poll_interval, poll_deadline
and poll_start as in the new block, retrying capture_artifact until success or
deadline. Replace the inline loop in the new post-idle block with a call to this
helper and also call the same helper from both early-success implement_code
artifact-capture branches (the places that currently do a single
capture_artifact() after a fixed 2s sleep) so all three paths use the same
bounded polling behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c126a42f-c5f0-4929-aeb6-80f6116cf794

📥 Commits

Reviewing files that changed from the base of the PR and between 54a2da0 and 5884891.

📒 Files selected for processing (1)

• src/automation/mod.rs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/automation/mod.rs`:
- Around line 1565-1586: The current early-exit block uses chained `&& let
Ok(...)` patterns for `capture_artifact_with_poll` and `store_artifact_output`
(referenced symbols: artifact_pre_snapshot, artifact_name,
capture_artifact_with_poll, store_artifact_output, output_files, outputs) which
swallows failures and still lets the step be recorded as successful; change this
so that when an artifact is required (artifact_pre_snapshot and artifact_name
present) any error from `capture_artifact_with_poll` or `store_artifact_output`
is not ignored but causes the step to fail or the error to be returned: replace
the chained `&& let Ok(...)` with explicit match/if let handling that on Err
either returns/propagates the error or sets the step status to failure, and only
inserts into `output_files`/`outputs` when both calls succeed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b0d9f3e0-af4c-47de-97b0-05c3b9fa541a

📥 Commits

Reviewing files that changed from the base of the PR and between 5884891 and 24e0424.

📒 Files selected for processing (1)

• src/automation/mod.rs

[nutt-adam]

Superseded and landed via #124 so the required repository CI could attach normally. #124 preserves Brian's original fix plus the CodeRabbit follow-ups, passed CI/CodeQL/CodeRabbit, and has been merged.