fix: tighten audit rules from RepoPrompt review (itd-85 follow-up) - #66
Merged
Conversation
An independent RepoPrompt oracle review of the merged itd-85 work surfaced three
clear correctness gaps, each fixed here test-first:
- three-tier-layout and conventions-router used fsutil.Exists, which accepts any
file type — a regular file at .abcd/development or .abcd/work, or a directory
named AGENTS.md, satisfied the rule. They now require a directory for the tiers
and a non-directory for the router.
- docs-currency silently passed when docs/ exists but .abcd/docs-lint.json is
missing — a check that could not run reading as clean. It now emits a warn
naming the gap, consistent with the engine's fail-closed stance elsewhere.
- the audit JSON omitted the "skipped" key when empty (omitempty), contradicting
the command doc's { findings, skipped } shape. It is now an always-present
array, guarded nil->[] in the serializer like findings.
Each fix has a watched-fail-then-pass test. abcd audit still exits 0 on this
repo; preflight clean.
Assisted-by: Claude:claude-opus-4-8
Completes the tier-type hardening: with the IsDir change, a repo whose .abcd/work is a regular file should report the layout violation — but decision-durability stats .abcd/work/DECISIONS.md first, which returned ENOTDIR (a file in the path prefix), and fsutil.Exists propagated it, so Evaluate aborted before three-tier could report. The audit errored out with a path-bearing stat message instead of naming the violation. fsutil.Exists/IsDir/DirHasEntries now treat ENOTDIR the same as ErrNotExist: a path whose parent component is a file cannot exist, so "not present" is the correct answer, not a fault a fail-closed caller should abort on. This is a general correctness fix to the shared primitive (lint uses it too, where the same semantics are right). Watched-fail-then-pass tests: the fsutil ENOTDIR case, and the audit case where .abcd/work is a file now surfaces the three-tier finding instead of aborting. Also corrects the rule_layout comment: IsDir follows symlinks, so a symlink-to-directory does satisfy a tier (fine for a layout check). Assisted-by: Claude:claude-opus-4-8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow-up to the itd-85
abcd auditwork (merged as #59–#65), applying the three clear correctness fixes an independent RepoPrompt oracle review surfaced. Each is test-first.three-tier-layoutandconventions-routerusedfsutil.Exists, which follows symlinks and accepts any type — so a regular file at.abcd/development/.abcd/work, or a directory namedAGENTS.md, satisfied the rule. Tiers now require a directory (IsDir); the router now requires a non-directory.docs-currencysilently passed when it could not run. Withdocs/present but.abcd/docs-lint.jsonmissing, it returned no findings — a check that cannot run reading as clean. It now emits awarnnaming the gap, consistent with the engine's fail-closed stance elsewhere.skippedkey vanished when empty.Result.Skippedhadomitempty, contradicting the command doc's{ findings, skipped }shape. It is now an always-present array, guardednil→[]in the serializer likefindings.Not changed (accurate findings, but deliberate)
The oracle also flagged, and I confirmed as by-design / documented: the privacy regex's precision-over-recall scope (misses
/Users/alicewith no trailing separator — also flagged by the earlier security review), the fail-open behaviour when git can't enumerate tracked files, and the unix-only build constraint (repo targets darwin/linux). The "presence vs tracked-durability" strictness point is a spec-interpretation the M3 review already settled on the looser reading; left as-is..abcd/worka regular file,decision-durabilitystats.abcd/work/DECISIONS.md, which returnedENOTDIR;fsutil.Existspropagated it andEvaluateaborted with a path-bearing stat error before three-tier could report the violation — so fix feat: harness-name prevention gate + name-banlist intent (itd-74) #1 silently no-op'd for theworktier specifically.fsutil.Exists/IsDir/DirHasEntriesnow treatENOTDIR(a file in the path prefix) the same asErrNotExist: the path cannot exist, so "not present" is the correct answer, not a fault to abort on. General correctness fix to the shared primitive (lint uses it too, same semantics).Notes
TestAC_PrivacyWaiverSuppresses's fixture was moved out ofdocs/so fix docs: acknowledgements, AI-attribution policy, badge fix #3's new warn doesn't perturb the exit code it asserts — it isolates the privacy-waiver behaviour, no coverage lost.rule_layoutcomment is corrected:IsDirfollows symlinks, so a symlink-to-directory does satisfy a tier — acceptable for a layout check (not the owned-store trust boundary).Each fix has a watched-fail-then-pass test;
abcd auditstill exits 0 on this repo; preflight clean. Correctness review of the first three fixes: PROMOTE (its two NOTES are both resolved by fix #4 and the comment correction).Assisted-by: Claude:claude-opus-4-8