it would be good to exercise this as part of test.bash integration test as well

Approve. Small, focused, well-tested addition that closes a real gap — fda logs only surfaces runtime output, so users currently have to spelunk through the pipeline JSON for SQL/Rust diagnostics. Splitting this into a dedicated program errors subcommand is the right call.

format_program_errors is straightforward and the four unit tests (empty, SQL with snippet, Rust stdout+stderr, system error) cover every branch including the warning vs. error severity flip and the trailing-newline normalisation. Good shape: pure function over &ProgramError, easy to extend if more fields land.

Two non-blocking notes:

1. crates/fda/src/main.rs — Errors arm: if program_error is None the command exits 1 with "did not include program errors". That's the safe choice for a pre-UPGRADE_NOTICE server, but for a current server a fully-compiled pipeline can legitimately return program_error: None (no diagnostics to report). Consider treating None the same as format_program_errors does for an empty struct ("No compilation errors or warnings.\n", exit 0). Otherwise scripting fda program errors X && deploy will spuriously fail.

2. format_program_errors: severity is a &str chosen from if message.warning { "warning" } else { "error" }. Worth a quick sanity check that sql.exit_code == 0 and all messages flagged warning: true is a valid state — if so, the header "SQL compiler exit code: 0" followed by a "warning:" line reads cleanly, which is what you want.

Nit: the snippet branch prints snippet directly with push_str; if a snippet ever contains a stray { it's still safe because you're not running it through format!. Good.

Re-APPROVE on f586b88 → 31bf0f6.

Apply formatter to program error snippets collapses the nested if let Some(snippet) = &message.snippet { if !snippet.is_empty() { … } } into a let-chain — pure 2024-edition formatting cleanup, semantics identical.

Test program error diagnostics in fda script adds the integration coverage that was missing from the earlier rounds: creates pbad with an intentionally-invalid SELECT invalid, polls fda program status pbad for up to 60s waiting for the SqlError to surface, then exercises both human and JSON output paths of fda program errors. The 60-second cap is consistent with the rest of the script's pattern, the JSON assertion uses jq -e so a zero-length messages array exits non-zero (test fails loudly), and the cleanup path adds fda delete --force pbad to the top-of-script cleanup block as well — no leak risk on re-run. Nice.

No blockers; APPROVE stands.

Re-APPROVE on 31bf0f6a4e. Two new commits since prior approval at f586b882e0:

1. e0a907df9b — formatter pass folding the nested if let Some(snippet) = ... { if !snippet.is_empty() { ... } } into a let-chain. Matches the rustfmt complaint Mihai flagged.
2. 31bf0f6a4e — adds an integration leg to test.bash per Mihai's ask: creates pbad with SELECT invalid, polls up to 60s for SqlError, then exercises both the text formatter (fda program errors pbad | grep -i "error:") and the JSON path (fda --format json program errors pbad | jq -e '.sql_compilation.messages | length > 0'). Cleanup at the top + bottom of test.bash properly removes pbad and bad-program.sql so reruns stay idempotent.

Two small nits, non-blocking:

• The poll uses grep -q "SqlError" against fda program status, then immediately re-runs fda program status pbad | grep "SqlError" as an assertion. The second invocation is functionally the same as breaking out of the loop having seen it once; you can drop it. As written, if the 60s window expires and status flips to RustError between the two reads, the loop exits via the for end (no break) and the assert fails for a different reason. Cleaner: add an explicit else echo "timeout"; exit 1 after the loop body.
• jq -e '.sql_compilation.messages | length > 0' is a good shape check but doesn't pin that error_type / message strings are non-empty. A follow-up could assert (.sql_compilation.messages[0].message | length) > 0 to catch a regression where the body is dropped but the array remains.

Nice follow-through on the maintainer feedback. Good to merge once CI is green.