Rebuild terminal UI and repair aggregate_records migration #3

Merged

kleb merged 2 commits from rebuild-tui into main

2026-06-15 10:26:15 +02:00

kleb commented

2026-06-15 10:11:21 +02:00

Owner

Summary

Non-interactive visual overhaul of the kMarc terminal UI ("calm scaffolding, loud signal": quiet dim structure, colour reserved for meaning), plus a fix for a database migration gap that broke export and report on pre-existing databases. No new runtime dependencies; the compile-to-binary setup is preserved.

Terminal UI

Commands, flags, data, ordering and counts are unchanged - only presentation.

src/ui.ts rewritten as a small design system:
- theme - single source of truth for verdict colour + glyph (auth / disposition / authorization / risk), still readable under NO_COLOR.
- Inset section rules replace the old two-line cyan headings; width-capped banner; subheading(meta); additive share bar; statRow.
- ANSI-safe word wrap - fixes colour being lost when a value wraps (the headline bug); ANSI-aware truncate; width-fitting table (flex-shrink widest-first, per-column narrow:"drop", optional footer row).
- Capability flags: NO_COLOR/FORCE_COLOR, KMARC_ASCII/KMARC_UNICODE, COLUMNS; terminalWidth clamped to [60, 200].
report.ts / cli.ts / mailbox.ts terminal output restyled to the shared vocabulary. Wide report tables transpose to stacked key-value blocks on narrow terminals so no security column is ever dropped.
New tests in tests/ui.test.ts.

Fix: aggregate_records migration

migrate() only ran CREATE TABLE IF NOT EXISTS, so databases created before aggregate_records.policy_override_reasons_json was added never received the column, and export / report failed with no such column. Added a guarded ALTER TABLE migration (mirrors the existing uid_validity migration), covered by tests/repo.test.ts.

Scope note

This branch also carries pre-existing reliability / performance / security hardening that was already in the working tree (aggregate, config, dmarc, forensic, processor, spf, types, util and their tests). It was entangled with the UI edits in the same files, so it is included here rather than split out.

Verification

bunx tsc --noEmit clean
bun test - 81 pass
bun run check:unused (knip) clean
bun run build - binary compiles
Rendered help / summary / report / export across wide, narrow (transpose), ASCII (KMARC_ASCII=1) and NO_COLOR modes; no rendered line exceeds the terminal width at 62 / 72 / 100 columns.

Generated with Claude Code.

## Summary Non-interactive visual overhaul of the kMarc terminal UI ("calm scaffolding, loud signal": quiet dim structure, colour reserved for meaning), plus a fix for a database migration gap that broke `export` and `report` on pre-existing databases. No new runtime dependencies; the compile-to-binary setup is preserved. ## Terminal UI Commands, flags, data, ordering and counts are unchanged - only presentation. - `src/ui.ts` rewritten as a small design system: - `theme` - single source of truth for verdict colour + glyph (auth / disposition / authorization / risk), still readable under NO_COLOR. - Inset `section` rules replace the old two-line cyan headings; width-capped `banner`; `subheading(meta)`; additive share `bar`; `statRow`. - ANSI-safe word wrap - fixes colour being lost when a value wraps (the headline bug); ANSI-aware `truncate`; width-fitting `table` (flex-shrink widest-first, per-column `narrow:"drop"`, optional footer row). - Capability flags: `NO_COLOR`/`FORCE_COLOR`, `KMARC_ASCII`/`KMARC_UNICODE`, `COLUMNS`; `terminalWidth` clamped to [60, 200]. - `report.ts` / `cli.ts` / `mailbox.ts` terminal output restyled to the shared vocabulary. Wide report tables transpose to stacked key-value blocks on narrow terminals so no security column is ever dropped. - New tests in `tests/ui.test.ts`. ## Fix: aggregate_records migration `migrate()` only ran `CREATE TABLE IF NOT EXISTS`, so databases created before `aggregate_records.policy_override_reasons_json` was added never received the column, and `export` / `report` failed with `no such column`. Added a guarded `ALTER TABLE` migration (mirrors the existing `uid_validity` migration), covered by `tests/repo.test.ts`. ## Scope note This branch also carries pre-existing reliability / performance / security hardening that was already in the working tree (aggregate, config, dmarc, forensic, processor, spf, types, util and their tests). It was entangled with the UI edits in the same files, so it is included here rather than split out. ## Verification - `bunx tsc --noEmit` clean - `bun test` - 81 pass - `bun run check:unused` (knip) clean - `bun run build` - binary compiles - Rendered help / summary / report / export across wide, narrow (transpose), ASCII (`KMARC_ASCII=1`) and `NO_COLOR` modes; no rendered line exceeds the terminal width at 62 / 72 / 100 columns. Generated with Claude Code.

