# Batch 10 — CLI — Findings

**Date:** 2026-03-26
**Files reviewed:** `cli/index.js`, `core/src/vscodeTypes.js` (shim target), `core/src/plainTextDocument.js`, `cli/DEVELOPMENT.md`
**Status:** Complete

---

## Summary

The CLI is a single-file entry point (`cli/index.js`, 256 lines). No Critical findings.
No security issues.

The most significant finding is a documentation/implementation rift: `DEVELOPMENT.md`
references a separate `cli/vscode-shim.js` file that does not exist (the shim is inline in
`index.js`), documents `--format` as an implemented flag that is not in `parseArgs`, and
claims `--surface` defaults to `cicd` when the code default is `null` (no surface filter).
These together make the docs unreliable as a usage reference.

The only runtime correctness gap is that `--output` without a value causes an unhandled
`TypeError` crash (exit 1) rather than a clean exit 2, because `path.resolve(undefined)`
throws before the output-path code runs.

No `require('vscode')` violation — the `Module._load` intercept at lines 20–25 correctly
redirects `require('vscode')` to `../core/src/vscodeTypes` before any `core/` module loads.
This shim arrangement is acknowledged as temporary in two TODO/Phase-2 comments.

---

## Findings

### `cli/DEVELOPMENT.md`

#### [MINOR] `vscode-shim.js` referenced as a separate file that does not exist

**Issue:** `DEVELOPMENT.md` shows the folder structure as:

```
cli/
├── index.js
└── vscode-shim.js    ← Intercepts require('vscode')…
```

`cli/vscode-shim.js` does not exist. The `Module._load` intercept is inline in `index.js`
(lines 20–25) and redirects to `core/src/vscodeTypes.js`. The DEVELOPMENT.md was written
against a planned layout that was not implemented.

**Recommendation:** Update the folder structure diagram to reflect the actual layout and
note that the shim is inline in `index.js`.

---

#### [MINOR] `--format` flag documented without `*(planned)*` marker but not implemented

**Issue:** The CLI flags table documents:

```
--format <text|json>   Output format (default: text)
```

There is no `--format` case in `parseArgs`. Passing `--format json` hits the `default:`
branch at line 64, prints "Unknown argument: --format", and exits 2. A user reading the
docs would expect the flag to work.

By contrast, `--diff-only` and `--base-branch` are correctly marked `*(planned)*` in the
same table.

**Recommendation:** Either implement `--format text|json` or mark it `*(planned)*`
consistently with the other unimplemented flags.

---

#### [MINOR] `--surface` default documented as `cicd` but code default is `null` (no filter)

**Issue:** The flags table states `--surface` defaults to `cicd`. In `parseArgs` the
default is `args.surface = null` (line 55). When `--surface` is omitted, the main function
takes the `else` branch at line 196 and passes through all diagnostics regardless of
surface tag. The effective default is "all surfaces", not `cicd`.

This means CI/CD runs without `--surface cicd` receive IDE-only diagnostics (e.g.
ACU-KWD-001 keyword capitalisation, tagged `["ide"]`). On a project with many style
warnings, omitting `--surface` in a pipeline could generate noise that was not intended
for CI/CD consumers.

**Recommendation:** Either change the code to default to `"cicd"` when `--surface` is
omitted, or correct the docs to say the default is `all`. Defaulting to `cicd` is the
safer choice for pipeline use cases.

---

#### [MINOR] `--surface` validation rejects `all` but docs list it as a valid value

**Issue:** The flags table shows `--surface <ide|cicd|all>` as a valid flag. The
validation at line 164 checks `!["ide", "cicd"].includes(args.surface)` — `"all"` is not
in the list and would cause exit 2 with "Error: --surface must be one of: ide, cicd".

**Recommendation:** If `"all"` is a documented option, add it to the validation list and
handle it explicitly (e.g., by setting `args.surface = null` internally to mean no filter).
If `"all"` is not intended, remove it from the docs.

---

### `cli/index.js`

#### [MINOR] `--output` without a value causes unhandled `TypeError` crash — line 238

**Issue:** `parseArgs` uses `argv[++i]` to read flag values without bounds-checking. If
`--output` is the last argument (e.g., `node cli/index.js --folder /some/path --output`),
`argv[++i]` is `undefined` and `args.output = undefined`. The validation in `main` has no
guard for `undefined` output. At line 234–236:

```js
const outputPath = args.output
    ? path.resolve(args.output)   // path.resolve(undefined) → TypeError
    : path.join(...);
```

