fix(email,api): AUTH PLAIN fallback for SMTP + expose input/error in workflow runs list#6293
Conversation
…workflow runs list
email.py: smtp.login() uses AUTH LOGIN which some providers
(e.g. Mailgun) reject. Try LOGIN first; fall back to AUTH PLAIN
on SMTPAuthenticationError so servers that only advertise PLAIN
still work.
workflow.rs: add input (the parameters used) and error to
GET /api/workflows/{id}/runs so the dashboard shows what
parameters were used and why a run failed.
github-actions
Bot
added
size/S
…estart update_workflow() only updated the in-memory HashMap but never wrote the JSON file. Agents assigned to steps via the dashboard would disappear on daemon restart because the old file still had the previous state. Now mirrors register()'s atomic write.
github-actions
Bot
added
the
area/kernel
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
Two issues found.
1. Missing integration test for list_workflow_runs response shape change
(crates/librefang-api/src/routes/workflows/workflow.rs)
The list_workflow_runs handler now includes "input" and "error" in the JSON response object. Per CLAUDE.md (§ "MANDATORY: Integration Testing"), route shape changes require a #[tokio::test] against TestServer in the matching tests/*.rs file. The test should spawn the router, fire GET /api/workflows/{id}/runs, and assert both fields appear in the response (and that "error" is populated on a failed run). Without it the serialization path and field names are not regression-tested.
2. Security-rationale comment removed from the STARTTLS check
(sdk/python/librefang/sidecar/adapters/email.py, STARTTLS branch)
The deleted block explained why STARTTLS is required (credential leakage over plaintext TCP, parity with lettre's starttls_relay semantics in the Rust adapter). The raise RuntimeError message says "refusing to send" but doesn't convey the threat model or the cross-language parity constraint. Per CLAUDE.md the comment qualifies as "hidden constraint / behaviour that would surprise a reader" — the STARTTLS requirement is non-obvious to someone adding a new SMTP provider. Please restore it (or an equivalent sentence) on the raise RuntimeError line.
Generated by Claude Code
|
Reviewed the diff. The email AUTH PLAIN fallback looks correct — SASL PLAIN 1. (should fix)
This is pre-existing, but this PR adds 2. (nice to fix) The new persistence block runs 3. Description / tests The body says "Two small fixes", but the diff has three changes — the |
2 participants
Two small fixes:
1. Email adapter: AUTH PLAIN fallback
Python's smtplib.login() uses AUTH LOGIN but some providers
(e.g. Mailgun) reject LOGIN and only accept PLAIN. Add a
fallback: try LOGIN first, retry with AUTH PLAIN on auth failure.
Non-breaking — standard servers use the fast path.
2. Workflow runs list: expose input and error
GET /api/workflows/{id}/runs now returns "input" (parameters used)
and "error" (failure reason) in addition to existing fields.
Gives the dashboard enough data to show what params were used and
why a run failed without requiring a second request to the detail
endpoint.
Verification: