Conversation
|
This change is part of the following stack: Change managed by git-spice. |
Adds crates/orchestrator/tests/webhook_http.rs covering the full
POST /webhooks/{workflow_id} handler path end-to-end via in-process
axum router (tower::ServiceExt::oneshot — no real TCP).
A NullBackend implements ExecutionBackend with no-ops so AgentManager
can be constructed without tmux/Docker. The handler only uses
state.scheduler so the backend is never invoked.
Tests cover:
- 202 Accepted: no secret, empty body, valid HMAC, GitHub PR event
- 401 Unauthorized: missing signature header, wrong secret, tampered body
- 404 Not Found: unknown workflow ID, workflow in storage but not running
- 400 Bad Request: workflow exists with non-webhook trigger type
- Response shape: 202 has empty body; errors carry {"error":"..."} JSON
Also adds tower = { version = "0.5", features = ["util"] } to
[dev-dependencies] for ServiceExt::oneshot.
Provides end-to-end baseline for the wiring work in #459.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #739 +/- ##
==========================================
+ Coverage 55.66% 55.69% +0.02%
==========================================
Files 126 126
Lines 13759 13763 +4
==========================================
+ Hits 7659 7665 +6
+ Misses 6100 6098 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
👀 Conductor: Awaiting ReviewerPR #739 ( Nudge posted by conductor pipeline sync. |
geoffjay
left a comment
There was a problem hiding this comment.
Review: refactor(common): extract ApiError::status_code helper
Stack position: issue-720 is directly on main with no parent PR
dependency, but git-spice reports (needs restack) — the branch is behind
main and must be rebased before the conductor can merge it.
Code quality: approved
The change is correct, focused, and well-tested. Specific notes:
status_code() method — the variant→status mapping is identical to the
old per-arm match in into_response. No behaviour change.
Internal case — the old code used e.to_string() (on the inner
anyhow::Error); the new code uses self.to_string(). Because Internal is
annotated #[error(transparent)], Display delegates to the inner error, so
the two produce identical strings. Correct.
into_response simplification — going from 7 match arms (each repeating
self.to_string()) to two lines is the right outcome. The match &self /
self.to_string() awkwardness (matching a reference to get the status then
calling to_string on the owner) is gone.
Tests — 7 tests, one per variant, all exercising status_code() directly.
anyhow::anyhow!("boom").into() for Internal is the idiomatic pattern.
Coverage is complete.
Scope — single file, single responsibility, no unrelated changes.
Action required before merge
Rebase onto main:
git-spice branch restack
git-spice branch submitOnce restacked, add merge-queue to trigger the conductor.
refactor(common): extract ApiError::status_code helper
refactor(common): extract ApiError::status_code helper from into_response (closes #720)