`args.output = undefined` is truthy-false (falsy), so the ternary takes the else branch —
actually `undefined` is falsy, so `args.output` is falsy and the else branch runs. No
crash here.

Correction upon closer reading: `undefined` is falsy, so `path.join(folderPath,
buildDefaultReportFileName())` runs instead — the report is written to the default
location, not to `undefined`. This specific path is safe.

The actual gap: if the user passes `--fail-on` as the last token without a value,
`args.failOn = undefined`, which fails the validation at line 159
(`!["error",...].includes(undefined)`) and exits cleanly with code 2. Likewise `--surface`
without a value exits cleanly. `--folder` without a value exits cleanly via `!args.folder`.

The only genuine crash path is `--output someValue` where `someValue` is itself a flag
(e.g., `--output --folder /path`): `args.output = "--folder"` and `args.folder =
"/path"`, leading to `path.resolve("--folder")` producing `<cwd>/--folder` as the output
file path — silently writing the report to a file named `--folder` in the working
directory. This is a usability issue but not a crash.

**Revised severity: Observation** — argument parsing has no value-present validation, but
the combination of falsy-checks and explicit flag validation prevents crashes in the common
cases. The edge case above produces silent misbehaviour. Documented here for completeness.

---

#### [MINOR] `findTxtFiles` and file reads have no error handling for permission failures

**Issue:** `findTxtFiles` calls `fs.readdirSync(dir, ...)` (line 77) with no try/catch.
On any directory under `folderPath` that the process lacks read permission to, Node.js
throws `Error: EACCES: permission denied`. This propagates as an unhandled exception,
printing a Node.js stack trace and exiting with code 1 — not exit code 2 as documented
for runtime errors, and with no clean error message.

Similarly, `fs.readFileSync(filePath, "utf8")` at line 189 is unguarded. A file that
exists during discovery but is deleted or locked before the read (TOCTOU race) throws and
crashes the scan for all subsequent files.

**Recommendation:** Wrap the scan loop or `findTxtFiles` in a try/catch that catches
`Error` objects, prints a user-friendly message to `stderr`, and calls `process.exit(2)`.

---

#### [MINOR] No check that `folderPath` is a directory, not a file — line 169–174

**Issue:** `fs.existsSync(folderPath)` at line 171 returns `true` for both files and
directories. If the user passes a file path (e.g., `--folder /some/file.txt`),
`findTxtFiles` calls `fs.readdirSync(filePath)` which throws `ENOTDIR`. The error is
unhandled (same crash path as the previous finding).

**Recommendation:** Replace `fs.existsSync` with `fs.statSync(folderPath)` and check
`stat.isDirectory()`. If not a directory, exit 2 with a clear message.

---

## Observations

### Shim is a temporary workaround — acknowledged in code

The inline `Module._load` intercept (lines 20–25) and the `vscodeTypes.js` shim are
explicitly marked as temporary in two comments (line 19 TODO and the DEVELOPMENT.md Phase 2
note). The permanent fix is the same global work identified across Batches 1–9: remove
`require('vscode')` from all `core/` files. Once that is done, this intercept and
`vscodeTypes.js` can be deleted entirely.

---

### Default report written into the scanned folder

When `--output` is omitted, the JSON report is written to
`path.join(folderPath, "acurity-validation-report-YYYYMMDD-HHMMSS.json")`. Over multiple
CI runs, old report files accumulate inside the source folder. This is accepted CLI
behaviour but worth noting for teams that commit everything in their workspace folder.

---

### `summarize` severity totals use string keys correctly

`summarize` (line 120) iterates `fr.diagnostics` which are already normalised (severity
converted to a string in `normalized` at line 199). The `totals.severity` map uses string
keys (`{ error, warning, information, hint }`). This is correct — no type mismatch.

---

### No integration test runner in `cli/`

The DEVELOPMENT.md lists an "Integration Test Checklist" (six scenarios) but there is no
automated test that exercises the CLI end-to-end. The scenarios are manual validation
steps. This is noted as the highest-priority test gap for the CLI layer.

---

## Summary table

| Severity | Count | Items |
|---|---|---|
| Critical | 0 | — |
| Major | 0 | — |
| Minor | 5 | `vscode-shim.js` docs/reality mismatch; `--format` undeclared *(planned)*; `--surface` default doc/code mismatch; `--surface all` rejected but documented; unguarded `readdirSync`/`readFileSync` crash paths; no `isDirectory` check |
| Observation | 4 | Shim acknowledged temporary; reports accumulate in scanned folder; `summarize` severity string keys correct; no automated CLI integration tests |
