build: Stacked on the relPath() pull request, re-cut the second 89773b06 hardening: introduce a single shared… #16

Merged
Connor merged 2 commits from build/570b419f into main 2026-09-14 12:29:34 +00:00
Owner

What changed

build/570b419f → main — 4 file(s), +134/−24.

  • src/lib/paths.ts
  • src/lib/vaultio.ts
  • test/paths.test.ts
  • test/vaultio.test.ts

Why

Green on the committed tree: 196 pass, 0 fail (npm run gate: tsc + build + node --test), up from a 192-pass baseline.

The premise needs the same correction the last step needed

The hardening was already standing on the relPath branch. TEXT_EXTENSIONS exists in src/lib/paths.ts, read refuses through textRefusal(), and list filters through isTextPath(). So there was no allow-list to introduce and no call site to gate — which means the real question was the one the brief actually cares about: can the two call sites drift? They could, and I closed the route.

What I changed

The parse underneath the set was still duplicated. extensionOf() lived in vaultio.ts as its own three-line copy of "find the last dot in the name," carrying a comment promising it "matched isTextPath" — while isTextPath() in paths.ts had the identical three lines inline. That pairing is load-bearing inside textRefusal(): it decides whether to refuse using isTextPath, and decides what to say using extensionOf. If those two ever disagreed about where an extension starts, a refusal would name the wrong thing about the file.

So extensionOf() moved to paths.ts beside the set, isTextPath() is now literally TEXT_EXTENSIONS.has(extensionOf(path)), and vaultio.ts re-exports rather than redefines. No behaviour change — the two implementations were character-identical, which is why the suite is green either way. This is the diff removing the second copy, not fixing a live bug.

What the tests prove […]

Gate

npm run gate ran 196 tests in 2.4s and exited 0 — green.

Built by Connor's backend backburner (dispatch 570b419f) in an isolated clone; shipped deterministically by ship.py. The full build notes were spoken in conversation and stored in memory (agent-dispatch:570b419f). Review is the gate.


The brief this build was given

Kept for provenance. This is the task the builder was dispatched with — not a description of the diff above.

Stacked on the relPath() pull request, re-cut the second 89773b06 hardening: introduce a single shared text-extension allow-list constant and gate both the read and the list operations on it, so read refuses non-text files outright and list omits them. Keep the set in one place so the two call sites cannot drift. Add unit tests covering a permitted extension, a binary extension, an extension-less file, a double extension, and case-insensitivity of the match. Ship as a pull request stacked on the previous one.

This task is one step of my plan: working-list item #57: Re-cut the two 89773b06 hardenings against current obsidian-connor main — relPath() must reject path segments beginning with '.', and read/list must be gated on a text-extension set; then sweep the five superseded stranded workdirs and fix the ledger's mislabelling

