Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .dev/features/capability-index-nonfile-md/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# GRILL — capability-index-nonfile-md

Plan: `.dev/features/capability-index-nonfile-md/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live
`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only.

## Findings

```yaml
- type: FINDING
rule_id: 'P5'
severity: minor
file: '.dev/features/capability-index-nonfile-md/PLAN.md:8'
problem: 'The capability DIRECTORY `<name>` itself is also joined; confirm the enumeration already requires it to be a real (non-symlink) directory, or the same class of bug exists one level up.'
evidence: 'replace `existsSync(capFile)` with `lstatSync(capFile, { throwIfNoEntry: false })`'
- type: FINDING
rule_id: 'P1'
severity: minor
file: '.dev/features/capability-index-nonfile-md/PLAN.md:34'
problem: "A FIFO case is named in the plan's rationale but not in its tests; mkfifo is POSIX-only — test it where available or state why not."
evidence: 'a FIFO would additionally make `readFileSync` block forever'
```

ADVISORY VERDICT: 2 concerns raised (0 blocking-severity, 2 advisory) — for the human to weigh before /pharn-dev-build.
57 changes: 57 additions & 0 deletions .dev/features/capability-index-nonfile-md/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# PLAN — capability-index-nonfile-md (PHARN-08: a non-file `<name>.md` must be one unknown capability, not a dead index)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: in `parseCapabilityIndex`, replace `existsSync(capFile)` with `lstatSync(capFile,
{ throwIfNoEntry: false })` and require `isFile()`: absent → the existing "missing its markdown"
refusal; present but a directory / symlink / FIFO / other → a new `ManifestValidationError` ("is not a
regular file"), so the per-capability tolerance reports it as `unknown` instead of the `EISDIR` from
`readFileSync` escaping as a non-validation error and aborting `init`/`add`/`update`/`status`.
- layer(s): the CLI itself (`src/lib/capability-index.ts`)
- constitution_refs: [P1, P2, P5, P7]

## Discovery — verified this run (P6)

Reproduced in the review: upstream tree `85bdaa37` + `pharn/pharn-review/newcap/newcap.md/` (a
directory) → `parseCapabilityIndex` threw `EISDIR: illegal operation on a directory, read` (not a
`ManifestValidationError`, so the catch rethrows it) → every command that parses the index exits 1 for
every deployed CLI at once, unpathed. Without that directory: `caps 35 unknown 1` (tolerated, as designed).
`capability-index.ts` header: "ANY ManifestValidationError raised while processing ONE capability …
becomes an `unknown` entry"; I/O failures deliberately still propagate. A wrong-TYPE path is a shape
problem of one capability, not a vanished clone, so it belongs on the tolerated side. A FIFO would
additionally make `readFileSync` block forever (the tar extractor rejects FIFOs today; this is the
second floor). Upstream history (264 commits) never had this shape — a future hazard, not an active one.

## Files

- `src/lib/capability-index.ts` — the `lstat` + `isFile()` check above — layer CLI/lib
- `tests/capability-index.test.ts` — `x/x.md` as a DIRECTORY → `unknown` contains `x` with a "not a
regular file" reason and the other capabilities still parse; `x/x.md` as a SYMLINK to a readable file
→ unknown too (never followed); a missing `x.md` keeps its existing message

## Contracts satisfied

- `LIMITS.md` §3e ("a per-capability grammar violation no longer aborts the parse … skipped and
reported") — now true for a wrong-type markdown path as well.

## Evals to write (P1)

- listed above; the directory case fails on the base source with EISDIR.

## Guarantee audit (P0)

- "a non-regular `<name>/<name>.md` never aborts the index" → floor: `lstat().isFile()` membership test
before any read; the refusal is a `ManifestValidationError`, which the existing loop tolerates.
- Genuine I/O failures (EACCES, a vanished clone) still propagate — unchanged, fail-closed.

## Trust audit (P2)

- The path is inside the untrusted clone; `lstat` never follows a link, so a symlinked markdown is refused
without being read (it could otherwise point outside the clone).

## Determinism audit (P5)

- File-type membership; terminal = the existing unknown-and-report path.

## Open questions (HALT)

- none
30 changes: 30 additions & 0 deletions .dev/features/capability-index-nonfile-md/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# REGRESSION — capability-index-nonfile-md

The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.

## Base and partition

- **base:** `a2fe83eef40a813bcbdb47ecb511c7932b133192` (`origin/main` at build time; the build is an uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`): `src/lib/capability-index.ts`, `tests/capability-index.test.ts`.
- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook scratch) and this
feature's own stage artifacts are not build output.
- **outside gates:** the stdlib `*.test.mjs` / `*.test.cjs` files + whole-repo `validate`; 0 committed eval pairs.
- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` / `lint:md` are absent from both maps.

## Per-gate exit codes

| gate | base | head | flipped? |
| ---------- | ---- | ---- | -------- |
| `tests` | 0 | 0 | no |
| `validate` | 0 | 0 | no |

- `regressions[]`: **empty**
- `pre_existing[]`: **empty**

## Verdict

**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.**
(`regression-report.json` `.verdict` = `no-regressions`.)

Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**` is
owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never the increment.
35 changes: 35 additions & 0 deletions .dev/features/capability-index-nonfile-md/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# REVIEW — capability-index-nonfile-md

Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**.

