fix: tighten audit rules from RepoPrompt review (itd-85 follow-up)

internal/core/audit/audit.go

@@ -81,7 +81,7 @@ type Finding struct {
 // than silently omitting them.
 type Result struct {
 	Findings []Finding `json:"findings"`
-	Skipped  []string  `json:"skipped,omitempty"`
+	Skipped  []string  `json:"skipped"`
 	Blockers int       `json:"-"`
 	Warnings int       `json:"-"`
 	ExitCode int       `json:"-"`

internal/core/audit/audit_test.go

@@ -200,6 +200,27 @@ func TestJSONSerializerCleanRepo(t *testing.T) {
 	}
 }
 
+// The JSON always carries a present "skipped" array, even when nothing was
+// skipped — the command doc promises { findings, skipped }, so neither key may
+// vanish.
+func TestJSONSerializerSkippedAlwaysPresent(t *testing.T) {
+	out, err := audit.JSONSerializer{}.Serialize(audit.Result{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var decoded map[string]json.RawMessage
+	if err := json.Unmarshal(out, &decoded); err != nil {
+		t.Fatal(err)
+	}
+	raw, ok := decoded["skipped"]
+	if !ok {
+		t.Fatalf(`clean result must still emit a "skipped" key: %s`, out)
+	}
+	if string(raw) != "[]" {
+		t.Errorf(`empty "skipped" must be [], got %s`, raw)
+	}
+}
+
 func TestJSONSerializerCarriesRuleID(t *testing.T) {
 	res := audit.Result{Findings: []audit.Finding{
 		{RuleID: "three-tier-layout", Severity: audit.SeverityError, File: ".abcd", Line: 0, Message: "missing work/"},

internal/core/audit/rule_docs.go

@@ -39,13 +39,18 @@ func (docsCurrency) Where(ctx Context) bool {
 func (docsCurrency) Eval(ctx Context) ([]Finding, error) {
 	cfgPath := filepath.Join(ctx.RepoRoot, ".abcd", "docs-lint.json")
 	// The engine reuse needs the repo's docs-lint config. A repo with docs/ but
-	// no committed config cannot be linted; degrade to no findings rather than
-	// fail — the config is what defines the checks, and its absence is a
-	// prepare-this-repo gap, not a docs-drift violation.
+	// no committed config cannot be linted — but the rule must not then read as a
+	// silent pass. Surface a warn that the check could not run (a
+	// prepare-this-repo gap), not an absence of drift.
 	if present, err := fsutil.Exists(cfgPath); err != nil {
 		return nil, err
 	} else if !present {
-		return nil, nil
+		return []Finding{{
+			RuleID:   "docs-currency",
+			Severity: SeverityWarn,
+			File:     ".abcd/docs-lint.json",
+			Message:  "docs/ is present but .abcd/docs-lint.json is missing — docs currency cannot be checked; run prepare-this-repo",
+		}}, nil
 	}
 
 	cfg, err := lint.LoadConfig(cfgPath)

internal/core/audit/rule_layout.go

@@ -37,16 +37,20 @@ func (threeTierLayout) Eval(ctx Context) ([]Finding, error) {
 		{".abcd/development", "durable-record tier .abcd/development/"},
 		{".abcd/work", "shared-working tier .abcd/work/"},
 	} {
-		present, err := fsutil.Exists(filepath.Join(ctx.RepoRoot, filepath.FromSlash(tier.rel)))
+		// A tier is a directory: a regular file at the tier path does not satisfy
+		// the convention, so check the type, not mere presence. (IsDir follows a
+		// symlink, so a symlink-to-directory does satisfy it — acceptable for a
+		// layout check, which is not the owned-store trust boundary.)
+		isDir, err := fsutil.IsDir(filepath.Join(ctx.RepoRoot, filepath.FromSlash(tier.rel)))
 		if err != nil {
 			return nil, err
 		}
-		if !present {
+		if !isDir {
 			out = append(out, Finding{
 				RuleID:   "three-tier-layout",
 				Severity: SeverityError,
 				File:     tier.rel,
-				Message:  "missing the " + tier.label,
+				Message:  "missing the " + tier.label + " (must be a directory)",
 			})
 		}
 	}

internal/core/audit/rule_router.go

@@ -24,17 +24,25 @@ func (conventionsRouter) Meta() RuleMeta {
 func (conventionsRouter) Where(Context) bool { return true }
 
 func (conventionsRouter) Eval(ctx Context) ([]Finding, error) {
-	present, err := fsutil.Exists(filepath.Join(ctx.RepoRoot, "AGENTS.md"))
+	// The router is a file: a directory named AGENTS.md does not satisfy it.
+	path := filepath.Join(ctx.RepoRoot, "AGENTS.md")
+	present, err := fsutil.Exists(path)
 	if err != nil {
 		return nil, err
 	}
 	if present {
-		return nil, nil
+		isDir, err := fsutil.IsDir(path)
+		if err != nil {
+			return nil, err
+		}
+		if !isDir {
+			return nil, nil // a real router file
+		}
 	}
 	return []Finding{{
 		RuleID:   "conventions-router",
 		Severity: SeverityError,
 		File:     "AGENTS.md",
-		Message:  "no AGENTS.md conventions router at the repo root",
+		Message:  "no AGENTS.md conventions router file at the repo root",
 	}}, nil
 }

internal/core/audit/rules_test.go

@@ -205,6 +205,91 @@ func TestRule_WorkLocalNotGitignored(t *testing.T) {
 	}
 }
 
+// A tier path that exists but is a regular FILE (not a directory) does not
+// satisfy three-tier-layout — the tiers are directories.
+func TestRule_TierPresentButNotADirectory(t *testing.T) {
+	b := newFixtureRepo(t).
+		file(".gitignore", ".abcd/.work.local/\n").
+		file(".abcd/development", "I am a file, not the durable-record tier\n"). // a file at a tier path
+		file(".abcd/work/DECISIONS.md", "x\n").
+		file(".abcd/.work.local/NEXT.md", "x\n").
+		file("AGENTS.md", "x\n").
+		commit()
+	res := b.run()
+
+	f := findingFor(res, "three-tier-layout")
+	if f == nil {
+		t.Fatal("no three-tier-layout finding when .abcd/development is a file, not a directory")
+	}
+	if f.Severity != audit.SeverityError {
+		t.Errorf("severity = %q, want error", f.Severity)
+	}
+}
+
+// The work tier specifically: when .abcd/work is a regular file, decision-durability
+// stats .abcd/work/DECISIONS.md (ENOTDIR). That must read as "not present", not
+// abort the audit — so the three-tier "must be a directory" finding still surfaces.
+func TestRule_WorkTierIsAFileDoesNotAbort(t *testing.T) {
+	b := newFixtureRepo(t).
+		file(".gitignore", ".abcd/.work.local/\n").
+		file(".abcd/development/README.md", "x\n").
+		file(".abcd/work", "I am a file, not the shared-working tier\n").
+		file(".abcd/.work.local/NEXT.md", "x\n").
+		file("AGENTS.md", "x\n").
+		commit()
+	res := b.run() // must not error/abort
+
+	f := findingFor(res, "three-tier-layout")
+	if f == nil {
+		t.Fatal("no three-tier-layout finding when .abcd/work is a file (audit aborted instead of reporting)")
+	}
+	if f.Severity != audit.SeverityError {
+		t.Errorf("severity = %q, want error", f.Severity)
+	}
+}
+
+// A directory named AGENTS.md does not satisfy conventions-router — the router is
+// a file.
+func TestRule_ConventionsRouterIsADirectory(t *testing.T) {
+	b := newFixtureRepo(t).
+		file(".gitignore", ".abcd/.work.local/\n").
+		file(".abcd/development/README.md", "x\n").
+		file(".abcd/work/DECISIONS.md", "x\n").
+		file(".abcd/.work.local/NEXT.md", "x\n").
+		file("AGENTS.md/keep.txt", "AGENTS.md is a directory here\n"). // dir, not a router file
+		commit()
+	res := b.run()
+
+	f := findingFor(res, "conventions-router")
+	if f == nil {
+		t.Fatal("no conventions-router finding when AGENTS.md is a directory")
+	}
+	if f.Severity != audit.SeverityError {
+		t.Errorf("severity = %q, want error", f.Severity)
+	}
+}
+
+// docs-currency: docs/ exists but no docs-lint config → a warn that the check
+// could not run, never a silent pass.
+func TestRule_DocsCurrencyNoConfigWarns(t *testing.T) {
+	b := newFixtureRepo(t).conforming().
+		file("docs/how-to/thing.md", "clean docs\n"). // docs/ present, but no .abcd/docs-lint.json
+		commit()
+	res := b.run()
+
+	f := findingFor(res, "docs-currency")
+	if f == nil {
+		t.Fatal("docs-currency silently passed when docs/ exists but the config is missing")
+	}
+	if f.Severity != audit.SeverityWarn {
+		t.Errorf("severity = %q, want warn", f.Severity)
+	}
+	// It must be a warn, not an error: exit 1, not 2.
+	if res.ExitCode != 1 {
+		t.Errorf("exit = %d, want 1", res.ExitCode)
+	}
+}
+
 // AC3: a committed file with an absolute local path → privacy-hygiene error
 // citing file:line — unless a waiver escape is on that line.
 func TestAC_PrivacyAbsolutePath(t *testing.T) {
@@ -231,8 +316,10 @@ func TestAC_PrivacyAbsolutePath(t *testing.T) {
 
 func TestAC_PrivacyWaiverSuppresses(t *testing.T) {
 	const waived = "example path /Users/alice/x is illustrative  abcd-audit:allow\n"
+	// Kept out of docs/ so docs-currency stays skipped and does not add a warn —
+	// this test isolates the privacy waiver's effect on the exit code.
 	b := newFixtureRepo(t).conforming().
-		file("docs/reference/paths.md", waived).
+		file("reference/paths.md", waived).
 		commit()
 	res := b.run()
 

internal/core/audit/serialize.go

@@ -16,9 +16,13 @@ type Serializer interface {
 type JSONSerializer struct{}
 
 func (JSONSerializer) Serialize(res Result) ([]byte, error) {
-	// Guarantee a present empty array rather than JSON null for a clean repo.
+	// Guarantee present empty arrays rather than JSON null: the command doc
+	// promises { findings, skipped }, so neither key may be null or absent.
 	if res.Findings == nil {
 		res.Findings = []Finding{}
 	}
+	if res.Skipped == nil {
+		res.Skipped = []string{}
+	}
 	return json.MarshalIndent(res, "", "  ")
 }

internal/fsutil/paths.go

@@ -4,8 +4,18 @@ import (
 	"errors"
 	"io"
 	"os"
+	"syscall"
 )
 
+// notPresent reports whether a stat/open error means the path cannot exist: it
+// is absent (ErrNotExist), or a component of its prefix is not a directory
+// (ENOTDIR, e.g. asking about a/b where a is a regular file). Both are "not
+// present", not a filesystem fault, so a fail-closed caller must not abort on
+// them.
+func notPresent(err error) bool {
+	return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR)
+}
+
 // Exists reports whether path exists, following symlinks — so a link to a real
 // file exists and a dangling link does not. A stat error other than not-exist is
 // returned rather than swallowed, so a caller checking a convention fails closed
@@ -15,7 +25,7 @@ func Exists(path string) (bool, error) {
 	if err == nil {
 		return true, nil
 	}
-	if errors.Is(err, os.ErrNotExist) {
+	if notPresent(err) {
 		return false, nil
 	}
 	return false, err
@@ -31,7 +41,7 @@ func IsDir(path string) (bool, error) {
 	if err == nil {
 		return fi.IsDir(), nil
 	}
-	if errors.Is(err, os.ErrNotExist) {
+	if notPresent(err) {
 		return false, nil
 	}
 	return false, err
@@ -47,7 +57,7 @@ func IsDir(path string) (bool, error) {
 func DirHasEntries(path string) (bool, error) {
 	f, err := os.Open(path)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
+		if notPresent(err) {
 			return false, nil
 		}
 		return false, err

internal/fsutil/paths_test.go

@@ -61,6 +61,29 @@ func TestExistsSymlink(t *testing.T) {
 	}
 }
 
+// A path whose parent component is a regular file cannot exist; stat returns
+// ENOTDIR, and Exists/IsDir/DirHasEntries must read that as "not present"
+// (false, nil), not propagate it as a hard error — otherwise a caller that
+// fails closed on errors aborts on an obviously-absent path.
+func TestPathUnderAFileIsNotPresent(t *testing.T) {
+	dir := t.TempDir()
+	file := filepath.Join(dir, "afile")
+	if err := os.WriteFile(file, []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	under := filepath.Join(file, "child") // afile is not a directory
+
+	if got, err := fsutil.Exists(under); err != nil || got {
+		t.Errorf("Exists(under-a-file) = %v, %v; want false, nil", got, err)
+	}
+	if got, err := fsutil.IsDir(under); err != nil || got {
+		t.Errorf("IsDir(under-a-file) = %v, %v; want false, nil", got, err)
+	}
+	if got, err := fsutil.DirHasEntries(under); err != nil || got {
+		t.Errorf("DirHasEntries(under-a-file) = %v, %v; want false, nil", got, err)
+	}
+}
+
 func TestIsDir(t *testing.T) {
 	dir := t.TempDir()
 	file := filepath.Join(dir, "f.txt")