fix: creating and generated writes go through the brain root (#188, #158 part A) #189

Merged
mathias merged 4 commits from fix/create-regular-188 into main 2026-10-05 21:25:54 +00:00
Owner

Fixes #188. Refs #158 (part A). risk/high: control code, adversarial review required.

Problem

These writers created or rewrote brain files by path, so a symlinked directory or file redirected the write elsewhere in the brain, or outside it for brain_write and promote/reject. Each was verified red before the fix:

  • brain_write (writeHallNote)
  • promote/reject (destination write, then source removal)
  • StampProvenanceFile (brain_edit's re-stamp)
  • _index.md, wiki/index.md
  • log.md, raw/tunnel-candidates-*.md

New brainfs primitives

  • PutRegular: create or replace.
    • Missing parents are created one component at a time through the root, and every existing component must be a real directory.
    • An existing file is replaced only through WriteRegular's checks.
    • A new file is created with O_EXCL|O_NOFOLLOW and confirmed (Lstat + SameFile) before writing.
  • AppendRegular: create-or-append, with the same parent rule. It writes only after confirming the path is the regular file it opened.
  • Remove: clean path and real parent; removes a final symlink itself, never its target.
  • RealParent: the clean-path and real-directory check, for writers that do their own atomic rename.

Converted

  • writeHallNote
  • promote/reject: PutRegular + Remove. The source is kept when the write is refused.
  • StampProvenanceFile: ReadRegular + WriteRegular. provenance.ApplyFile, now unused, is removed.
  • BuildWingIndex and RebuildIndex: PutRegular.
  • AppendLog and logFuzzyCandidates: AppendRegular.
  • brain_edit's own rename was already refused by #187's read check. TestEditRefusesANoteBehindASymlinkedDirectory is a guard (green before and after).

Tests

In-brain and outside targets, each with a positive control:

  • TestPutRegularCreatesOrReplacesOnlyThroughRealDirectories
  • TestAppendRegularCreatesOrAppendsOnlyThroughRealDirectories
  • TestRealParentRefusesASymlinkedDirectoryOrUncleanPath
  • TestWriteNoteRefusesASymlinkedWing (in-brain and outside)
  • TestPromoteAndRejectRefuseASymlinkedDestinationDirectory
  • TestStampProvenanceFileRefusesANoteBehindASymlinkedDirectory
  • TestBuildWingIndexDoesNotWriteThroughASymlinkedIndex
  • TestAutoTunnelDoesNotAppendTheCandidateLogThroughASymlinkedRaw
  • TestRebuildIndexAndAppendLogDoNotWriteThroughSymlinks

Flips

Red:

  • PutRegular with root.MkdirAll instead of mkdirReal
  • AppendRegular without the pre-check

Green, by design:

  • PutRegular create path without SameFile: O_EXCL already refuses any existing entry, including a symlink.

Not in this PR (#158 part B)

  • pipeline buildAndWrite
  • BackfillRefs
  • the watcher's raw/ markers and move
  • the session archive in cmd/server

Residual: a directory swapped in after the checks is #167.

🤖 Generated with Claude Code


Created via git-mcp on behalf of @mathias

Fixes #188. Refs #158 (part A). risk/high: control code, adversarial review required. ## Problem These writers created or rewrote brain files by path, so a symlinked directory or file redirected the write elsewhere in the brain, or outside it for `brain_write` and promote/reject. Each was verified red before the fix: - `brain_write` (`writeHallNote`) - promote/reject (destination write, then source removal) - `StampProvenanceFile` (brain_edit's re-stamp) - `_index.md`, `wiki/index.md` - `log.md`, `raw/tunnel-candidates-*.md` ## New brainfs primitives - **`PutRegular`:** create or replace. - Missing parents are created **one component at a time through the root**, and every existing component must be a real directory. - An existing file is replaced only through `WriteRegular`'s checks. - A new file is created with `O_EXCL|O_NOFOLLOW` and confirmed (Lstat + SameFile) before writing. - **`AppendRegular`:** create-or-append, with the same parent rule. It writes only after confirming the path is the regular file it opened. - **`Remove`:** clean path and real parent; removes a final symlink itself, never its target. - **`RealParent`:** the clean-path and real-directory check, for writers that do their own atomic rename. ## Converted - `writeHallNote` - promote/reject: `PutRegular` + `Remove`. The source is kept when the write is refused. - `StampProvenanceFile`: `ReadRegular` + `WriteRegular`. `provenance.ApplyFile`, now unused, is removed. - `BuildWingIndex` and `RebuildIndex`: `PutRegular`. - `AppendLog` and `logFuzzyCandidates`: `AppendRegular`. - `brain_edit`'s own rename was already refused by #187's read check. `TestEditRefusesANoteBehindASymlinkedDirectory` is a guard (green before and after). ## Tests In-brain and outside targets, each with a positive control: - `TestPutRegularCreatesOrReplacesOnlyThroughRealDirectories` - `TestAppendRegularCreatesOrAppendsOnlyThroughRealDirectories` - `TestRealParentRefusesASymlinkedDirectoryOrUncleanPath` - `TestWriteNoteRefusesASymlinkedWing` (in-brain and outside) - `TestPromoteAndRejectRefuseASymlinkedDestinationDirectory` - `TestStampProvenanceFileRefusesANoteBehindASymlinkedDirectory` - `TestBuildWingIndexDoesNotWriteThroughASymlinkedIndex` - `TestAutoTunnelDoesNotAppendTheCandidateLogThroughASymlinkedRaw` - `TestRebuildIndexAndAppendLogDoNotWriteThroughSymlinks` ## Flips **Red:** - `PutRegular` with `root.MkdirAll` instead of `mkdirReal` - `AppendRegular` without the pre-check **Green, by design:** - **`PutRegular` create path without SameFile:** `O_EXCL` already refuses any existing entry, including a symlink. ## Not in this PR (#158 part B) - pipeline `buildAndWrite` - `BackfillRefs` - the watcher's `raw/` markers and move - the session archive in `cmd/server` **Residual:** a directory swapped in after the checks is #167. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Created via git-mcp on behalf of @mathias_
mathias added 2 commits 2026-10-05 20:39:58 +00:00
brain_write created a new note with os.MkdirAll + os.WriteFile by path,
so a symlinked wing or hall directory redirected the note elsewhere in
the brain or outside it.

New brainfs primitives for creating writes:
- PutRegular: create or replace. Missing parents are created one
  component at a time through the root and every existing component
  must be a real directory; an existing file is replaced only via
  WriteRegular's checks; a new one is created with O_EXCL and confirmed
  (Lstat + SameFile) before writing.
- AppendRegular: create-or-append with the same parent rule, writing
  only after confirming the path is the regular file it opened.
- RealParent: the clean-path + real-directory check, for writers that do
  their own atomic rename.

writeHallNote now uses PutRegular.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix: generated and promoted writes go through the brain root (#188, #158)
CI / Lint / Test / Vet (pull_request) Successful in 1m17s
CI / Mirror to GitHub (pull_request) Skipped
138d6e2545
Writers that created or rewrote brain files by path followed symlinks:
- promote/reject: os.MkdirAll + os.WriteFile of the destination, then
  os.Remove of the raw source. Now PutRegular + brainfs.Remove; the source
  is kept when the write is refused.
- StampProvenanceFile (brain_edit's re-stamp): provenance.ApplyFile read
  and rewrote by path. Now ReadRegular + WriteRegular; ApplyFile, now
  unused, is removed.
- BuildWingIndex (_index.md) and wiki.RebuildIndex (wiki/index.md): now
  PutRegular.
- wiki.AppendLog (log.md) and AutoTunnel's fuzzy-candidate log
  (raw/tunnel-candidates-*.md): now AppendRegular; tunnel's fileExists,
  now unused, is removed.
- New brainfs.Remove: clean path, real parent, removes a final symlink
  itself, never its target.

brain_edit's own atomic rename was already refused behind a symlinked
directory by #187's read check; a guard test pins it.

Pipeline writes, BackfillRefs, the watcher's raw/ markers and the session
archive in cmd/server follow in a separate PR (#158).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mathias added the risk/high label 2026-10-05 20:40:16 +00:00
Author
Owner

Adversarial review: PR #189 (head 138d6e2). Verdict: BLOCKED

I tested this by running it, not by reading it. The production code held up: no entry point created or changed anything at a link target, nothing hung, and the raw source survived every refused promote or reject. The block is on three tests that pass whether or not the behaviour they name is there. Rules 8 and 9 apply: a check that cannot go red is decoration.

Blocking

B1. internal/notes/edit_redirect_test.go: TestEditRefusesANoteBehindASymlinkedDirectory is decoration.

  • The temp brain has no .git, so notes.Edit refuses with "brain dir is not the sync source" before any symlink or real-directory check runs. The test would pass on any path.
  • Flip: replace readContained(t) in notes.Edit with os.ReadFile(t.Abs). All notes tests stay green.
  • With a .git dir added, the same test goes red under that flip, so the fix is small: create .git and add a positive control (the same edit through a real directory succeeds).
  • The PR body cites this test as the guard for brain_edit's rename. As written, it guards nothing.

B2. internal/brainfs/brainfs_test.go: in TestAppendRegularCreatesOrAppendsOnlyThroughRealDirectories, the unclean-path case is masked.

  • The case raw/../log.md is refused because log.md is a symlink. It never reaches cleanRel.
  • Flip: disable cleanRel in AppendRegular. Every PR test stays green.
  • That cleanRel matters. Without it, an unclean path with .. after an in-brain directory symlink appends to a different regular file in the brain. I verified this with a scratch test: the content of the other file changed.
  • No current caller passes an unclean path: both callers use constants. But the primitive's guard has no test.
  • Fix: an unclean case that resolves to a regular file, so cleanRel is the only thing that can refuse it.

B3. Brain.Remove has no test.

  • Its doc comment says it refuses "a path behind a symlinked directory".
  • Flip: drop the RealParent call from Remove. Everything stays green.
  • Through promote and reject it cannot be observed, because readRawSource refuses a symlinked raw/ first.
  • Fix: a direct brainfs test (a symlinked parent is refused and the target survives; a final symlink is removed, not its target).

Run end to end: all clean

I called the MCP handlers through ServeHTTP: brain_write, brain_promote, brain_reject and brain_edit. I also called StampProvenanceFile, wiki.RebuildIndex, wiki.AppendLog and brain.BuildWingIndex directly.

Planted shapes:

  • a symlinked wing or hall, with in-brain, outside and dangling targets
  • a final symlink, with in-brain, outside, dangling-in and dangling-out targets
  • a FIFO with no peer, and one held open by a writer
  • a symlinked _index.md, wiki/index.md, log.md and tunnel-candidate log
  • a symlinked raw/ and raw/rejected
  • a .. after a link

Results:

  • Every planted shape was refused. I snapshotted the whole brain and the outside dir before and after each call: zero changes at any target, and no hangs (5 s timeout).
  • The raw source was kept in every refused promote or reject.
  • A refused _index.md or tunnel-log write leaves the note written. The error is logged, as before.

Regressions: none found

  • brain_write: a new wing and hall get their directories created; overwriting an existing note works.
  • Promote and reject: collisions are refused and the source is kept.
  • Logs: log.md and the tunnel-candidate log grow across calls, and the header is written once.
  • Brain dir shapes: all of the above also holds with the brain dir as a mount symlink and with a trailing slash.

Primitive semantics (go1.27.1)

  • O_EXCL through os.Root: it refuses a live symlink, a dangling in-root one and a dangling outside one (EEXIST), and creates nothing at the target. The PutRegular create path is sound.
  • AppendRegular's O_CREATE open: it does create an empty file at a dangling in-brain symlink's target. That open is reachable only if the entry is swapped between the pre-Lstat and the open, because a static dangling link is refused by the pre-check. That is better than session.Append on main, which has no pre-check. Non-blocking. The fix would be an O_EXCL create when the Lstat said "absent", then an append.
  • mkdirReal race: if a component is swapped for an in-brain directory symlink between its Lstat and the child's Mkdir, the directory is created at the link target. The follow-up Lstat follows that intermediate link, so the note is written there too. Outside targets are refused by os.Root. This is the #167 residual, as the doc comment says.

Non-blocking

  1. Race guards with no test.
    • Removing O_EXCL alone, or O_EXCL and SameFile together, from PutRegular's create path leaves every test green. The PR body mentions only SameFile.
    • Removing SameFile from AppendRegular is also green.
    • These guards only matter under a race. A wrapAfterStat-style seam between the Lstat and the open would make them testable.
  2. Removing removeBrainFile (back to os.Remove(src)) is green. Expected, for the B3 reason.
  3. Stamp flips. Reverting StampProvenanceFile's read alone, or its write alone, to path I/O is green. Only reverting both goes red, because the read guard covers the static case.
  4. gofmt.
    • internal/provenance/provenance.go: two blank lines are left where ApplyFile was removed (gofmt -l flags it).
    • internal/wiki/log.go: the module import is mixed into the stdlib import group.
    • The gate catches neither.
  5. _ = wingDir in brain/index.go keeps a dead variable alive. Delete it.
  6. Promote collision check. It still uses os.Stat(dest), and PutRegular then replaces an existing file. A file created between the two is overwritten, as before this PR. The create-only O_EXCL path would fit promote's "never silently overwrite" contract better.
  7. Path I/O left in the brain_edit flow. Pre-existing, outside this diff, and not in #158 part B's list, so it needs its own issue:
    • The snapshot read at internal/mcp/tools_notes.go:121 (os.ReadFile(target.Abs)) hangs on a FIFO held open by a writer. Verified through the MCP handler.
    • The rollback write at tools_notes.go:134 is by path.
    • The flock file in internal/notes/edit.go:225 is opened by path with O_CREATE. A planted symlink at the lock path lets a successful brain_edit create an empty file at the link target, including outside the brain. Verified.

Flip results

Red:

  • writeHallNote reverted to path I/O
  • MkdirAll before PutRegular
  • promote by path
  • reject by path
  • StampProvenanceFile read and write both by path
  • BuildWingIndex
  • RebuildIndex
  • AppendLog
  • the tunnel log
  • mkdirReal replaced by root.MkdirAll
  • mkdirReal without its IsDir check
  • PutRegular without the replace branch
  • PutRegular without cleanRel
  • AppendRegular without its pre-check
  • RealParent without cleanRel

Green (survivors):

  • PutRegular without O_EXCL, and without O_EXCL+SameFile
  • AppendRegular without SameFile
  • AppendRegular without cleanRel (B2)
  • Remove without RealParent (B3)
  • os.Remove(src) in place of removeBrainFile
  • the stamp read or write alone
  • notes.Edit without its read check (B1)

Gates

  • CI=1 task check at the PR head exits 0, with 46 packages ok.
  • go test -race -count=3 on brainfs, api, brain, wiki and notes: ok.
  • CI run 9490 (Lint / Test / Vet) succeeded on 138d6e2.
  • Green, and green covers none of B1–B3.

Done-when

#188's done-when is met: TestWriteNoteRefusesASymlinkedWing covers in-brain and outside targets, and it goes red when writeHallNote is reverted to os.MkdirAll + os.WriteFile. The block is B1–B3 only.

Not checked

  • Hard links (an accepted residual).
  • Live races beyond the primitive demonstrations above (no seam exists).
  • Behaviour on the pod's real mount.
  • The HTTP POST /write handler. It reaches the same WriteNote, which I exercised through MCP and directly.
  • #158 part B paths.
## Adversarial review: PR #189 (head 138d6e2). Verdict: BLOCKED I tested this by running it, not by reading it. The production code held up: no entry point created or changed anything at a link target, nothing hung, and the raw source survived every refused promote or reject. The block is on three tests that pass whether or not the behaviour they name is there. Rules 8 and 9 apply: a check that cannot go red is decoration. ### Blocking **B1. `internal/notes/edit_redirect_test.go`: `TestEditRefusesANoteBehindASymlinkedDirectory` is decoration.** - The temp brain has no `.git`, so `notes.Edit` refuses with "brain dir is not the sync source" before any symlink or real-directory check runs. The test would pass on any path. - Flip: replace `readContained(t)` in `notes.Edit` with `os.ReadFile(t.Abs)`. All notes tests stay green. - With a `.git` dir added, the same test goes red under that flip, so the fix is small: create `.git` and add a positive control (the same edit through a real directory succeeds). - The PR body cites this test as the guard for brain_edit's rename. As written, it guards nothing. **B2. `internal/brainfs/brainfs_test.go`: in `TestAppendRegularCreatesOrAppendsOnlyThroughRealDirectories`, the unclean-path case is masked.** - The case `raw/../log.md` is refused because `log.md` is a symlink. It never reaches `cleanRel`. - Flip: disable `cleanRel` in `AppendRegular`. Every PR test stays green. - That `cleanRel` matters. Without it, an unclean path with `..` after an in-brain directory symlink appends to a different regular file in the brain. I verified this with a scratch test: the content of the other file changed. - No current caller passes an unclean path: both callers use constants. But the primitive's guard has no test. - Fix: an unclean case that resolves to a regular file, so `cleanRel` is the only thing that can refuse it. **B3. `Brain.Remove` has no test.** - Its doc comment says it refuses "a path behind a symlinked directory". - Flip: drop the `RealParent` call from `Remove`. Everything stays green. - Through promote and reject it cannot be observed, because `readRawSource` refuses a symlinked `raw/` first. - Fix: a direct brainfs test (a symlinked parent is refused and the target survives; a final symlink is removed, not its target). ### Run end to end: all clean I called the MCP handlers through `ServeHTTP`: brain_write, brain_promote, brain_reject and brain_edit. I also called `StampProvenanceFile`, `wiki.RebuildIndex`, `wiki.AppendLog` and `brain.BuildWingIndex` directly. Planted shapes: - a symlinked wing or hall, with in-brain, outside and dangling targets - a final symlink, with in-brain, outside, dangling-in and dangling-out targets - a FIFO with no peer, and one held open by a writer - a symlinked `_index.md`, `wiki/index.md`, `log.md` and tunnel-candidate log - a symlinked `raw/` and `raw/rejected` - a `..` after a link Results: - Every planted shape was refused. I snapshotted the whole brain and the outside dir before and after each call: zero changes at any target, and no hangs (5 s timeout). - The raw source was kept in every refused promote or reject. - A refused `_index.md` or tunnel-log write leaves the note written. The error is logged, as before. ### Regressions: none found - **brain_write:** a new wing and hall get their directories created; overwriting an existing note works. - **Promote and reject:** collisions are refused and the source is kept. - **Logs:** `log.md` and the tunnel-candidate log grow across calls, and the header is written once. - **Brain dir shapes:** all of the above also holds with the brain dir as a mount symlink and with a trailing slash. ### Primitive semantics (go1.27.1) - **`O_EXCL` through os.Root:** it refuses a live symlink, a dangling in-root one and a dangling outside one (EEXIST), and creates nothing at the target. The `PutRegular` create path is sound. - **`AppendRegular`'s `O_CREATE` open:** it does create an empty file at a dangling in-brain symlink's target. That open is reachable only if the entry is swapped between the pre-`Lstat` and the open, because a static dangling link is refused by the pre-check. That is better than `session.Append` on main, which has no pre-check. Non-blocking. The fix would be an `O_EXCL` create when the `Lstat` said "absent", then an append. - **mkdirReal race:** if a component is swapped for an in-brain directory symlink between its `Lstat` and the child's `Mkdir`, the directory is created at the link target. The follow-up `Lstat` follows that intermediate link, so the note is written there too. Outside targets are refused by os.Root. This is the #167 residual, as the doc comment says. ### Non-blocking 1. **Race guards with no test.** - Removing `O_EXCL` alone, or `O_EXCL` and `SameFile` together, from `PutRegular`'s create path leaves every test green. The PR body mentions only `SameFile`. - Removing `SameFile` from `AppendRegular` is also green. - These guards only matter under a race. A `wrapAfterStat`-style seam between the `Lstat` and the open would make them testable. 2. **Removing `removeBrainFile` (back to `os.Remove(src)`) is green.** Expected, for the B3 reason. 3. **Stamp flips.** Reverting `StampProvenanceFile`'s read alone, or its write alone, to path I/O is green. Only reverting both goes red, because the read guard covers the static case. 4. **gofmt.** - `internal/provenance/provenance.go`: two blank lines are left where `ApplyFile` was removed (`gofmt -l` flags it). - `internal/wiki/log.go`: the module import is mixed into the stdlib import group. - The gate catches neither. 5. **`_ = wingDir` in `brain/index.go`** keeps a dead variable alive. Delete it. 6. **Promote collision check.** It still uses `os.Stat(dest)`, and `PutRegular` then replaces an existing file. A file created between the two is overwritten, as before this PR. The create-only `O_EXCL` path would fit promote's "never silently overwrite" contract better. 7. **Path I/O left in the brain_edit flow.** Pre-existing, outside this diff, and not in #158 part B's list, so it needs its own issue: - The snapshot read at `internal/mcp/tools_notes.go:121` (`os.ReadFile(target.Abs)`) **hangs on a FIFO held open by a writer**. Verified through the MCP handler. - The rollback write at `tools_notes.go:134` is by path. - The flock file in `internal/notes/edit.go:225` is opened by path with `O_CREATE`. A planted symlink at the lock path lets a successful brain_edit create an empty file at the link target, including outside the brain. Verified. ### Flip results **Red:** - writeHallNote reverted to path I/O - MkdirAll before `PutRegular` - promote by path - reject by path - `StampProvenanceFile` read and write both by path - `BuildWingIndex` - `RebuildIndex` - `AppendLog` - the tunnel log - `mkdirReal` replaced by `root.MkdirAll` - `mkdirReal` without its `IsDir` check - `PutRegular` without the replace branch - `PutRegular` without `cleanRel` - `AppendRegular` without its pre-check - `RealParent` without `cleanRel` **Green (survivors):** - `PutRegular` without `O_EXCL`, and without `O_EXCL`+`SameFile` - `AppendRegular` without `SameFile` - `AppendRegular` without `cleanRel` (B2) - `Remove` without `RealParent` (B3) - `os.Remove(src)` in place of `removeBrainFile` - the stamp read or write alone - `notes.Edit` without its read check (B1) ### Gates - `CI=1 task check` at the PR head exits 0, with 46 packages ok. - `go test -race -count=3` on brainfs, api, brain, wiki and notes: ok. - CI run 9490 (Lint / Test / Vet) succeeded on 138d6e2. - Green, and green covers none of B1–B3. ### Done-when #188's done-when is met: `TestWriteNoteRefusesASymlinkedWing` covers in-brain and outside targets, and it goes red when writeHallNote is reverted to `os.MkdirAll` + `os.WriteFile`. The block is B1–B3 only. ### Not checked - Hard links (an accepted residual). - Live races beyond the primitive demonstrations above (no seam exists). - Behaviour on the pod's real mount. - The HTTP `POST /write` handler. It reaches the same `WriteNote`, which I exercised through MCP and directly. - #158 part B paths.
mathias added 1 commit 2026-10-05 20:57:28 +00:00
test: pin the edit symlink refusal, AppendRegular's clean check and Remove (#188)
CI / Lint / Test / Vet (pull_request) Failing after 1m6s
CI / Mirror to GitHub (pull_request) Skipped
db18785bfc
Review round 1 on PR #189 (three tests stayed green with their guard
removed):
- TestEditRefusesANoteBehindASymlinkedDirectory had no .git, so Edit
  refused "not the sync source" before any path check. It now has one,
  plus a positive control.
- AppendRegular's unclean-path case was refused by a final symlink, not
  cleanRel. New test where ".." after a symlinked dir resolves to a real
  file, for AppendRegular and PutRegular.
- brainfs.Remove had no test; new one (refuses behind a symlinked dir,
  removes a final link but never its target, positive control).

Also: promote/reject create with O_EXCL (new brainfs.CreateRegular), so a
note that appears after the collision check is never overwritten;
gofmt fixes in provenance.go and wiki/log.go; dead wingDir and
putBrainFile removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

Adversarial review, round 2: PR #189 (head db18785). Verdict: BLOCKED

Round-1 blockers: all three are fixed, each confirmed by a flip

  • B1, edit test: with notes.Edit reading via os.ReadFile(t.Abs), TestEditRefusesANoteBehindASymlinkedDirectory goes red, and the positive control is real.
  • B2, unclean path: with cleanRel removed from AppendRegular, TestAppendRegularRefusesAnUncleanPathThatResolvesToARealFile goes red.
  • B3, Remove: with RealParent removed from Remove, TestRemoveRefusesAPathBehindASymlinkedDirectoryAndRemovesOnlyTheLink goes red.

New blocking finding: N1, CreateRegular has no test, and its parent check can be removed unnoticed

CreateRegular (internal/brainfs/brainfs.go) is new in this round and is now the only write path for promote and reject. The whole suite stays green under every flip I tried on it:

Flip Result
mkdirReal replaced by root.MkdirAll green
cleanRel disabled green
CreateRegular delegating to PutRegular (replace, not create-only) green
createBrainFile calling PutRegular green

The first flip is a real regression. I verified it through the MCP handlers, using an in-brain target for the symlinked wing and raw/rejected:

  • With that flip, brain_promote into a symlinked wing writes the promoted note at the link's in-brain target and deletes the raw source.
  • brain_reject with a symlinked raw/rejected writes into the link target in the same way.
  • Unmodified db18785 refuses both and keeps the source.

So the PR code is correct, but nothing would notice it regressing. TestPromoteAndRejectRefuseASymlinkedDestinationDirectory plants only outside targets, and os.Root refuses those whatever the parent check does. This is the same pattern as round 1's B3.

Fix:

  • Add a direct brainfs test for CreateRegular:
    • an in-brain symlinked parent is refused, and nothing is created at the target
    • an unclean path is refused
    • an existing file and a dangling link are refused with fs.ErrExist, and the existing content is unchanged
    • a positive control that creates a file with missing parents
  • Give the promote/reject test an in-brain target as well as the outside one.

Blocking: N2, CI is red on db18785

  • Run 9497 (CI / Lint / Test / Vet) failed in internal/symlinkguard: TestWatchDeliversEveryQueuedAlertAfterAShortOutage asserted an alert ordering.
  • This PR does not touch that package. Locally the test fails 1 in 100 under -race (0 in 200 without), which matches open issue #193.
  • So this is almost certainly the known flake, not this change. But a red run on the head is an automatic block. Re-run CI and get it green.

Everything else holds at db18785

  • End to end: I re-ran the full set of MCP shapes (symlinked wing or hall, final symlink, dangling links in and out, FIFOs with and without a writer, symlinked index and log files, symlinked raw/ and raw/rejected). There were zero changes at any target, no hangs, and the source was kept on every refusal.
  • O_EXCL now applies to promote and reject: a dangling destination link is refused with "file exists".
  • Regressions: none. New wing, overwriting a note, collisions, log growth, the brain dir as a mount symlink and with a trailing slash all behave as before.
  • Gates: CI=1 task check at db18785 exits 0 (46 packages ok). go test -race -count=3 on brainfs, api, brain, wiki and notes passes.

Accepted as disclosed

  • The race-only guards have no seam test: PutRegular's and createNew's O_EXCL/SameFile, and AppendRegular's SameFile.
  • AppendRegular's O_CREATE on a swapped-in dangling link is the #167 class.
  • The brain_edit path I/O is #192.

Not checked

  • Hard links.
  • Live races.
  • The pod's real mount.
  • #158 part B.
## Adversarial review, round 2: PR #189 (head db18785). Verdict: BLOCKED ### Round-1 blockers: all three are fixed, each confirmed by a flip - **B1, edit test:** with `notes.Edit` reading via `os.ReadFile(t.Abs)`, `TestEditRefusesANoteBehindASymlinkedDirectory` goes red, and the positive control is real. - **B2, unclean path:** with `cleanRel` removed from `AppendRegular`, `TestAppendRegularRefusesAnUncleanPathThatResolvesToARealFile` goes red. - **B3, Remove:** with `RealParent` removed from `Remove`, `TestRemoveRefusesAPathBehindASymlinkedDirectoryAndRemovesOnlyTheLink` goes red. ### New blocking finding: N1, `CreateRegular` has no test, and its parent check can be removed unnoticed `CreateRegular` (`internal/brainfs/brainfs.go`) is new in this round and is now the only write path for promote and reject. The whole suite stays green under every flip I tried on it: | Flip | Result | |---|---| | `mkdirReal` replaced by `root.MkdirAll` | green | | `cleanRel` disabled | green | | `CreateRegular` delegating to `PutRegular` (replace, not create-only) | green | | `createBrainFile` calling `PutRegular` | green | The first flip is a real regression. I verified it through the MCP handlers, using an in-brain target for the symlinked wing and `raw/rejected`: - With that flip, brain_promote into a symlinked wing writes the promoted note at the link's in-brain target and **deletes the raw source**. - brain_reject with a symlinked `raw/rejected` writes into the link target in the same way. - Unmodified db18785 refuses both and keeps the source. So the PR code is correct, but nothing would notice it regressing. `TestPromoteAndRejectRefuseASymlinkedDestinationDirectory` plants only **outside** targets, and os.Root refuses those whatever the parent check does. This is the same pattern as round 1's B3. **Fix:** - Add a direct brainfs test for `CreateRegular`: - an in-brain symlinked parent is refused, and nothing is created at the target - an unclean path is refused - an existing file and a dangling link are refused with `fs.ErrExist`, and the existing content is unchanged - a positive control that creates a file with missing parents - Give the promote/reject test an in-brain target as well as the outside one. ### Blocking: N2, CI is red on db18785 - Run 9497 (CI / Lint / Test / Vet) failed in `internal/symlinkguard`: `TestWatchDeliversEveryQueuedAlertAfterAShortOutage` asserted an alert ordering. - This PR does not touch that package. Locally the test fails 1 in 100 under `-race` (0 in 200 without), which matches open issue #193. - So this is almost certainly the known flake, not this change. But a red run on the head is an automatic block. Re-run CI and get it green. ### Everything else holds at db18785 - **End to end:** I re-ran the full set of MCP shapes (symlinked wing or hall, final symlink, dangling links in and out, FIFOs with and without a writer, symlinked index and log files, symlinked `raw/` and `raw/rejected`). There were zero changes at any target, no hangs, and the source was kept on every refusal. - **O_EXCL now applies to promote and reject:** a dangling destination link is refused with "file exists". - **Regressions:** none. New wing, overwriting a note, collisions, log growth, the brain dir as a mount symlink and with a trailing slash all behave as before. - **Gates:** `CI=1 task check` at db18785 exits 0 (46 packages ok). `go test -race -count=3` on brainfs, api, brain, wiki and notes passes. ### Accepted as disclosed - The race-only guards have no seam test: `PutRegular`'s and `createNew`'s `O_EXCL`/`SameFile`, and `AppendRegular`'s `SameFile`. - `AppendRegular`'s `O_CREATE` on a swapped-in dangling link is the #167 class. - The brain_edit path I/O is #192. ### Not checked - Hard links. - Live races. - The pod's real mount. - #158 part B.
mathias added 1 commit 2026-10-05 21:19:04 +00:00
test: pin CreateRegular directly and promote/reject with in-brain targets (#188)
CI / Lint / Test / Vet (pull_request) Successful in 1m3s
CI / Mirror to GitHub (pull_request) Skipped
ac84ddf5a9
Review round 2 on PR #189: CreateRegular (promote's and reject's only
write path) had no test; replacing mkdirReal with root.MkdirAll, dropping
cleanRel or calling PutRegular all stayed green, and the promote/reject
test planted only outside targets, which os.Root refuses on its own.

- TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories: in-brain
  symlinked parent, unclean path, existing file and dangling link
  (fs.ErrExist, nothing changed), positive control with missing parents.
- TestPromoteAndRejectRefuseAnInBrainSymlinkedDestinationDirectory.

All three flips are red now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

Adversarial review, round 3: PR #189 (head ac84ddf). Verdict: PASS

This round changes only tests. The production code is the same as db18785, which passed the full end-to-end shape sweep in round 2.

Round-2 blockers are fixed

N1, CreateRegular had no test. Every flip I tried on it now goes red:

Flip Result
mkdirReal replaced by root.MkdirAll red: TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories and the new in-brain promote/reject test
cleanRel disabled red
CreateRegular delegating to PutRegular red
O_EXCL dropped from createNew red (the dangling-link case catches it)

N2, red CI. Run 9506 on ac84ddf: Lint / Test / Vet passed (the GitHub mirror job was skipped). Run 9497 was the #193 symlinkguard flake, in a package this PR doesn't touch.

Earlier pins still hold

Each of these flips still turns its test red:

  • notes.Edit without its read check
  • AppendRegular without cleanRel
  • Remove without RealParent
  • mkdirReal without its IsDir check (five tests go red)

Gates

  • CI=1 task check at ac84ddf exits 0, with 46 packages ok.
  • go test -race -count=3 on brainfs, api, brain, wiki and notes passes.
  • #188's done-when is met.

Non-blocking, recorded for the trail

  • Promote and reject could revert to PutRegular unnoticed. If createBrainFile calls PutRegular instead of CreateRegular, every test stays green. The collision check before the write catches the static case, so create-only matters only for a note that appears between that check and the write. That is the same race-only class as the disclosed O_EXCL/SameFile guards, which have no seam test.
  • Already accepted: AppendRegular's O_CREATE on a swapped-in dangling link is the #167 class, and the brain_edit path I/O is #192.

Not checked

  • Hard links.
  • Live races.
  • The pod's real mount.
  • #158 part B.

Merge is Mathias's call: it deploys ingestion.

## Adversarial review, round 3: PR #189 (head ac84ddf). Verdict: PASS This round changes only tests. The production code is the same as db18785, which passed the full end-to-end shape sweep in round 2. ### Round-2 blockers are fixed **N1, `CreateRegular` had no test.** Every flip I tried on it now goes red: | Flip | Result | |---|---| | `mkdirReal` replaced by `root.MkdirAll` | red: `TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories` and the new in-brain promote/reject test | | `cleanRel` disabled | red | | `CreateRegular` delegating to `PutRegular` | red | | `O_EXCL` dropped from `createNew` | red (the dangling-link case catches it) | **N2, red CI.** Run 9506 on ac84ddf: Lint / Test / Vet passed (the GitHub mirror job was skipped). Run 9497 was the #193 symlinkguard flake, in a package this PR doesn't touch. ### Earlier pins still hold Each of these flips still turns its test red: - `notes.Edit` without its read check - `AppendRegular` without `cleanRel` - `Remove` without `RealParent` - `mkdirReal` without its `IsDir` check (five tests go red) ### Gates - `CI=1 task check` at ac84ddf exits 0, with 46 packages ok. - `go test -race -count=3` on brainfs, api, brain, wiki and notes passes. - #188's done-when is met. ### Non-blocking, recorded for the trail - **Promote and reject could revert to `PutRegular` unnoticed.** If `createBrainFile` calls `PutRegular` instead of `CreateRegular`, every test stays green. The collision check before the write catches the static case, so create-only matters only for a note that appears between that check and the write. That is the same race-only class as the disclosed `O_EXCL`/`SameFile` guards, which have no seam test. - **Already accepted:** `AppendRegular`'s `O_CREATE` on a swapped-in dangling link is the #167 class, and the brain_edit path I/O is #192. ### Not checked - Hard links. - Live races. - The pod's real mount. - #158 part B. Merge is Mathias's call: it deploys ingestion.
mathias merged commit f4636c7ef0 into main 2026-10-05 21:25:54 +00:00
Sign in to join this conversation.