## Floor-gate findings (blocking)

None. The first `/pharn-dev-verify` run was `FAIL` (`test`): the new symlink test wrote its "outside"
file to the tmp dir's PARENT (`/tmp`), where a root-owned leftover from the root run blocked the
non-root run. Fixed in build — the fixture now lives entirely inside the test's own tmp dir — and
verify re-ran `PASS`.

- **L-floor (P0):** `lstat().isFile()` membership before any read; the refusal is a
`ManifestValidationError`, so the existing per-capability tolerance reports it. Genuine I/O errors
still propagate (unchanged).
- **L-eval (P1):** directory and symlink cases fail on the base source (`EISDIR`); the FIFO case runs on
POSIX (`mkfifo`), skipped on win32 — and was deliberately NOT run against the base source, where the
read would block forever.
- **L-trust (P2):** a symlinked markdown is refused without being read, so it can no longer read a file
outside the clone.
- **L-axis (P3):** one-file change within the fetch-boundary module.

## Advisory findings

```yaml
- type: FINDING
rule_id: 'P4'
severity: minor
file: 'CHANGELOG.md:8'
problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)."
evidence: '## [Unreleased]'
```

## Verdict

**GREEN** — 0 floor-gate findings, 1 advisory finding. No lesson proposed for canon.
17 changes: 17 additions & 0 deletions .dev/features/capability-index-nonfile-md/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# SHIP — capability-index-nonfile-md

Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` →
`/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2.

| stage | structural verdict (verbatim) |
| -------------------- | ------------------------------------------------------ |
| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit `0` |
| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` |
| `/pharn-dev-verify` | `verify-report.json` `.verdict` = `PASS` |

- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md)
- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if
its CI checks are green.

chain ran; the named floor verdicts are as shown — this is NOT a judgment that the increment is good or
wise; that is the human's call at the post-review gate.
27 changes: 27 additions & 0 deletions .dev/features/capability-index-nonfile-md/VERIFY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# VERIFY — capability-index-nonfile-md

## FLOOR layer (owns the verdict)

Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session
proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` /
`update.test.ts` fail for environmental reasons, identically at the baseline).

| gate | exit |
| -------------- | ---- |
| `format:check` | 0 |
| `lint` | 0 |
| `lint:md` | 0 |
| `test` | 0 |
| `typecheck` | 0 |
| `validate` | 0 |

No `structural:*` gate — the increment ships no eval-actual pair.

**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`).

## ADVISORY layer

`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only.

Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense
the suite does not encode.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"base": "a2fe83eef40a813bcbdb47ecb511c7932b133192",
"inside": [
"src/lib/capability-index.ts",
"tests/capability-index.test.ts"
],
"outside_gates": {
"tests": {
"base": 0,
"head": 0
},
"validate": {
"base": 0,
"head": 0
}
},
"regressions": [],
"pre_existing": [],
"verdict": "no-regressions"
}
17 changes: 17 additions & 0 deletions .dev/features/capability-index-nonfile-md/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"feature": "capability-index-nonfile-md",
"gates": {
"format:check": 0,
"lint": 0,
"lint:md": 0,
"test": 0,
"typecheck": 0,
"validate": 0
},
"verdict": "PASS",
"failing_gates": [],
"verifiers": {
"registered": 0,
"findings": []
}
}
4 changes: 2 additions & 2 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"scope": [
".dev/features/publish-split-oidc/SHIP.md"
".dev/features/capability-index-nonfile-md/SHIP.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T08:38:52.408Z"
"set_at": "2026-09-24T08:43:14.331Z"
}
72 changes: 64 additions & 8 deletions src/lib/capability-index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { existsSync, readFileSync, readdirSync } from 'node:fs';
import {
closeSync,
constants as fsConstants,
existsSync,
fstatSync,
openSync,
readFileSync,
readdirSync,
} from 'node:fs';
import { detectLayout, layoutPaths } from './layout.js';
import {
assertAppliesToken,
Expand Down Expand Up @@ -116,14 +124,8 @@ export function parseCapabilityIndex(repoDir: string): CapabilityIndex {
assertNoDotDot(name, `capability "${name}"`);

const capFile = safeJoin(subtreeDir, `${name}/${name}.md`);
if (!existsSync(capFile)) {
throw new ManifestValidationError(
`Capability "${name}" in ${subtree.dir} is missing its markdown ${name}/${name}.md.`,
);
}

const frontmatter = extractFrontmatter(
readFileSync(capFile, 'utf8'),
readCapabilityMarkdown(capFile, name, subtree.dir),
name,
);
// Cross-check the declared role against the authoritative subtree role.
Expand Down Expand Up @@ -162,6 +164,60 @@ export function parseCapabilityIndex(repoDir: string): CapabilityIndex {
return { capabilities, unknown };
}

// O_NOFOLLOW refuses a symlinked final component (ELOOP) and O_NONBLOCK keeps
// the open of a FIFO from blocking forever. Both are POSIX; where a platform
// lacks one (win32) the constant is absent and the flag is simply not set.
const OPEN_FLAGS =
fsConstants.O_RDONLY |
(fsConstants.O_NOFOLLOW ?? 0) |
(fsConstants.O_NONBLOCK ?? 0);

/**
* Read one capability's markdown, refusing anything that is not a regular file
* with a ManifestValidationError — the error the per-capability loop tolerates.
*
* `existsSync` + `readFileSync` let a DIRECTORY at this path through the check
* and then threw EISDIR, which is NOT a validation error, so one oddly-shaped
* upstream capability aborted the whole index for every deployed CLI. The
* type check is made on the OPENED descriptor and the read goes through that
* same descriptor, so nothing can swap the path between the check and the read.
* A directory, a symlink (never followed — it could point outside the clone)
* or a FIFO becomes `unknown`. Other I/O failures still propagate.
*/
function readCapabilityMarkdown(
capFile: string,
name: string,
subtreeDir: string,
): string {
let fd: number;
try {
fd = openSync(capFile, OPEN_FLAGS);
} catch (err) {
const code = (err as NodeJS.ErrnoException).code;
if (code === 'ENOENT') {
throw new ManifestValidationError(
`Capability "${name}" in ${subtreeDir} is missing its markdown ${name}/${name}.md.`,
);
}
if (code === 'ELOOP' || code === 'EISDIR') {
throw new ManifestValidationError(
`Capability "${name}" in ${subtreeDir}: ${name}/${name}.md is not a regular file.`,
);
}
throw err;
}
try {
if (!fstatSync(fd).isFile()) {
throw new ManifestValidationError(
`Capability "${name}" in ${subtreeDir}: ${name}/${name}.md is not a regular file.`,
);
}
return readFileSync(fd, 'utf8');
} finally {
closeSync(fd);
}
}

/**
* Extract the raw `---`-fenced frontmatter block from a capability markdown
* file. Only this block is parsed — a field-looking line in the prose body is
Expand Down
52 changes: 51 additions & 1 deletion tests/capability-index.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { mkdirSync, writeFileSync } from 'node:fs';
import { execFileSync } from 'node:child_process';
import { mkdirSync, symlinkSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';
import { describe, expect, it } from 'vitest';
import { useTmpDir } from './helpers.js';
Expand Down Expand Up @@ -250,6 +251,55 @@ describe('parseCapabilityIndex', () => {
expect(index.unknown[0]!.name).toBe('bad_name');
});

// PHARN-08: `<name>/<name>.md` present but NOT a regular file. existsSync
// said yes, readFileSync threw EISDIR (not a ManifestValidationError), and the
// whole index — init/add/update/status for every deployed CLI — died.
it('skips and reports a DIRECTORY named <name>.md instead of aborting the index', () => {
const repo = tmp.path();
scaffold(repo);
mkdirSync(join(repo, LENSES, 'newcap', 'newcap.md'), { recursive: true });
writeCap(repo, LENSES, 'n-plus-one', fm('lens', '["universal"]'));

const index = parseCapabilityIndex(repo);
expect(index.capabilities.map((c) => c.name)).toEqual(['n-plus-one']);
expect(index.unknown).toEqual([
{
name: 'newcap',
role: 'lens',
subtree: LENSES,
reason: expect.stringMatching(
/newcap\/newcap\.md is not a regular file/,
) as unknown as string,
},
]);
});

it('never follows a SYMLINKED <name>.md (it could point outside the clone)', () => {
const repo = join(tmp.path(), 'repo');
scaffold(repo);
const outside = join(tmp.path(), 'outside-cap.md');
writeFileSync(outside, fm('griller', '["universal"]'));
mkdirSync(join(repo, GRILLERS, 'linked'), { recursive: true });
symlinkSync(outside, join(repo, GRILLERS, 'linked', 'linked.md'));

const index = parseCapabilityIndex(repo);
expect(index.capabilities).toEqual([]);
expect(index.unknown.map((u) => u.name)).toEqual(['linked']);
});

it.skipIf(process.platform === 'win32')(
'refuses a FIFO <name>.md without reading it (a read would block forever)',
() => {
const repo = tmp.path();
scaffold(repo);
mkdirSync(join(repo, GRILLERS, 'piped'), { recursive: true });
execFileSync('mkfifo', [join(repo, GRILLERS, 'piped', 'piped.md')]);

const index = parseCapabilityIndex(repo);
expect(index.unknown.map((u) => u.name)).toEqual(['piped']);
},
);

it('skips and reports a directory with no capability markdown (the live repro)', () => {
const repo = tmp.path();
scaffold(repo);
Expand Down
Loading