Skip to content

Keep registered scratch writable below an enclosing source tree - #3835

Merged
1 commit merged into
mainfrom
fix/pi-toad-errors-scratch-source
Nov 1, 2026
Merged

1 commit merged into
mainfrom
fix/pi-toad-errors-scratch-source

Conversation

@trissim

@trissim trissim commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

2 files, +36/-8; one production classification decision fixed, 4 regression cases now explicit. An enclosing Git worktree no longer steals a thread's registered scratch allocation. A Git repository rooted at or inside that scratch remains protected source and still needs an edit allocation.

Initially source-only delivery for pi-commander's storage RED from #3831; now merged and installed through combined #3836 at6e3a2a3a1e4eb3dbcc2911775ea81549cec30abb. The coherent pair's affected controls completed 8/8,4.76s,rc0 and installed live-registry preflight admission/refusal completed0. Follow-up to #3834; its canonical containment implementation is unchanged. This is not a fixture-path exception or a claim of full native/frontend acceptance.

Defect and owner

StorageRule.order_text declares registry-assigned scratch as writable; StorageRule.require_destination separately forbids foreign scratch and .artifacts. StorageProjection.require_write authenticates the current writer and determines which source boundaries require an edit allocation.

Before: the projection removed owned scratch from in_project but independently treated every enclosing Git root as source. With an allocated test basetemp inside a Git checkout, the existing homed test's audit path was therefore refused. Exactly the same layout can occur outside tests. The fixture correctly asks to write its declared scratch; teaching it to hide the outer checkout would hide the production bug.

After: following destination and registered-writer validation, the projection makes the owned-scratch boundary decision once. A more distant Git ancestor cannot turn that allocation into read/launch source. A Git root at or below scratch prevents this shortcut and still follows the unchanged source-allocation checks. The old in_project scratch exception is deleted.

No new class, roster, basename check, writable store, source-root suppression or Git mock. Checkout.git_roots, registry assignment, canonical resolution and edit-allocation ownership retain their existing owners. The shortcut cannot bypass .artifacts, foreign-scratch refusal or missing-writer refusal. Canonical resolution prevents a link back into the read-only project from inheriting scratch permission.

This does not turn StorageRule into a general filesystem sandbox or change unrelated non-source write policy.

Original evidence

The exact #3831 integration head was 2c8a6d5521184f9a98bdb1f4d16ad947952c9fdf, containing unchanged #3834. The retained run completed 152 passed, 2 failed, 13 deselected, 154 subtests passed in 17.06s. Its storage failure was:

tests/test_storage_rules.py::test_a_thread_registered_above_its_scratch_may_write_its_scratch_and_only_that

at StorageProjection.require_write, storage_rules.py:702. The other failure is Shell's separately owned empty merge-hook output, not cleared by this repair.

Artifact: /home/ts/.cache/agent-scratch/pi-commander/heartbeat-publication/shell-3831-combined-1.log.

Source relationships

Fresh baseline e26a1897b1c8eff5fa76f06da96e764e10b02179; branch pushed and remote=HEAD verified before edits. Open-PR file census found no overlap in storage_rules/thread_storage/storage tests. #3609 touches repo.py; this change does not.

Existing 75-module storage source map retained. Targeted original-source closure query reuses NRA SourceModule / ModuleSyntaxIndex for storage_rules, thread_storage, repo, shell_effects, native_tool_call and storage tests: 6 modules, 112 original ClassDefs, 2,238 import/write/decision/call sites, zero parse omissions. Original positions, bases, enclosing executable owners and source hashes are retained in scratch-source-map.json; after-source receipt is scratch-source-map-after.json. Dynamic resolution remains OPEN. This is source evidence, not execution equivalence. One-shot query code is removed before delivery.

Verification and installed scope

  • Both changed files compile; git diff --check passes. Receipt: scratch-source-codec.{log,rc}, rc0.
  • No pytest, native/model/frontend launch, new Comms root or integration job was run by this author for this repair.
  • The existing positive test now explicitly covers both ordinary and enclosing-Git layouts, retaining project/foreign-scratch refusals and adding .artifacts and a symlink back into the read-only project.
  • Two negative cases use real Git roots at scratch itself and in a nested directory; both must remain refused without a source edit allocation. No patching of owned code.

The integration owner completed the coherent hook/storage selection at66b96aeec07b600fb2331800ab2daad672a99d90: 8passed,3warnings,4.76s,rc0, deliberately retaining the Git-nested basetemp and asserting source module origins/equality. Artifacts shell-storage-combined-2.{log,rc} in /home/ts/.cache/agent-scratch/pi-commander/heartbeat-publication/ were independently read. The original152-case red remains immutable; it was not rerun for reassurance or relabeled as passing.

Forge readback confirms both3835 and3836 MERGED at6e3a2a3a. The existing single installer completed 93phases/68.5s/rc0,0pending carries/reconciliation changes,792identical files/0rewrites,2881threads. All seven owners restarted, Connector18766 and the session bridge moved; protected old Toads were not signalled. Actual installer log/rc: shell-storage-live-install.{log,rc}.

Independently read the current symlink and build manifest: runtime core-6e3a2a3a1-lock-b7cd09e875fefab3, Core6e3a2a3a, unchanged Toad ec224a6c4. Installed StorageProjection.require_write at storage_rules.py:684 is AST-identical to this PR's owned source; receipt /home/ts/.cache/agent-scratch/pi-toad-errors/scratch-installed-source-readback.json. This is installed-source correspondence, not a behavioral run.

The integrator's actual installed StorageProjection on the live registry admitted registered pi-commander scratch and refused foreign scratch plus .artifacts, rc0; shell-storage-installed-projection.{json,log,rc} was independently read. Its scope is preflight only: no destination file, root or input was created. Salvage's fresh answer52374 to finished-install52373 demonstrates post-install communication; it does not establish frontend behavior.

This closes the narrow source/installed-preflight repair, not expandedToad1389, original-coordinate retention, donor-worker qualification, mobile/native/physical rendering or a write performed through the full native tool path. Those remain with their existing owners. No new pytest/root/model/frontend/native launch or failed-input replay by this author.

Assumed: filesystem membership remains stable over one preflight (the existing point-in-time contract). Existing policy, assignment and Git roots are not migrated, rewritten or hidden. The original failure and failed/UNKNOWN session evidence remain untouched. No peer or live checkout was edited.

The registry gave scratch a room; the ancestor Git checkout tried to evict it. A nearer repository still has its own locked door.

Copilot AI balanced review requested due to automatic review settings November 1, 2026 10:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim trissim closed this pull request by merging all changes into main in 6e3a2a3 Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants