audit: stop re-asking about findings already decided, make the WARNING git-config tier ackable, and colour the review #95

2026-08-06T20:39:29-07:00

dejima-agent commented

2026-08-06 20:39:29 -07:00

Summary generated by LLM agent

Closes #65. Closes #45.

The headline bug was not what it looked like

#65 reports that exact approvals "are not being recorded". They were being recorded, correctly, the whole time — I reproduced it and read the baseline file after the first ack:

safe .git/hooks/pre-commit 79554d38...

ack simply never consulted the baseline when deciding what to show. It walks the report file top to bottom and presents every line in it, so a second run re-presented everything and appended a duplicate entry for each. Worth stating plainly because it changes the fix: nothing was wrong with what got written, and a fix aimed at the writing would have found nothing to correct.

It now checks each finding against the baseline before displaying it, exactly as compare does when it writes a report.

A second way the same complaint appears

Two reports can list the same finding. Approving it in the first left the second permanently -CRITICAL.txt, so the launcher warned about a report whose every finding had been dealt with — forever, with no way to clear it. The rename off -CRITICAL.txt now also runs on a pass that records nothing new, so a report covered entirely by earlier decisions stops counting as unacknowledged.

#45: the finding that could not be acknowledged

A WARNING-tier git config change had no ack arm to derive an id from, and neither compare branch that emits one consulted the baseline. It was the only class of finding with no way to curate it at all, so keys that some tools rewrite on every run re-fired every session and pushed real WARNINGs out of the capped list.

Both ends are fixed, and the safety argument is the one the issue laid out — it holds by construction rather than by care taken here. id_severity still classifies an exec key or any alias as CRITICAL whatever wrote the baseline entry, and baseline_suppresses still refuses a spam entry against a CRITICAL surface at read time. I verified both directions: [i] is refused on a CRITICAL git config surface, and a hand-written spam gitconfig:core.pager does not suppress.

Approve vs. ignore, which is where the prompt was failing

You asked what the difference was, and the honest answer is that the prompt was hiding it:

	writes	means
approve	`safe <id> <sha>`	this exact content is fine; any other content re-fires
ignore	`spam <id>`	this surface is fine, whatever it later contains

The WARNING prompt said a bare [a] approve, which reads identically to [s] always ignore sitting next to it. Both prompts now state what they do to future findings, and the noun follows the surface so a git config key is not called a file:

[a] approve this exact content  [i] ignore this key from now on  [s] skip (ask again next time)  [q] quit

Skip is [s] as asked, and the permanent option moved to [i].

Findings that cannot be acknowledged

After #45 that is only a cross-project write. There is nothing to record for one, so a reviewed <report> line marks a report walked to the end, and its un-ackable findings are not presented again for that report. A later session that does it again produces a new report and tells you — no signal is permanently lost. Quitting does not mark it, since the report was not walked.

Colour

A small bin/dejima-color-lib.sh that both bin/dejima and bin/dejima-audit source, so the launcher's warning and the review it sends you to agree about what red means.

severity on the tier, the file or key picked out of the label around it
dim on the audit's own framing, so it stops running together with the content between it — the specific complaint in #65
green/red on a retained diff
in the launcher: the tier, the count, the run ids, the command and each keystroke

Off unless the stream being written to is a terminal, and off for NO_COLOR and TERM=dumb. That matters beyond politeness: reports and piped output are read back by machines, this suite included. compare writes to stderr and ack to stdout, so each decides its palette from its own stream. There are tests for both directions — no escapes when piped, escapes present under a pty.

`[a] review now`

The launcher knew the findings were unacknowledged and then asked the operator to go run a command in another terminal, at the one moment when nothing is waiting on them — the container has not started. It now offers [a], runs the review in place, and re-checks. It loops, because ack walks one report per run.

Tests

7 new in audit.bats, 3 in static.bats. The four load-bearing ones were confirmed to fail against the previous dejima-audit and pass against this one. Four existing tests changed, each for a reason the change created:

two asserted the old refusal wording and sent s for ignore
one was written against a changed git config as an example of an un-ackable finding, which is exactly what #45 makes ackable — re-pointed at a cross-project write, which is the class that still has no id
the rotation test counted every entry in the report directory with ls | wc -l; a git config WARNING now retains a diff, so it leaves a <report>.d sidecar beside the report. It counts *.txt now.

That last one is a real behaviour change worth naming: this tier of finding now stores retained content where it previously stored none, under the same budget, denylist and secret-withholding rules as every other retained diff.

Suites: audit 148, static 202, config 94, project 132, providers 64. All green.

Docs

Deliberately untouched, per the README/docs rewrite in flight. What that rewrite will want to pick up:

