Fail predictions when output uploads fail

[michaeldwan]

Coglet can report status: "succeeded" with output: null after a file upload times out or the upload server rejects it. Failed uploads can also disappear from otherwise successful list outputs.

In crates/coglet/src/orchestrator.rs, upload tasks return their errors to prediction completion. Any failed upload or upload task marks the prediction failed with an error. Cancellation aborts pending uploads and preserves the canceled status.

Uploads also include X-Prediction-Id, so Director handles them through its normal upload path rather than its legacy redirect fallback. The 25-second timeout and retry policy are unchanged.

Regression tests exercise HTTP rejection and the real 25-second timeout through the orchestrator event loop against a local upload server. They also cover partial list output, cancellation, and successful uploads.

[ask-bonk]

I've completed a thorough review of PR #3197. Let me summarize my analysis:

Changes reviewed:

• crates/coglet/src/orchestrator.rs (+361/-52): Upload tasks now return Result<(), String> wrapped in AbortOnDropHandle; the Done handler propagates upload errors to set_failed, aborts uploads on cancellation via a biased select, and the no-uploads sync path now checks is_canceled() before set_succeeded. Also adds X-Prediction-Id header to uploads and a distinct timeout error message.
• Cargo.toml/Cargo.lock: tokio-util gains the rt feature (needed for AbortOnDropHandle); lock change is consistent.

Key correctness checks I verified:

• Upload failure → set_failed (with no URL/token leak via without_url()), and partial list outputs no longer silently succeed. ✓
• Cancellation: the biased select on cancel_token.cancelled() aborts in-flight uploads (dropping the join_all drops the AbortOnDropHandles) and preserves the canceled status; the is_canceled() check in the sync path correctly handles the case where the orchestrator's cancel handler already removed+aborted the uploads before Done arrives. ✓
• Ordering: upload tasks append output before returning Ok, and the join task awaits all handles before set_succeeded, so no race with take_outputs. ✓
• Poisoned-mutex edge cases all funnel through try_lock_prediction, which sets failed. ✓
• No API/schema/IPC protocol changes; tests are deterministic (the 25s real-timeout test is slow but bounded and exercises the intended behavior).

I found no actionable defects in the logic, error handling, or tests.

LGTM

github run

[ask-bonk]

LGTM

github run