Skip to content

fix(comments): require a complete status record - #17

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/768-status-record
Oct 26, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
fix/768-status-record

Conversation

@znerol2

@znerol2 znerol2 commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Summary

The review-history check writes each git grep exit status to a small file and reads it back. The reader accepted a bare leading number, so a write cut short after 1 would read as "no match" and skip the scan.

  • Each status file is now a complete record: <status> done followed by a newline.
  • read_status reads the raw bytes and accepts only that exact form. Anything else fails the check with the existing error.
  • Statuses 0, 1 and higher still go through the existing handling.

The scanner matches the other repos' copies. Only this repo's exclude list differs.

Testing

  • bin/check-review-comments.sh --self-test passes. It plants the partial records 1, 1 do and 1 done without a newline, and expects exit 1. All three fail with exit 0 against a reader that accepts a leading number.
  • bin/check-review-comments.sh gives the same result as before this change.
  • shellcheck bin/check-review-comments.sh is clean.

Refs dmfdeploy/dmfdeploy#768

The scanner records each command's exit status in a file and reads it
back. A write that stopped after the first digit left a bare number,
which read as a valid status and could pass the check unchecked.

Write the status as "<digits> done" with a newline, and accept only that
exact form. The reader keeps the final newline by appending a sentinel
before command substitution strips trailing newlines. Anything else
fails with the existing "could not record the exit status" error. The
self-test plants partial records and expects each one to fail.

Refs dmfdeploy/dmfdeploy#768

Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
@znerol2
znerol2 requested a review from a team as a code owner October 26, 2026 06:29
@github-actions
github-actions Bot enabled auto-merge (rebase) October 26, 2026 06:29

@lkirc lkirc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of e6e47fb: the complete-record parser rejects incomplete status writes while preserving the existing exit-status handling. No blocking findings.

@github-actions
github-actions Bot merged commit fd054ab into main Oct 26, 2026
10 checks passed
@znerol2
znerol2 deleted the fix/768-status-record branch November 5, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants