fix: keep scan history rendering total over workbench payloads

[mldangelo]

Summary

runWorkbench (src/runtime.ts) validates only that the workbench response parsed as JSON and isRecord(result). Nothing checks the shape between the Python workbench and renderScanHistory, and the renderer then cast every field unconditionally. Any drift — a plugin/database version skew, a partially migrated sqlite row, a hand-edited workbench.sqlite3 — surfaced as an unhandled TypeError with a stack trace rather than a usable view or an actionable error.

Three crashes confirmed by calling the renderer directly:

Payload	Before
finding with severity absent	TypeError: undefined is not an object (evaluating 'severity.level')
finding with severity: null	same — typeof null === "object"
scans row without progress	TypeError: undefined is not an object (evaluating 'scan.progress.status')
knownSince: "not-a-timestamp"	RangeError: Invalid time value from Intl.DateTimeFormat.format

Scope — please read before reviewing as a severity call

These are not reachable with the plugin as currently bundled. I checked _bundled_plugin/scripts/workbench_db.py: severity is always emitted as {level: ...} and progress is always populated, and every timestamp comes from now() = datetime.now(timezone.utc).isoformat(), so knownSince is always ISO-8601. The renderer also only runs on a TTY with --format toon, so --format json was never affected.

So this is drift resilience, not a live bug. I'm raising it because the class is one bad row away, and a read-only history view crashing is a poor failure mode for a command whose whole job is to tell you what happened.

Approach: make the renderer total, don't validate at the boundary

I considered adding a schema check in runWorkbench instead. I did not, for two reasons: the workbench payload has a large optional surface, so a schema would carry real risk of rejecting valid data and breaking working installs; and a display layer's correct response to a field it cannot read is to omit it, not to refuse the whole view.

So every read goes through three small accessors — record(), records(), text() — and the behavior is:

• missing/null severity → blank badge, default color
• missing progress.status → UNKNOWN
• unparsable knownSince → the raw value, bounded to 32 chars (information preserved rather than dropped)
• unrecognized severity → sorts below the known levels. Previously Object.keys(SEVERITY_COLORS).indexOf(...) returned -1, which sorted an unknown severity above CRITICAL — a separate real ordering bug fixed here via severityRank().

Testing / QA instructions

Baseline before this branch: 470 pass / 6 skip / 0 fail. After: 475 pass / 6 skip / 0 fail (five added tests).

cd sdk/typescript          # run pnpm from here, not the repo root
CI=true pnpm install --frozen-lockfile
CI=true pnpm run types
CI=true pnpm run test
CI=true pnpm run format
CI=true pnpm run build

The regression guard that matters most

CI=true bun test --timeout 30000 ./tests-ts/scan-history-renderer.test.ts

Expect 11 pass / 0 fail. The six pre-existing tests in this file assert exact rendered text (via stripVTControlCharacters) across compare/list/show/match-all, narrow and wide widths, and dim-styling rules. They are unmodified and still pass, which is the proof that output for well-formed payloads did not change. If any of those six fail, the accessors changed real behavior and the diff is wrong.

New coverage — scan history renderer resilience

1. renders comparison findings with a missing or null severity — both shapes; asserts the title and src/search.ts:10 still render.
2. sorts an unrecognized severity below every known severity — asserts CRITICAL appears before the unknown-severity finding in the output. Fails on main (unknown sorts first).
3. lists scans that carry no progress record — asserts the scan id renders and status shows UNKNOWN.
4. falls back to the raw value for an unparsable knownSince — asserts Known since not-a-timestamp rather than a RangeError.
5. renders every command from an empty payload — {} through all four commands, asserting none throw. This is the broad net for the casts I did not individually reproduce (result["coverage"], result["findings"], result["severityCounts"], basename(result["targetPath"]), basename(result["repository"])).

Verifying the crashes exist on main

git stash && cd sdk/typescript && bun -e '
import { renderScanHistory } from "./src/scan-history-renderer.js";
try { renderScanHistory({ beforeScanId:"b", afterScanId:"a", coverage:{afterCompleteness:"complete"},
  summary:{resolved:1}, findings:[{ status:"resolved", title:"x", locations:[{path:"a.ts"}] }] }, "compare"); }
catch (e) { console.log("CRASH:", e.constructor.name, e.message); }'
# then: git stash pop

Note for maintainers

This file is also touched by PR "fix: align the severity badge column in finding tables". The two diffs are in different regions (accessors + helper functions here; badge padding and its dependent indents there) but both are small, so whichever lands second may need a trivial rebase.

[mldangelo-oai]

@codex review Please review exact head 0282909, focusing on total scan-history rendering over malformed optional workbench values, safe presentation, and the current-main merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0282909557

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Validate knowledge-base paths before mapping

When a malformed or version-skewed workbench payload supplies the optional recipe.knowledgeBasePaths as a nonempty string (or an object with a truthy length) instead of an array, the length check passes and .map(...) throws a TypeError, aborting the entire history show rendering. Normalize this field with an array/type guard, as is already done for findings and matches, so one malformed optional value cannot crash the view.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Avoid rendering missing fields as undefined

When a malformed show payload omits optional scalar fields such as scanId or mode, the new total-rendering path no longer throws but passes those values to clean, which turns them into the literal text undefined; an empty payload consequently displays · undefined and UNKNOWN · undefined rather than degrading to blank or UNKNOWN values. The same pattern truncates absent comparison IDs to undefine, so these remaining scalar reads should use the new fallback-aware accessor before presentation.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Sanitize severity-count keys before output

When a malformed or hand-edited workbench database contains a severity value with terminal control sequences, severityCounts exposes that value as an object key and this normalization accepts it unchanged; the rendering loop then uppercases the key and interpolates it without calling clean. On the TTY-only TOON path, a key such as evil\u001b[2J\u001b[H therefore emits active clear-screen and cursor-positioning escapes, so severity keys need the same sanitization as counts and other workbench-derived labels.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Security review completed. No security issues were found in this pull request.

Reviewed commit: 0282909557

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.