fix: harden the emit idempotency cache and surface silent truncations

[jjuanrivvera99]

Summary

Fixes every actionable finding from a full code review of the project, in one pass. The headline fix is the invoices emit idempotency cache, where silent failures could lead to double electronic (fiscal) emission.

Emission cache (commands/emit.go) — the serious one

• Corrupt/unreadable cache no longer reads as empty. loadEmitCache previously did _ = json.Unmarshal and returned an empty map, silently dropping the guard against re-emission. emit now aborts with the cache path and a remediation hint. A missing file is still a normal fresh start.
• Persist after every successful batch, not once at the end. A crash mid-run used to lose every stamped id of the run. If a save fails, emission stops and the error lists the just-stamped ids so they can be recorded before re-running.
• Atomic writes (temp + rename). An in-place os.WriteFile could tear emitted.json on a crash mid-write — which, combined with the first bug, produced exactly the empty-guard scenario.

Note: the review suggested recording ids before the API call. That was deliberately not done — a write-ahead record marks invoices as emitted when the call fails, so they would be silently skipped forever (a missing fiscal emission, arguably worse than a duplicate). Per-batch persistence + loud failures covers the same window without that failure mode.

Silent truncations

• Auto-detected columns (table/CSV): when the 10-column cap drops fields, a note now goes to stderr with the dropped count and a --columns / --output json hint. Stdout stays clean for piping. The cap is now the named constant maxAutoColumns.
• ListAll page cap: the warning was already user-visible at the default log level (the review misread this one); it now also carries a remediation hint.

Edge cases

• generic.go filter collisions: list filters dropped over a flag collision or empty definition are recorded in droppedListFilters and asserted empty by a new registry test — a bad resource definition now fails CI instead of silently losing the filter, while keeping the documented never-panic-at-init behavior.
• api.ID precision: decoding tries Int64 before the float64 normalization path, so ids above 2⁵³ keep full precision.
• AsAPIError now delegates to errors.As, which also walks errors.Join multi-error trees the manual unwrap loop missed (test added).
• Retry loop dead code: removed the unreachable lastErr branch after the loop in client.go; the remaining return is documented as a guard against a negative MaxRetries.

Test plan

• New tests: corrupt-cache unit + integration (emit aborts before any API call), atomic-save leaves no temp files, missing-cache-is-fresh-start, >2⁵³ id precision, errors.Join unwrap, column-cap dropped count + explicit-columns-never-capped, registry collision assertion.
• make check green: gofmt -s, go vet, golangci-lint (0 issues), full test suite.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 63043eab-8267-4866-8052-8e6e4344bf79

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/emit-cache-and-edge-cases

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 55.43478% with 41 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
commands/generic.go	7.1%	11 Missing and 2 partials ⚠️
internal/config/config.go	15.4%	6 Missing and 5 partials ⚠️
commands/emit.go	60.0%	6 Missing and 4 partials ⚠️
commands/auth.go	33.3%	1 Missing and 1 partial ⚠️
internal/api/resource.go	0.0%	2 Missing ⚠️
internal/output/output.go	85.7%	1 Missing and 1 partial ⚠️
internal/api/types.go	94.4%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[jjuanrivvera99]

Second pass on this branch: implemented the test-effectiveness improvements that came out of the suite scan, plus the production bugs they immediately surfaced.

Production fixes (bd56a67)

• Money accepted "NaN"/"Inf" strings via ParseFloat — the value poisons any later json.Marshal of the containing document. Found by the strengthened fuzzer within seconds of its first run; the counterexample is committed as a permanent corpus regression. Both Money and Int now reject non-finite values.
• Int lost precision above 2⁵³ (same float64 round-trip bug as ID); now tries int64 first, for bare and quoted integers.
• import --dry-run and create --draft swallowed json.Marshal errors, risking garbage output or a stale body sent to the API.
• auth status ignored config.Load errors (nil-deref waiting to happen).
• config.Save is now atomic (temp + rename), same treatment as emitted.json.

Test suite (c15f85d)

Baseline: 26 error assertions across 241 test functions; integration fake servers only ever returned 200/404.

• Fuzzers assert value-level properties instead of "doesn't panic": integer literals survive decoding verbatim, Money matches ParseFloat exactly.
• Failure-path integration tests: partial import fails loudly with per-row errors and non-zero exit; failed emit batches stay out of the idempotency cache; a cache save failure stops emission before the next batch (read-only dir, skipped on Windows); unconfirmed delete never sends the DELETE; auth login never persists the token into config.yaml.
• Hostile-response client tests: malformed JSON on 200, HTML error bodies, cancellation mid-backoff, 429 throttle/restore cycle, HTTP-date Retry-After, missing rate-limit headers, all five list wrapper shapes.
• config/auth/output: corrupt YAML errors, atomic save leaves no temp files, locked keyring degrades gracefully, key/value column filtering.
• CONTRIBUTING: resource tests should cover what's unique to the resource (not re-test generic CRUD); documented failure-path and fuzzing expectations.

Deliberate non-changes: toRows dropping non-map rows is pinned by a test rather than changed (by-design normalization); country.go's best-effort save is documented as deliberate; mutation testing stays a periodic manual audit, not CI.

Local: make check green, -race green on touched packages, coverage 83.0%.

[jjuanrivvera99]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Five independent review passes (CLAUDE.md adherence, shallow bug scan, git-history context, prior-PR feedback, in-code comment constraints) produced 9 candidate findings; all scored below the confidence threshold after verification. Highest-scoring (50/100, informational): warnDroppedColumns writes to os.Stderr directly rather than an injectable writer — intentional and documented (stdout must stay clean for piping), but inconsistent with the io.Writer pattern used elsewhere; worth considering in a future cleanup. The remaining findings were pre-existing issues outside this PR's diff or deliberate, documented safety choices (e.g. corrupt-cache hard stop not bypassable by --force).