the keymap and, more importantly, the approve-vs-ignore distinction — the thing that was unclear here is worth a sentence in prose, not just in the prompt
that a WARNING git config finding is now curatable (#45), since docs/security.md's baseline discussion predates it
the reviewed verb, if the baseline file format is ever documented — it is not today

**Summary generated by LLM agent** Closes #65. Closes #45. ## The headline bug was not what it looked like #65 reports that exact approvals "are not being recorded". They were being recorded, correctly, the whole time — I reproduced it and read the baseline file after the first `ack`: ``` safe .git/hooks/pre-commit 79554d38... ``` `ack` simply never consulted the baseline when deciding what to **show**. It walks the report file top to bottom and presents every line in it, so a second run re-presented everything and appended a duplicate entry for each. Worth stating plainly because it changes the fix: nothing was wrong with what got written, and a fix aimed at the writing would have found nothing to correct. It now checks each finding against the baseline before displaying it, exactly as `compare` does when it writes a report. ## A second way the same complaint appears Two reports can list the same finding. Approving it in the first left the second permanently `-CRITICAL.txt`, so the launcher warned about a report whose every finding had been dealt with — forever, with no way to clear it. The rename off `-CRITICAL.txt` now also runs on a pass that records nothing new, so a report covered entirely by earlier decisions stops counting as unacknowledged. ## #45: the finding that could not be acknowledged A WARNING-tier git config change had no `ack` arm to derive an id from, and neither `compare` branch that emits one consulted the baseline. It was the only class of finding with no way to curate it at all, so keys that some tools rewrite on every run re-fired every session and pushed real WARNINGs out of the capped list. Both ends are fixed, and the safety argument is the one the issue laid out — it holds by construction rather than by care taken here. `id_severity` still classifies an exec key or any alias as CRITICAL whatever wrote the baseline entry, and `baseline_suppresses` still refuses a `spam` entry against a CRITICAL surface at **read** time. I verified both directions: `[i]` is refused on a CRITICAL git config surface, and a hand-written `spam gitconfig:core.pager` does not suppress. ## Approve vs. ignore, which is where the prompt was failing You asked what the difference was, and the honest answer is that the prompt was hiding it: | | writes | means | |---|---|---| | approve | `safe <id> <sha>` | this **exact content** is fine; any other content re-fires | | ignore | `spam <id>` | this **surface** is fine, whatever it later contains | The WARNING prompt said a bare `[a] approve`, which reads identically to `[s] always ignore` sitting next to it. Both prompts now state what they do to future findings, and the noun follows the surface so a git config key is not called a file: ``` [a] approve this exact content [i] ignore this key from now on [s] skip (ask again next time) [q] quit ``` Skip is `[s]` as asked, and the permanent option moved to `[i]`. ## Findings that cannot be acknowledged After #45 that is only a cross-project write. There is nothing to record for one, so a `reviewed <report>` line marks a report walked to the end, and its un-ackable findings are not presented again for that report. A later session that does it again produces a new report and tells you — no signal is permanently lost. Quitting does not mark it, since the report was not walked. ## Colour A small `bin/dejima-color-lib.sh` that both `bin/dejima` and `bin/dejima-audit` source, so the launcher's warning and the review it sends you to agree about what red means. - severity on the tier, the file or key picked out of the label around it - dim on the audit's own framing, so it stops running together with the content between it — the specific complaint in #65 - green/red on a retained diff - in the launcher: the tier, the count, the run ids, the command and each keystroke Off unless the stream being written to is a terminal, and off for `NO_COLOR` and `TERM=dumb`. That matters beyond politeness: reports and piped output are read back by machines, this suite included. `compare` writes to stderr and `ack` to stdout, so each decides its palette from its own stream. There are tests for both directions — no escapes when piped, escapes present under a pty. ## `[a] review now` The launcher knew the findings were unacknowledged and then asked the operator to go run a command in another terminal, at the one moment when nothing is waiting on them — the container has not started. It now offers `[a]`, runs the review in place, and re-checks. It loops, because `ack` walks one report per run. ## Tests 7 new in `audit.bats`, 3 in `static.bats`. The four load-bearing ones were confirmed to fail against the previous `dejima-audit` and pass against this one. Four existing tests changed, each for a reason the change created: - two asserted the old refusal wording and sent `s` for ignore - one was written against a changed git config as an example of an un-ackable finding, which is exactly what #45 makes ackable — re-pointed at a cross-project write, which is the class that still has no id - the rotation test counted every entry in the report directory with `ls | wc -l`; a git config WARNING now retains a diff, so it leaves a `<report>.d` sidecar beside the report. It counts `*.txt` now. That last one is a real behaviour change worth naming: this tier of finding now stores retained content where it previously stored none, under the same budget, denylist and secret-withholding rules as every other retained diff. Suites: audit 148, static 202, config 94, project 132, providers 64. All green. ## Docs Deliberately untouched, per the README/docs rewrite in flight. What that rewrite will want to pick up: - the keymap and, more importantly, the approve-vs-ignore distinction — the thing that was unclear here is worth a sentence in prose, not just in the prompt - that a WARNING git config finding is now curatable (#45), since `docs/security.md`'s baseline discussion predates it - the `reviewed` verb, if the baseline file format is ever documented — it is not today

dejima-agent added 1 commit 2026-08-06 20:39:30 -07:00

audit: stop re-asking about findings already decided, and colour the review

tests / sandbox (proxy) (pull_request_target) Successful in 2m7s

Details

tests / runner-env (sandbox) (pull_request_target) Successful in 1m26s

Details

tests / sandbox (static) (pull_request_target) Successful in 3m32s

Details

tests / runner-env (host-docker) (pull_request_target) Successful in 1m23s

Details

tests / sandbox (project) (pull_request_target) Successful in 5m24s

Details

tests / sandbox (providers) (pull_request_target) Successful in 4m0s

Details

tests / sandbox (remote) (pull_request_target) Successful in 4m44s

Details

tests / host-docker (pull_request_target) Successful in 4m59s

Details

tests / sandbox (config) (pull_request_target) Successful in 5m28s

Details

tests / sandbox (audit) (pull_request_target) Successful in 8m17s

Details

e7e6a57dc3

Closes #65 and #45.

The headline bug was not what it looked like. Approvals were being written
correctly the whole time - `ack` simply never consulted the baseline when
deciding what to SHOW, so every later run re-presented the same CRITICALs
and appended a duplicate entry for each. It reads from the outside as
"exact approvals are not being recorded".

Two reports can also list the same finding, and approving it in the first
left the second permanently -CRITICAL.txt: bin/dejima then warned about a
report whose every finding had been dealt with, forever. The rename now runs
on a pass that records nothing new, so a report covered entirely by earlier
decisions stops counting as unacknowledged.

#45: a WARNING-tier git config change could not be acknowledged at all - no
`ack` arm derived an id for it, and neither branch of `compare` that emits
one consulted the baseline. It was the only class of finding with no way to
curate it, so keys some tools rewrite on every run re-fired every session
and pushed real WARNINGs out of the capped list. Safe by construction rather
than by care taken here: id_severity still calls an exec key or any alias
CRITICAL whatever wrote the entry, and baseline_suppresses still refuses a
`spam` entry against a CRITICAL surface at read time.

Findings that cannot be acknowledged at all - now only a cross-project write
- are presented once per report rather than on every run. There is nothing
to record for one, so a `reviewed <report>` line marks the report walked.
Only an un-ackable finding is governed by it; skip still means "ask again"
for everything else, which is the whole difference between skip and ignore.
Quitting does not mark it, since the report was not walked.

The keys: skip is [s], and the permanent option is [i]. Both prompts now say
what they do to future findings, because the difference between them was
invisible - approve pins this exact content and re-fires on any other,
ignore blankets the surface whatever it later contains, and the WARNING
prompt used to offer a bare "approve" that read as the same thing. The noun
follows the surface, so a git config key is not called a file.

Colour, from a small lib both bin/dejima and bin/dejima-audit source so the
launcher's warning and the review it sends you to agree about what red
means: severity on the tier, the file or key picked out of the label around
it, dim on the audit's own framing so it stops running together with the
content between it, and green/red on a retained diff. Off unless the stream
being written to is a terminal, and off for NO_COLOR and TERM=dumb - reports
and piped output are read back by machines, this suite included.

And the launcher's warning offers [a] to review right there. It already knew
the findings were unacknowledged and then asked the operator to go and run a
command in another terminal, at the one moment when nothing was waiting on
them: the container has not started yet. It loops, because `ack` walks one
report per run.

Tests: 7 new in audit.bats and 3 in static.bats, the four load-bearing ones
confirmed to fail against the previous dejima-audit. Suites green - audit
148, static 202, config 94, project 132, providers 64.

Docs are deliberately untouched: a README/docs rewrite is in flight, and
this belongs in that rather than in the version being replaced.

branderson approved these changes 2026-08-06 23:39:41 -07:00

branderson merged commit b71b0c1dda into main

2026-08-06 23:39:49 -07:00

branderson referenced this issue from a commit

2026-08-06 23:39:51 -07:00

Merge pull request 'audit: stop re-asking about findings already decided, make the WARNING git-config tier ackable, and colour the review' (#95) from audit-ux into main

dejima-agent referenced this issue from a commit

2026-08-07 00:35:57 -07:00

docs: pick up what #95 changed about acknowledging findings

dejima-agent referenced this pull request

2026-08-07 00:41:46 -07:00

README: say what dejima is for, and what it does not guarantee #96