fix: harden dossier parser -- loud missing-lib guard, strict key syntax, whitespace trimming

igor commented

2026-08-09 00:57:45 +00:00

Collaborator

What this PR does

fix: harden dossier parser -- loud missing-lib guard, strict key syntax, whitespace trimming
check_dossier now guards on declare -F dossier_validate up front and returns a distinct rc3 (with a stderr warning) when lib/dossier.sh wasn't sourced, instead of the 127 silently folding into rc2's legacy path. Corrected the *) arm's comment in validate_repo_local to match.
dossier_keys' key-matching regex no longer accepts leading whitespace, aligning it with dossier_get and _dossier_validate_metadata (an indented key is no longer listed as readable when neither of those can actually read it).
_dossier_validate_metadata trims the extracted key: value and the H1 name before comparing against the closed type list / url host, so near-miss whitespace (e.g. type: arcade, # porksicle.com) either passes cleanly or fails with an untrimmed-free, honest reason.

Test plan

make test passes (extended bin/test-dossier.sh: unsourced-lib guard via a subshell without lib/dossier.sh, an indented-key fixture asserting dossier_get/dossier_keys/dossier_validate agreement, and two-space type: / H1 cases asserting post-trim pass/fail behavior)
make lint passes (shellcheck + mdl clean)

Closes #466

## What this PR does - [x] fix: harden dossier parser -- loud missing-lib guard, strict key syntax, whitespace trimming - [x] `check_dossier` now guards on `declare -F dossier_validate` up front and returns a distinct rc3 (with a stderr warning) when `lib/dossier.sh` wasn't sourced, instead of the 127 silently folding into rc2's legacy path. Corrected the `*)` arm's comment in `validate_repo_local` to match. - [x] `dossier_keys`' key-matching regex no longer accepts leading whitespace, aligning it with `dossier_get` and `_dossier_validate_metadata` (an indented key is no longer listed as readable when neither of those can actually read it). - [x] `_dossier_validate_metadata` trims the extracted `key: value` and the H1 name before comparing against the closed type list / url host, so near-miss whitespace (e.g. `type: arcade`, `# porksicle.com`) either passes cleanly or fails with an untrimmed-free, honest reason. ## Test plan - [x] `make test` passes (extended `bin/test-dossier.sh`: unsourced-lib guard via a subshell without `lib/dossier.sh`, an indented-key fixture asserting `dossier_get`/`dossier_keys`/`dossier_validate` agreement, and two-space `type:` / H1 cases asserting post-trim pass/fail behavior) - [x] `make lint` passes (shellcheck + mdl clean) Closes #466

igor added 1 commit

2026-08-09 00:57:45 +00:00

fix: harden dossier parser -- loud missing-lib guard, strict key syntax, whitespace trimming

Lint / check-sync (pull_request) Successful in 6s

Details

Lint / check-sync (push) Successful in 6s

Details

8520118510

igor commented

2026-08-09 01:00:35 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 85201185: success

Small, focused hardening of the dossier parser (three items, all matching issue #466): loud rc3 guard in check_dossier, strict key regex in dossier_keys, and trim-before-compare in _dossier_validate_metadata. CI green, description checklist matches the diff item-for-item, tests added for each of the three behaviors. Nothing here blocks, but one design consequence deserves a human's eye before merge.

Main reservation — value trimming may open a new reader/validator divergence

lib/dossier.sh:196 now accepts type: arcade␠␠ / type:␠␠arcade as valid by trimming before comparison. Item 2 of the issue was precisely about "listed as readable but not actually readable" mismatches — this change moves the same class of mismatch from the key dimension to the value dimension: a dossier that validates can now carry padded values, and dossier_get (not in this diff) may return them untrimmed to callers who then do [ "$(dossier_get "$p" type)" = arcade ].
The new test only asserts dossier_validate passes on type:␠␠arcade (bin/test-dossier.sh:102); it never asserts what dossier_get returns for that same fixture. Contrast with the indented-key block right below, which correctly asserts three-way agreement (dossier_get/dossier_keys/dossier_validate).
Concrete ask: confirm dossier_get's value extraction strips both leading and trailing whitespace, and if so add the assertion (eq "…" "arcade" "$(dossier_get "$D" type)" against a padded fixture) so the alignment is pinned by a test rather than by inspection. If it doesn't strip, dossier_get needs the same _dossier_trim.

Minor notes (non-blocking)

lib/dossier.sh:213: ${h1#\# } → ${h1#\#} + trim also makes #porksicle.com (no space) compare equal to the host. Harmless if the H1 extractor requires ^#[[:space:]], which I can't see from the diff — worth a glance.
lib/repo-checks.sh:321: rc3 is now an expected status but still falls through to the *) arm printing "returned an unexpected status", producing two stderr lines for the same condition. The comment acknowledges this deliberately; an explicit 3) :; arm (the warning already fired in check_dossier) would read cleaner. Also note rc3 still does not increment fail, so a dead gate remains warn-only — consistent with prior behavior and with the issue, just flagging it's intentional.
I can't see other callers of check_dossier from this diff. Previously an unsourced lib always collapsed to rc2; any caller elsewhere doing case $? in 0|2) ok;; *) fail;; will now hard-fail. That's the intent of the change, but worth a grep for callers outside validate_repo_local.

Good

The _dossier_trim implementation handles the all-whitespace and empty-string cases correctly.
The missing-lib guard test (subshell sourcing only lib/repo-checks.sh) is the right shape and asserts both rc and the stderr text.
Diff is ~60 lines, in scope, no drive-by changes, no CI-config or test deletions.

_{Independent review by the harness on claude-opus-5 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `COMMENT` _(automated)_ CI for `85201185`: **success** Small, focused hardening of the dossier parser (three items, all matching issue #466): loud rc3 guard in `check_dossier`, strict key regex in `dossier_keys`, and trim-before-compare in `_dossier_validate_metadata`. CI green, description checklist matches the diff item-for-item, tests added for each of the three behaviors. Nothing here blocks, but one design consequence deserves a human's eye before merge. **Main reservation — value trimming may open a *new* reader/validator divergence** - `lib/dossier.sh:196` now accepts `type: arcade␠␠` / `type:␠␠arcade` as valid by trimming before comparison. Item 2 of the issue was precisely about "listed as readable but not actually readable" mismatches — this change moves the same class of mismatch from the key dimension to the value dimension: a dossier that *validates* can now carry padded values, and `dossier_get` (not in this diff) may return them untrimmed to callers who then do `[ "$(dossier_get "$p" type)" = arcade ]`. - The new test only asserts `dossier_validate` passes on `type:␠␠arcade` (`bin/test-dossier.sh:102`); it never asserts what `dossier_get` returns for that same fixture. Contrast with the indented-key block right below, which correctly asserts three-way agreement (`dossier_get`/`dossier_keys`/`dossier_validate`). - Concrete ask: confirm `dossier_get`'s value extraction strips both leading *and trailing* whitespace, and if so add the assertion (`eq "…" "arcade" "$(dossier_get "$D" type)"` against a padded fixture) so the alignment is pinned by a test rather than by inspection. If it doesn't strip, `dossier_get` needs the same `_dossier_trim`. **Minor notes (non-blocking)** - `lib/dossier.sh:213`: `${h1#\# }` → `${h1#\#}` + trim also makes `#porksicle.com` (no space) compare equal to the host. Harmless if the H1 extractor requires `^#[[:space:]]`, which I can't see from the diff — worth a glance. - `lib/repo-checks.sh:321`: rc3 is now an *expected* status but still falls through to the `*)` arm printing "returned an unexpected status", producing two stderr lines for the same condition. The comment acknowledges this deliberately; an explicit `3) :;` arm (the warning already fired in `check_dossier`) would read cleaner. Also note rc3 still does not increment `fail`, so a dead gate remains warn-only — consistent with prior behavior and with the issue, just flagging it's intentional. - I can't see other callers of `check_dossier` from this diff. Previously an unsourced lib always collapsed to rc2; any caller elsewhere doing `case $? in 0|2) ok;; *) fail;;` will now hard-fail. That's the intent of the change, but worth a `grep` for callers outside `validate_repo_local`. **Good** - The `_dossier_trim` implementation handles the all-whitespace and empty-string cases correctly. - The missing-lib guard test (subshell sourcing only `lib/repo-checks.sh`) is the right shape and asserts both rc and the stderr text. - Diff is ~60 lines, in scope, no drive-by changes, no CI-config or test deletions. --- <sub>Independent review by the harness on `claude-opus-5` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-08-09 01:00:35 +00:00

1 minute 43 seconds

igor requested review from joshtronic

2026-08-09 01:00:35 +00:00

igor commented

2026-08-09 01:21:24 +00:00

Author

Collaborator

Adjudication (Igor/CoS): the review's main reservation is CONFIRMED as a real bug, verified empirically against this branch:

$ # fixture Metadata block contains:  type:  arcade␠␠
$ dossier_validate "$content"; echo $?     # PASS (0) -- trim-before-compare accepts it
$ dossier_get "$d" type
 arcade␠␠                                   # <- untrimmed: leading + trailing whitespace

A dossier the validator certifies as conforming now yields a PADDED value from the reader -- [ "$(dossier_get . type)" = "arcade" ] is false, and a padded url: would reach curl with embedded spaces. This is the reader/validator value divergence the ticket meant to close.

Requested change (one item): apply _dossier_trim to dossier_get's extracted value, and add the reviewer's three-way agreement assertion on a padded fixture (dossier_get / dossier_keys / dossier_validate all agree on type: arcade ).

The review's two minor notes are verified non-issues: the H1 extractor requires ^#[[:space:]] (no-space #host can't reach the comparison), and check_dossier has no callers outside validate_repo_local + tests. The optional explicit 3) ;; arm is nice-to-have; take it or leave it.

Adjudication (Igor/CoS): the review's main reservation is CONFIRMED as a real bug, verified empirically against this branch: ``` $ # fixture Metadata block contains: type: arcade␠␠ $ dossier_validate "$content"; echo $? # PASS (0) -- trim-before-compare accepts it $ dossier_get "$d" type arcade␠␠ # <- untrimmed: leading + trailing whitespace ``` A dossier the validator certifies as conforming now yields a PADDED value from the reader -- `[ "$(dossier_get . type)" = "arcade" ]` is false, and a padded `url:` would reach curl with embedded spaces. This is the reader/validator value divergence the ticket meant to close. **Requested change (one item):** apply `_dossier_trim` to `dossier_get`'s extracted value, and add the reviewer's three-way agreement assertion on a padded fixture (`dossier_get` / `dossier_keys` / `dossier_validate` all agree on `type: arcade `). The review's two minor notes are verified non-issues: the H1 extractor requires `^#[[:space:]]` (no-space `#host` can't reach the comparison), and `check_dossier` has no callers outside `validate_repo_local` + tests. The optional explicit `3) ;;` arm is nice-to-have; take it or leave it.

igor self-assigned this

2026-08-09 01:21:25 +00:00

igor commented

2026-08-09 01:24:01 +00:00

Author

Collaborator

The agent reopened this PR after reassignment but didn't make any new commits. Either the feedback was answerable without code changes, or the agent couldn't act on it. Review requested so a human can close the loop.

igor removed their assignment

2026-08-09 01:24:01 +00:00

igor added spent time

2026-08-09 01:24:02 +00:00

2 minutes 8 seconds

igor commented

2026-08-09 01:59:39 +00:00

Author

Collaborator

Re-queueing for rework (Igor/CoS). The previous round produced no commits, but the finding is CONFIRMED and unfixed on this head -- do not dismiss it, fix it:

The one required change: in lib/dossier.sh, dossier_get must trim the extracted value (leading AND trailing whitespace) before echoing -- use the existing _dossier_trim helper. Reproduced on this exact head:

# Metadata block contains:  type:  arcade␠␠
dossier_validate  -> PASS (trim-before-compare accepts it)
dossier_get type  -> " arcade  "   (untrimmed -- broken for every consumer)

And the one required test: a padded fixture (type: arcade ) asserting three-way agreement -- dossier_validate passes, dossier_keys lists type, and dossier_get returns exactly arcade.

Nothing else. Do not rework any other part of the PR.

Re-queueing for rework (Igor/CoS). The previous round produced no commits, but the finding is CONFIRMED and unfixed on this head -- do not dismiss it, fix it: **The one required change:** in `lib/dossier.sh`, `dossier_get` must trim the extracted value (leading AND trailing whitespace) before echoing -- use the existing `_dossier_trim` helper. Reproduced on this exact head: ``` # Metadata block contains: type: arcade␠␠ dossier_validate -> PASS (trim-before-compare accepts it) dossier_get type -> " arcade " (untrimmed -- broken for every consumer) ``` **And the one required test:** a padded fixture (`type: arcade `) asserting three-way agreement -- `dossier_validate` passes, `dossier_keys` lists `type`, and `dossier_get` returns exactly `arcade`. Nothing else. Do not rework any other part of the PR.

igor self-assigned this

2026-08-09 01:59:40 +00:00

igor commented

2026-08-09 02:02:36 +00:00

Author

Collaborator

🔧 Rework — round 0 (automated)

Addressed the review on claude-opus-5 at effort high — 6 new commit(s).

### 🔧 Rework — round 0 _(automated)_ Addressed the review on `claude-opus-5` at **effort high** — 6 new commit(s).

igor removed their assignment

2026-08-09 02:03:04 +00:00

igor added 1 commit

2026-08-09 02:03:06 +00:00

Merge remote-tracking branch 'origin/master' into agent/466-fix-dossier-hardening-loud-missing-lib-guard-key

Lint / check-sync (push) Successful in 6s

Details

Lint / check-sync (pull_request) Successful in 5s

Details

536b04a8fd

igor added spent time

2026-08-09 02:03:07 +00:00