iter-27: fix 5 bugs surfaced by dogfooding 2026-05-10#32
Merged
Conversation
- container logs: drop per-command --format that collided with the
global flag and crashed clap before any work ran.
- DNS zone scan results: tolerate "Records": null while a scan is
in flight via a null-to-empty-vec serde helper.
- pull-zone update --optimizer-classes: send the field as a JSON
string (matches bunny.net's wire format; response side already
tolerated either shape).
- shield API errors: parse the nested {error: {statusCode, errorKey,
message}} envelope and surface the real status + key + message
uniformly across all shield modules.
- db config optimal-single: hide the subcommand and bail with a
clean message — endpoint is broken upstream.
Includes regression tests for each fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR packages five localized dogfooding bugfixes from May 2026, plus comprehensive backlog documentation and iteration planning. Fixes address DNS null-record deserialization panic, Shield API nested error-envelope parsing, container logs clap argument collision, and a broken DB config endpoint. Also includes extensive UX friction documentation and iteration-28 scope for CLI flag/output consistency cleanup. ChangesDogfooding Bugfixes and Planning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
There was a problem hiding this comment.
Pull request overview
This PR bundles several dogfooding-driven correctness fixes across the CLI and API client crates, focusing on preventing panics, tolerating inconsistent upstream payloads, and improving Shield API error reporting.
Changes:
- Removed the
container logssubcommand’s local--formatflag to avoid a clap type-collision, and added parse regression tests. - Made DNS scan results deserialization tolerate
"Records": nullby adding a serde helper and regression tests. - Added Shield API nested error-envelope parsing (with RFC7807 fallback) and wired it into the Shield client, plus unit tests.
- Hid the broken
db config optimal-singlesubcommand and replaced the call with a clean error message. - Added iteration/backlog knowledgebase entries documenting the dogfooding findings.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/snapshots/e2e__cli_container__container_logs_help.snap | Updates CLI help snapshot for container logs after argument changes. |
| src/commands/database.rs | Replaces optimal-single execution with a clean bail message. |
| src/commands/container.rs | Removes per-command --format usage and validates the global output format for log streaming. |
| src/cli.rs | Drops the conflicting container logs --format, hides OptimalSingle, and adds CLI parse regression tests. |
| hoppy-knowledgebase/iterations/iteration-28-dogfooding-ux-polish.md | New iteration planning doc (UX polish follow-up). |
| hoppy-knowledgebase/iterations/iteration-27-dogfooding-bugfixes.md | New iteration doc documenting the bugfix scope and acceptance. |
| hoppy-knowledgebase/backlog/*.md | New backlog entries capturing dogfooding issues. |
| crates/bunny-api-shield/src/types.rs | Adds nested Shield error envelope types + parse_shield_error and associated tests. |
| crates/bunny-api-shield/src/lib.rs | Re-exports new Shield error types/helpers. |
| crates/bunny-api-shield/src/client.rs | Uses parse_shield_error when mapping non-success responses. |
| crates/bunny-api-core/src/types.rs | Applies null_to_empty_vec to DNS scan records and adds regression tests. |
| crates/bunny-api-core/src/serde_helpers.rs | Introduces null_to_empty_vec helper with unit tests. |
Comment on lines
2135
to
2143
| // --- 1. Validate format --------------------------------------------------- | ||
| // Reject --format table: tail output is a live stream, not tabular data. | ||
| let logs_format = match format { | ||
| "text" => LogsFormat::Text, | ||
| "json" => LogsFormat::Json, | ||
| "table" => bail!( | ||
| OutputFormat::Text => LogsFormat::Text, | ||
| OutputFormat::Json => LogsFormat::Json, | ||
| OutputFormat::Table => bail!( | ||
| "`hoppy container logs` does not support --format table; \ | ||
| tail output is not tabular. Use --format text (default) or --format json." | ||
| ), |
Comment on lines
+507
to
+512
| if let Ok(env) = serde_json::from_slice::<ShieldApiErrorEnvelope>(bytes) { | ||
| return Some(anyhow::anyhow!(env)); | ||
| } | ||
| if let Ok(problem) = serde_json::from_slice::<ProblemDetails>(bytes) { | ||
| return Some(anyhow::anyhow!(problem)); | ||
| } |
- container logs: map global --format=table (the default) to text rather than bailing — the previous validation made the command fail on every default invocation. - shield parse_shield_error: only accept the ProblemDetails fallback when at least one RFC 7807 field is present, so non-7807 bodies fall through to status + raw body instead of "error 0: unknown". - knowledgebase: doc consistency tweaks flagged by CodeRabbit (deserialize spelling, sibling/shield typo, db-delete JSON shape, -1 sentinel handling, iter-28 acceptance checkboxes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five independent correctness fixes from the 2026-05-10 dogfooding round:
--formatfield that collided with the global flag;hoppy container logs --app-id …now parses cleanly.null_to_empty_vecserde helper so an in-flight scan returning"Records": nulldeserialises to an empty vec instead of panicking.serialize_withhelper. CLI flag is unchanged.{error: {statusCode, errorKey, message}}shape uniformly across all shield modules; replacesShield API error 0: unknownwith the real status + key + message.#[command(hide = true)]and now bails with a clean message.Each fix has a regression test (unit or wiremock-based).
Test plan
cargo fmtcargo clippy --workspace --all-targets -- -D warningscargo test --workspace --quiet(all green)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
container logs; global--formatnow works and table/text output is accepted.nullrecords during in-progress scans.db config optimal-singlereports upstream unavailability instead of failing.Documentation