kleb added 1 commit

2026-06-15 10:11:21 +02:00

Rebuild terminal UI and repair aggregate_records migration 7e7837464b

Terminal UI (non-interactive visual overhaul; commands, flags, data,
ordering and counts unchanged):
- Rewrite src/ui.ts as a semantic design system: theme (verdict color +
  glyph, single source of truth), inset section rules, width-capped banner,
  ANSI-safe word wrap (fixes color being lost on wrapped values), ANSI-aware
  truncate, width-fitting table (flex-shrink, narrow:drop, optional footer),
  additive share bar, statRow, and capability flags (NO_COLOR,
  KMARC_ASCII/KMARC_UNICODE, COLUMNS; terminalWidth clamped 60-200).
- Restyle report/cli/mailbox terminal output to the shared vocabulary; wide
  report tables transpose to stacked key-value blocks on narrow terminals so
  no security column is dropped.
- Add tests/ui.test.ts.

Fix:
- db.ts: add a guarded migration that adds
  aggregate_records.policy_override_reasons_json to databases created before
  the column existed. export and report previously failed on such databases
  with "no such column". Covered by tests/repo.test.ts.

Also carries pre-existing reliability, performance and security hardening
that was already in the working tree (aggregate, config, dmarc, forensic,
processor, spf, types, util and their tests).

Verified: tsc clean, 81 tests pass, knip clean, binary compiles.

kAgent commented

2026-06-15 10:15:39 +02:00

Collaborator

Codex review

Verdict: 1 medium

Overall risk is moderate; the main concern is that failed message handling can now delete messages that were not successfully imported.

Medium (1)

Avoid deleting previously failed imports — src/mailbox.ts:187
A message that fails during import can be permanently removed from the remote mailbox even though it was not saved locally. The new per-message catch increments failedMessages and continues, but because the delete step for already-processed UIDs batches all UIDs present in the repository, a message whose failed aggregate artifact was partially persisted and whose message row exists can be treated as already processed on the next sync and deleted when deleteRemote is enabled.
Suggested fix: Track failed or incomplete imports separately from successfully processed UIDs, and exclude them from the already-processed remote-delete batch until the import has completed successfully.

Reviewed by Codex at f32bec583c. This comment is conservative and based only on the PR diff, metadata, and supplied repository context.

Est. cost ~$0.36 (70.6k in / 733 out / gpt-5.5).

## Codex review **Verdict:** 1 medium Overall risk is moderate; the main concern is that failed message handling can now delete messages that were not successfully imported. ### Medium (1) - **Avoid deleting previously failed imports** — [`src/mailbox.ts:187`](http://git.kleb.sh/kleb/kMarc/src/commit/f32bec583c06696b95a1633a04ddc88c247fe467/src/mailbox.ts#L187) A message that fails during import can be permanently removed from the remote mailbox even though it was not saved locally. The new per-message catch increments `failedMessages` and continues, but because the delete step for already-processed UIDs batches all UIDs present in the repository, a message whose failed aggregate artifact was partially persisted and whose message row exists can be treated as already processed on the next sync and deleted when `deleteRemote` is enabled. Suggested fix: Track failed or incomplete imports separately from successfully processed UIDs, and exclude them from the already-processed remote-delete batch until the import has completed successfully. _Reviewed by Codex at `f32bec583c`. This comment is conservative and based only on the PR diff, metadata, and supplied repository context._ _Est. cost ~$0.36 (70.6k in / 733 out / gpt-5.5)._

kleb added 1 commit

2026-06-15 10:19:43 +02:00

Serialize import transaction to fix aggregate dedup race f32bec583c

Run the per-message import transaction with BEGIN IMMEDIATE so concurrent
importer processes serialize on the write lock. The aggregate-report dedup
does a check-then-insert (aggregateReportExists then insertAggregateReport);
under a deferred transaction two importers could each read "no existing
report" before either commits and then both insert, double-counting a
retransmitted report. Acquiring the write lock up front makes the second
importer observe the first's committed row and skip.

Addresses PR review (race-prone aggregate report deduplication).

kleb merged commit bfcc62a9b5 into main

2026-06-15 10:26:15 +02:00

kleb deleted branch rebuild-tui

2026-06-15 10:26:15 +02:00

kleb referenced this pull request from a commit

2026-06-15 10:26:15 +02:00

Merge pull request 'Rebuild terminal UI and repair aggregate_records migration' (#3) from rebuild-tui into main