Result of earlier step 1 (read in obsidian-connor: In the obsidian-connor repo, locate commit 89773b06 (likely on an abandoned or s):
I could not save the file: this run is read-only (no Write tool), and /home/joel/.local/state/connor/notes/ does not exist on disk. The finished writeup is below, ready to save verbatim. The headline is that the premise of item #57 is stale.

First, a correction to what I thought I knew

89773b06 is not a commit sha — it is a dispatch id. It appears as one in my own run journal: /home/joel/.local/state/connor/runtool-runs.jsonl:568-570 — {"at": "2026-09-02T12:59:31+00:00", "dispatch": "89773b06", "target": "(discover)", "rc": 1}, then two rc: 0 runs at 13:02. Its workdir survives at /home/joel/.local/state/connor/frontend-work/89773b06, on a local-only branch build/89773b06 (.git/refs/heads/build/89773b06 → 1a08f083). Its reflog holds exactly one commit:

8b29e100 → 1a08f083  commit: build: add the streamable-HTTP MCP server to src/main.ts per the wire contract

So the "two hardenings" were never a hardening commit — they were details inside the stranded MCP-server build, which was superseded when the same feature landed as caffc2c (PR #1, origin/feat/mcp-server). That cut dropped both. My memory that "PR #8 landed commit 89773b06 in `src/ […]

### What changed `build/570b419f` → `main` — 4 file(s), +134/−24. - `src/lib/paths.ts` - `src/lib/vaultio.ts` - `test/paths.test.ts` - `test/vaultio.test.ts` ### Why Green on the committed tree: **196 pass, 0 fail** (`npm run gate`: tsc + build + node --test), up from a 192-pass baseline. ## The premise needs the same correction the last step needed The hardening was already standing on the relPath branch. `TEXT_EXTENSIONS` exists in `src/lib/paths.ts`, `read` refuses through `textRefusal()`, and `list` filters through `isTextPath()`. So there was no allow-list to introduce and no call site to gate — which means the real question was the one the brief actually cares about: *can the two call sites drift?* They could, and I closed the route. ## What I changed **The parse underneath the set was still duplicated.** `extensionOf()` lived in `vaultio.ts` as its own three-line copy of "find the last dot in the name," carrying a comment promising it "matched isTextPath" — while `isTextPath()` in `paths.ts` had the identical three lines inline. That pairing is load-bearing inside `textRefusal()`: it decides *whether* to refuse using `isTextPath`, and decides *what to say* using `extensionOf`. If those two ever disagreed about where an extension starts, a refusal would name the wrong thing about the file. So `extensionOf()` moved to `paths.ts` beside the set, `isTextPath()` is now literally `TEXT_EXTENSIONS.has(extensionOf(path))`, and `vaultio.ts` re-exports rather than redefines. **No behaviour change** — the two implementations were character-identical, which is why the suite is green either way. This is the diff removing the second copy, not fixing a live bug. ## What the tests prove […] ### Gate `npm run gate` ran 196 tests in 2.4s and exited 0 — green. Built by Connor's backend backburner (dispatch `570b419f`) in an isolated clone; shipped deterministically by `ship.py`. The full build notes were spoken in conversation and stored in memory (`agent-dispatch:570b419f`). Review is the gate. --- ### The brief this build was given _Kept for provenance. This is the task the builder was dispatched with — not a description of the diff above._ > Stacked on the relPath() pull request, re-cut the second 89773b06 hardening: introduce a single shared text-extension allow-list constant and gate both the read and the list operations on it, so read refuses non-text files outright and list omits them. Keep the set in one place so the two call sites cannot drift. Add unit tests covering a permitted extension, a binary extension, an extension-less file, a double extension, and case-insensitivity of the match. Ship as a pull request stacked on the previous one. > > This task is one step of my plan: working-list item #57: Re-cut the two 89773b06 hardenings against current obsidian-connor main — relPath() must reject path segments beginning with '.', and read/list must be gated on a text-extension set; then sweep the five superseded stranded workdirs and fix the ledger's mislabelling > > Result of earlier step 1 (read in obsidian-connor: In the obsidian-connor repo, locate commit 89773b06 (likely on an abandoned or s): > I could not save the file: this run is read-only (no Write tool), and `/home/joel/.local/state/connor/notes/` does not exist on disk. The finished writeup is below, ready to save verbatim. The headline is that the premise of item #57 is stale. > > ## First, a correction to what I thought I knew > > **`89773b06` is not a commit sha — it is a dispatch id.** It appears as one in my own run journal: `/home/joel/.local/state/connor/runtool-runs.jsonl:568-570` — `{"at": "2026-09-02T12:59:31+00:00", "dispatch": "89773b06", "target": "(discover)", "rc": 1}`, then two `rc: 0` runs at 13:02. Its workdir survives at `/home/joel/.local/state/connor/frontend-work/89773b06`, on a **local-only** branch `build/89773b06` (`.git/refs/heads/build/89773b06` → `1a08f083`). Its reflog holds exactly one commit: > > ``` > 8b29e100 → 1a08f083 commit: build: add the streamable-HTTP MCP server to src/main.ts per the wire contract > ``` > > So the "two hardenings" were never a hardening commit — they were details *inside* the stranded MCP-server build, which was superseded when the same feature landed as `caffc2c` (PR #1, `origin/feat/mcp-server`). That cut dropped both. My memory that "PR #8 landed commit 89773b06 in `src/ […]
read and list were already gated on TEXT_EXTENSIONS via isTextPath(), so the
hardening itself was standing. What was NOT single-sourced was the parse
underneath it: extensionOf() lived in vaultio.ts as its own three-line copy of
the same "find the last dot in the name" logic, with a comment promising it
"matched isTextPath". Two copies and a promise between them is precisely the
drift the one-set rule exists to prevent — textRefusal() decides WHETHER to
refuse with isTextPath and decides WHAT TO SAY with extensionOf, so a
disagreement between them names the wrong file in a refusal.

So extensionOf() moves to paths.ts beside TEXT_EXTENSIONS, isTextPath() is now
written as TEXT_EXTENSIONS.has(extensionOf(path)), and vaultio.ts re-exports
rather than redefines it. No behaviour changes: the two implementations were
character-identical, which is why the suite is green either way.

Tests: the five shapes an extension check gets wrong (permitted, binary,
extension-less, double extension, case) are now driven through read AND list
off one shared table, and the two shapes the fixture vault was missing —
Notes/SHOUTING.MD and Attachments/backup.md.gz — are in the shared fixtures so
every existing list assertion has to survive them too.

The load-bearing one is "the allow-list is the ONE knob": it adds an extension
to the set at runtime and asserts BOTH lanes start honouring it. Verified red
by giving listNotes a private regex copy of the rule — a copy that agrees with
the set on all five shapes, so none of the shape tests noticed and only the
knob test failed.

Co-Authored-By: Claude Opus 5 <[email protected]>
Merge main into the build before shipping
All checks were successful
gate / gate (pull_request) Successful in 8s
e37f293695
reviewer-bot left a comment

Independent review — reviewer-bot (local model qwen2.5-coder:14b)

The PR introduces a shared allow-list for text file extensions, ensuring consistency between read and list operations. It also refactors the extension extraction logic to improve clarity and maintainability.

**Independent review — reviewer-bot (local model `qwen2.5-coder:14b`)** The PR introduces a shared allow-list for text file extensions, ensuring consistency between read and list operations. It also refactors the extension extraction logic to improve clarity and maintainability.
Connor merged commit 6436a30640 into main 2026-09-14 12:29:34 +00:00
Connor deleted branch build/570b419f 2026-09-14 12:29:34 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
ZSDev/obsidian-connor!16
No description provided.