Plan 282fabbb (Filbert) and the six adversarial rounds (Rocko, r6 80cde839). Lead item 15: Gate F first, then A1 and A2 as separate reviewed commits. Co-Authored-By: Claude Opus 5.5 <[email protected]>
134 lines
7.9 KiB
Markdown
134 lines
7.9 KiB
Markdown
# Queue as data — adversarial review, round 5
|
|
|
|
Verdict: **revise**. Rocko for Sage, 2026-09-26.
|
|
|
|
Plan hash verified before reading:
|
|
`889f25665b42f620a82051e916b017e26e2aeb5c5ef4a4549e327f796b1ae3d3`.
|
|
This supersedes 7d61f18d. Review target is active section 8, against round-4
|
|
report `fcb8933d515bcf98c509d17cb7c5bb384f2bce844b2b87eab6f83e67658efcff`.
|
|
Sage's choices and credential rulings are accepted. Two bounded fixes
|
|
remain; neither needs a new owner decision or architecture.
|
|
|
|
## Round-four disposition
|
|
|
|
| Finding | Assessment |
|
|
|---|---|
|
|
| F1 HEAD/shared-index gap | The installed, active pre-commit guard closes the ordinary-commit rollback schedule. Cooperative exclusions are explicit in 8.5. Its active status needs the additional validation in finding 2 below. |
|
|
| F2 incoherent reads | Resolved. Witness-first reading plus a locked recheck before adverse diagnosis avoids falsely declaring lost history during a normal publication. An ahead revision is only visible/unconfirmed. Snapshot verify explicitly does not certify live witness continuity. |
|
|
| F3 genesis/bootstrap | Resolved at plan level. Implementation-first commit, genesis-only first queue commit, base-absent exception, explicit base bytes from H and a nonexistent temporary-index path are concrete. |
|
|
| F4 freshness/branch coaching | The intended proof closes ordinary resume and abandoned-branch coaching, but the newly prescribed pre-instruction file pin cannot be obtained from pinned Pi. See finding 1. |
|
|
| F5 op-id length | Resolved. External maximum 72 plus reserved eight-character suffix fits internal maximum 80; boundaries have tests. |
|
|
|
|
## 1. Medium — Gate G requires evidence Pi has not written yet, and its first-parent rule rejects a fresh session
|
|
|
|
**Where:** 8.11 freshness and linear-file bullets.
|
|
|
|
The plan requires the new session file/header to be pinned before Jason's
|
|
instruction, containing only header and launch settings. Pinned Pi's
|
|
SessionManager creates these entries in memory but defers file creation
|
|
until an assistant message exists. Even appending the initial user message
|
|
does not create the file. The launcher does not pre-create it.
|
|
|
|
Separately, the session header has a session id, but is not the parent of
|
|
the first tree entry. That entry has parentId null. “Every entry's parentId
|
|
must be the previous entry's id” fails on a valid session immediately after
|
|
the header. Subsequent tree entries should form the requested linear chain.
|
|
|
|
**Executed evidence:** imported the repository's installed
|
|
`@earendil-works/pi-coding-agent/dist/core/session-manager.js`, created a
|
|
session in a temporary directory, appended model/thinking settings and a
|
|
user instruction, then a synthetic assistant message. Results:
|
|
|
|
- before instruction: session file absent;
|
|
- after instruction: session file absent;
|
|
- after assistant: file present;
|
|
- first nonheader entry parentId is null, not header.id.
|
|
|
|
Source: `_persist` at line 739 and `newSession` around line 650. This used
|
|
no model, live session or credentials; the temporary fixture was removed.
|
|
|
|
**Fix:** pin the pre-launch directory listing, launch snapshot, launch
|
|
arguments and start time before the instruction. Pin the completed session
|
|
file at cutoff (or once Pi first creates it, with a final cutoff pin).
|
|
Audit the resulting header timestamp, absence of parentSession and the
|
|
prefix before the instruction retrospectively against those pre-launch
|
|
receipts. Do not introduce a warm-up message or manually seed Pi's session
|
|
file. State that the first nonheader entry has parentId null and every
|
|
later tree entry points to its preceding tree entry. The header's id is
|
|
session identity, not an entry-chain node.
|
|
|
|
**Acceptance:** run the checklist on a real fresh session generated by the
|
|
pinned Pi; it must pass without a pre-instruction model turn. Retain the
|
|
ordinary-resume, prior-compaction and branching negative controls.
|
|
|
|
## 2. Medium — matching hook bytes does not prove the commit guard will run
|
|
|
|
**Where:** 8.12 installation and step-1 guard.
|
|
|
|
Installation writes mode 0755 and refuses core.hooksPath. Each subsequent
|
|
queue commit checks only that `.git/hooks/pre-commit` bytes match HEAD.
|
|
If the executable bit is lost during a hook copy/restore, the bytes still
|
|
match, but Git ignores the hook. Likewise a later core.hooksPath setting
|
|
can select a different location while the checked file remains unchanged.
|
|
No malicious edit or --no-verify flag is needed on the ordinary commit.
|
|
|
|
**Executed schedule in an isolated temporary repository:** HEAD/index
|
|
queue=10, unrelated source staged, private-index commit publishes queue=12
|
|
without reconciling the shared index. With the executable guard, plain
|
|
`git commit` refuses. Remove only the hook executable bit, preserving its
|
|
bytes; the same plain commit succeeds and HEAD queue becomes 10.
|
|
|
|
**Fix:** before publication, verify both expected bytes and that the hook
|
|
is executable and selected by the effective Git configuration/environment
|
|
used by ordinary committers. Recheck core.hooksPath at invocation, not only
|
|
installation. Refuse before publishing if the guard is inactive. Explicitly
|
|
include disabling/replacing hooks or overriding their selection among the
|
|
cooperative bypasses seats must not perform during integration. No software
|
|
check can prevent a deliberate later same-user bypass; that accepted trust
|
|
limit remains. Do not represent bytes alone as an active maintenance hold.
|
|
|
|
**Acceptance:** same-byte nonexecutable hook and a newly set alternate
|
|
hooksPath both make queue-commit refuse before update-ref. Keep the existing
|
|
ordinary-commit race tests with an active hook.
|
|
|
|
## What is fine, and the remaining bounds
|
|
|
|
The cooperative model is now stated honestly in 8.5: ordinary commits use
|
|
the guard; --no-verify is forbidden; merge, rebase, cherry-pick, revert and
|
|
am are lead operations because the guard does not protect them. These are
|
|
routine Git operations that bypass this protection, so the lead must apply
|
|
the recovery/integration discipline too. “Lead-only” is not an enforcement
|
|
mechanism. The witness detects lost history afterward; it cannot prevent
|
|
an uncooperative Git write. No stronger same-user security boundary is
|
|
requested here.
|
|
|
|
The private index, exact snapshot blobs and expected-old-HEAD publication
|
|
are sound for the intended ordinary source-commit workflow. commit-tree
|
|
intentionally skips hooks; its own prospective-tree checks and compare-and-
|
|
swap remain necessary. I independently reproduced the active guard refusing
|
|
an ordinary commit after publication. I did not rerun Filbert's claimed
|
|
full three-order race experiment; the remaining schedules were reviewed
|
|
from the specification.
|
|
|
|
Jarvis is an explicit authorized lead identity, not a fallback token. The
|
|
read-in-place rule does not authorize copies, token inspection by this
|
|
review, or credential changes. GET user must map the lead to expected login
|
|
jarvis, while preserving the queue actor as the lead; “seat's expected
|
|
login” in step 2 must be read with that exception. Include a fake-transport
|
|
lead success case and wrong-login refusal to prevent accidentally comparing
|
|
jarvis to sage. Stat ownership/mode and path checks are metadata checks;
|
|
the helper alone reads the token. A 403 is a recorded failed attempt under
|
|
the accepted endpoint assumption, never an automatic retry or scope upgrade.
|
|
The permission-scope question remains with Sage, as ruled.
|
|
|
|
No new defect was found in the uncertain-outcome protocol: durable intent
|
|
precedes network work, all three request types have bounded process groups,
|
|
and unresolved attempts are not resent. Abandonment explicitly withdraws
|
|
the at-most-one claim. Fake transport is appropriate for this build; it is
|
|
not evidence that a live token has comment permission.
|
|
|
|
Only this report was written in the repository. Scratch Git and Pi fixtures
|
|
were isolated and removed. No source/index edits, commit, live process
|
|
change, secret read or external request. The source fixes above are plan
|
|
clarifications for the builder; implementation acceptance remains necessary.
|