feat(queue): Piece D, reviews as issue comments, raw per-seat token helper (row 12, #1508)
queue move ID in-review posts the review request as a Gitea comment and review record reads verdicts back, so reviews stop being files in docs/plans/reviews/. On a comment round, in-review to waiting-on-jason now needs every listed reviewer's approval for the current round, the same as in-review to done (Filbert r1 C1). scripts/gitea-api.sh reads the raw per-seat token files (lead decisions 37 to 39): config built and checked before curl starts, export attribute cleared, fixed base URL. test-queue.sh skips its live checks outside the canonical root. Darkwing authored. Filbert approved D r2 (cf1d3fd0) after r1 (a2dc2302) and corrected the plan (293747cd). Rocko reviewed the helper (e896192f, 2096b0a3), and Sage's lead check passed under decision 38. Manifest b402fb38, 19 files. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,247 @@
|
||||
# Queue D (#1508, row 12), candidate for review, round 2
|
||||
|
||||
Darkwing, 2026-09-27. Piece D is review requests, per plan section 8.9
|
||||
(`agents/filbert/work/queue-as-data-plan-2026-09-26.md`). Moving a row
|
||||
with reviewers to in-review posts one request comment on its issue as the
|
||||
acting seat, and the log records what happened. The row closes on the
|
||||
reviewers' recorded verdicts. The candidate also carries the DEFERRED
|
||||
item Sage attached to rows 12 and 13: `test-queue.sh` skips its live checks
|
||||
outside the canonical root.
|
||||
|
||||
Base is 8ffbd73b. The patch applies unchanged to HEAD cdcedb27, which
|
||||
since then has changed only `lead-decisions.md`, `QUEUE.md` and
|
||||
`queue.json`. Filbert reviews D. The helper patch is separate
|
||||
(`helper.md`); Sage approved it in lead decision 39, and it ships in the
|
||||
D commit. Sage commits. Nothing is committed, staged or pushed.
|
||||
|
||||
Round 2 fixes Filbert's C1
|
||||
(`agents/filbert/work/queue-d-review-r1-2026-09-27.md`): on a request
|
||||
round, in-review→waiting-on-jason now makes the same checks as
|
||||
in-review→done. See "Waiting-on-jason on a request round" below.
|
||||
|
||||
## Files
|
||||
|
||||
`build.patch` (sha256
|
||||
`28d0790e797e1f24f295f70b2e0162bb1b4b0a154b220c0527f7d6624f0aa1c8`) changes 7 files and adds 3.
|
||||
`build-manifest.sha256` (sha256
|
||||
`af57ead2697a88d74a9214783dfa35ba3bd171aa32a594584f018181e261e956`) pins all 10 after the
|
||||
patch. In a fresh clone at cdcedb27 the patch applies and the result
|
||||
matches the manifest 10/10.
|
||||
|
||||
- `packages/queue/src/review.mjs` (new): the credential check, the request
|
||||
body, the helper call under the deadline, and the reading of each
|
||||
answer.
|
||||
- `packages/queue/src/queue.mjs`: `SEMANTICS` 2, the five review verbs,
|
||||
the round, attempt and receipt shapes, and the rules for moves and done.
|
||||
- `packages/queue/src/store.mjs`: the request step after the move is
|
||||
logged, the resolve check, `review verify-commit`, and the retry and
|
||||
settle hints.
|
||||
- `packages/queue/src/cli.mjs`: the `review` subcommands.
|
||||
- `packages/queue/README.md`: a "Review requests" section, verbs, exit 3,
|
||||
known limits, tests.
|
||||
- `scripts/test-queue.sh`: the live checks skip outside the canonical root.
|
||||
- Tests: `review.test.mjs` (new, 20 tests), `fixtures/fake-gitea.mjs`
|
||||
(new), `fixtures/kill-at.mjs` (a step can be `NAME#N`, the Nth time it is
|
||||
reached), `store.test.mjs` (two A2 tests set row 9's reviewers to none
|
||||
first, so their moves post nothing; a note there moves from filbert to
|
||||
sage, since filbert is no longer a reviewer). 122 tests at A2, 142 now.
|
||||
|
||||
Outside the patch:
|
||||
|
||||
- `helper.patch` (sha256 `48edd46b93c54908e9d59737aab78a33e007ddfafa1e6ad4d34333015bb75430`): raw per-seat token files in
|
||||
`scripts/gitea-api.sh`, lead decision 37. Rocko reviewed two rounds,
|
||||
and Sage approved the result (decisions 38 and 39); notes in
|
||||
`helper.md`. D's tests don't need it, but the live round does.
|
||||
- `tools-md.patch` (sha256 `d30d65b842bd56077ccd9164eccd946f2aa9f53290d02eba61f284960d230e14`): `docs/TOOLS.md`, the review
|
||||
commands and the raw token file. It is Sage's file, so it's a proposal.
|
||||
|
||||
## How a request goes
|
||||
|
||||
1. **Intent.** `move ID in-review --candidate C --op OP` on a row with
|
||||
reviewers logs the round and its first attempt, `requesting`, under the
|
||||
lock, like any other op. If that write doesn't finish, nothing is sent.
|
||||
2. **Pre-send.** Without the lock: the credential file check (`lstat`
|
||||
only), then `GET user`, which must return the acting seat's login, or
|
||||
`jarvis` for sage. A failure here is `failed`, and the POST never runs.
|
||||
3. **POST** one comment on the round's issue through
|
||||
`scripts/gitea-api.sh`, under `timeout -s KILL 30`. The body carries
|
||||
two markers: `<!-- mosaic-queue-op: OP -->` and
|
||||
`<!-- mosaic-queue-round: row=N round=R candidate=DIGEST -->`.
|
||||
4. **Outcome.** A second entry, `OP.outcome`, under the lock. It holds the
|
||||
HTTP status, the comment id and a fixed detail string. Nothing from the
|
||||
response body is logged or printed.
|
||||
|
||||
Exit 0 is posted, 1 is failed (400, 401, 403, 404, 422, or pre-send), and
|
||||
3 is uncertain (anything else). A same-op retry prints the logged state and
|
||||
sends nothing. An uncertain or `requesting` attempt prints where to look
|
||||
and the two commands that settle it.
|
||||
|
||||
## Choices I made
|
||||
|
||||
- **The CLI shape of `resolve`.** The plan has
|
||||
`review resolve ID --attempt REQOP --posted <id>`. I made it
|
||||
`review resolve ID REQOP --comment N`, and abandon takes the attempt the
|
||||
same way. The attempt is always required, and `--comment` is the flag
|
||||
name `record` uses for a comment id too.
|
||||
- **Resolve checks the author.** The plan lists issue, marker, round and
|
||||
candidate. I added the comment's author: it must be the requester's
|
||||
login, fetched with the resolver's own token. Otherwise anyone could
|
||||
copy the markers into a comment of their own.
|
||||
- **Done on a request round** needs an approval recorded by every listed
|
||||
reviewer in the current round, and refuses `--evidence`. If the reviewers
|
||||
were removed after the round opened, done refuses until a privileged
|
||||
actor sets them. A round from before D, or on a row with no reviewers,
|
||||
closes with `--evidence` as in A2.
|
||||
- **Waiting-on-jason on a request round** (round 2, C1). The plan's table
|
||||
let the owner move in-review→waiting-on-jason with no other check, so a
|
||||
Jason-gated row could reach Jason, and then close, with no reviewer's
|
||||
verdict. The move now refuses while a request is unresolved, and on a
|
||||
request round it needs an approval recorded by every listed reviewer.
|
||||
`requireApprovals` in `queue.mjs` holds the approval checks that done
|
||||
and this move share. A round with no request is unchanged, so v1 replay
|
||||
is too. Filbert approved round 2 and asked for one more check (n1): an
|
||||
approval from an earlier round doesn't count in the current one. The
|
||||
Jason-gated test now has rocko approve round 2 first, and the move
|
||||
refuses until filbert approves round 2 as well. Jason gets no exemption, because the owner makes this move.
|
||||
In-review→in-progress, the changes path, is unchanged.
|
||||
- **The owner records no verdict,** even when listed as a reviewer.
|
||||
- **A late outcome on a done row.** `review-outcome` is the one entry a
|
||||
done row accepts, so a POST that answers after the row closed is still
|
||||
recorded. It can only set `conflict`; nothing reopens the row, and
|
||||
`review resolve` on it refuses without fetching the comment.
|
||||
- **A late `uncertain` after a resolve** keeps `posted`: the resolve saw
|
||||
the comment. A late `failed` after a resolve is `conflict`, because the
|
||||
server said it refused a comment someone found.
|
||||
- **Semantics per entry.** Every log entry already records `semantics`.
|
||||
Entries at 1 replay under A2's rules, so the live log (rev 13, all
|
||||
semantics 1) loads unchanged. Review verbs need 2, checked in the entry
|
||||
shape and again in apply.
|
||||
- **Body limit.** A body over 60,000 bytes is a pre-send `failed`, and
|
||||
nothing is sent. A 700-line manifest is enough to reach it.
|
||||
- **`verify-commit` on a commit candidate** compares every path the
|
||||
candidate changed against its first parent, by blob and mode, and
|
||||
requires the paths it deleted to be absent in REF. A `diff-tree` line it
|
||||
can't parse refuses with exit 1 instead of being skipped.
|
||||
- **The claim refusal message.** `ownerOrPriv` read "darkwing cannot
|
||||
resolve a request on". It now names the owner and the privileged actors
|
||||
in every case, with the claim first when there is one. No A2 test
|
||||
matched the old text; the resolve test pins the new one.
|
||||
- **`test-queue.sh`.** After genesis it reads `canonicalRoot` from
|
||||
`HEAD:docs/plans/queue.json` and compares it with the real path of the
|
||||
toplevel. If they differ, it prints
|
||||
`skip queue verify and render --check: this checkout (TOP) is not the queue's canonical root (CANON)`
|
||||
and passes. An empty `canonicalRoot` is a failure.
|
||||
|
||||
## Tests
|
||||
|
||||
`review.test.mjs` runs each case in a scratch repository with genesis
|
||||
committed. `scripts/gitea-api.sh` there is a stub that runs
|
||||
`fixtures/fake-gitea.mjs`: same argv and output as the helper, calls
|
||||
logged to a file, rules from a scenario file, and posted comments served
|
||||
back by id. The token files are dummies under the scratch directory. No
|
||||
test reads a real token or opens `~/.mosaic` or `~/.t3`.
|
||||
|
||||
Against the test list in 8.9:
|
||||
|
||||
| 8.9 asks for | Test |
|
||||
|---|---|
|
||||
| a kill before the POST, after it, at the outcome write, while retaking the lock | "a same-op retry after a kill sends nothing, even with a stale view" (steps `pre-send`, `posted`, `outcome`, `locked#2`), and "a held lock at the outcome exits 3" |
|
||||
| posted; each listed 4xx; 5xx; request failed; timeout; 201 without an id | "each transport answer maps to posted, failed or uncertain" (deadline 300 ms) |
|
||||
| a new op while `requesting` or `uncertain` refuses | "an unresolved request blocks a new request, a new round, waiting-on-jason and done", which also reaches waiting-on-jason with a late POST still out, and Jason's close refuses |
|
||||
| (round 2, C1) a Jason-gated row skips its reviewers | "a Jason-gated row reaches waiting-on-jason only on every reviewer's approval": Filbert's steps refuse, then pass after both reviewers approve a later round; reviewers removed refuse too |
|
||||
| a same-op retry after a stale view sends nothing | the kill test |
|
||||
| a late POST after abandon; a late outcome after resolve, same id and another | "late outcomes": rounds 1 to 3, plus a late 500 (stays posted) and a late 422 (conflict); "a late POST on a closed row" (abandoned, approved and closed while the POST was out) |
|
||||
| resolve with the wrong issue, marker, round or candidate | "resolve checks the comment", which adds author, id, 404, 500, a transport failure and the wrong credential |
|
||||
| `GET user` mismatch and timeout | "the pre-send checks" |
|
||||
| the lead posts as jarvis; `sage` as a login refuses | "the pre-send checks" and "the lead's request refuses a token for login sage" |
|
||||
| the credential checks: mode, another seat's path, the default file | "the credential file" (also unset, relative, missing, symlink, a linked directory, and jarvis's path for sage) |
|
||||
| request, changes, new candidate, approval, pinned | "request, changes, a new candidate, approval", which also checks no file lands under `docs/plans/reviews/` or `agents/*/work/` |
|
||||
|
||||
Every test that retries, kills or answers late counts the POSTs the fake
|
||||
saw. None sees a second POST for one attempt.
|
||||
|
||||
## Mutations
|
||||
|
||||
61 hand-written mutants: 29 in `queue.mjs`, 16 in `store.mjs`, 16 in
|
||||
`review.mjs`. Each runs alone against the whole queue test directory.
|
||||
Against the round 2 tests, 59 are killed, the same result as round 1.
|
||||
|
||||
Round 2 adds six in `queue.mjs` for C1, run against the round 2 tests.
|
||||
All six are killed:
|
||||
|
||||
- the round 1 branch restored (waiting-on-jason checks only the actor);
|
||||
- the approval check dropped from waiting-on-jason;
|
||||
- the unresolved check dropped from waiting-on-jason;
|
||||
- the unresolved check dropped from waiting-on-jason→done;
|
||||
- the no-reviewers check dropped from `requireApprovals`;
|
||||
- the approval check applied to every round, not only request rounds.
|
||||
|
||||
Seven survived earlier runs. Five of them now have a test that kills them:
|
||||
a second outcome for one attempt, the owner recording a verdict, done on a
|
||||
row whose reviewers were removed, a same-op resolve retry that fetched the
|
||||
comment again, and a resolve on a done row that fetched the comment before
|
||||
the log refused it. The last one is the new test "a late POST on a closed
|
||||
row".
|
||||
|
||||
Two survive, and each is equivalent while the other stays:
|
||||
|
||||
- the semantics check in `applyEntry` dropped;
|
||||
- the semantics check in the entry shape dropped.
|
||||
|
||||
Every entry passes the shape check before it is applied, so either check
|
||||
alone refuses a review verb at semantics 1. With both dropped, the
|
||||
semantics test fails.
|
||||
|
||||
## Suites
|
||||
|
||||
In the scratch clone with the patch and the helper patch applied: config
|
||||
24/0, task 90/0, foundation 44/0, conductor 17/0, release 14/0, auth 15/0,
|
||||
discord 64/0, extension-package 18/0, queue 27/0. `node --test
|
||||
packages/queue/tests/` passes 142, and `node --test packages/ledger/tests/`
|
||||
passes 58.
|
||||
|
||||
The scratch clone isn't the canonical root, so the queue suite printed its
|
||||
skip line for the two live checks. In the canonical tree they run.
|
||||
|
||||
In a fresh clone at cdcedb27 with `build.patch` applied: the manifest
|
||||
matches 10/10, the queue tests pass 142, the queue suite 27/0. With
|
||||
`helper.patch` applied on top, the ledger tests pass 58.
|
||||
|
||||
## Known limits
|
||||
|
||||
- **Scope of darkwing's and dewey's tokens.** They hold
|
||||
`write:repository`. If that doesn't cover an issue comment, the live
|
||||
POST gets 403, which is `failed` with nothing posted (8.9 says so). The
|
||||
scope question then goes to Sage.
|
||||
- **Post to outcome.** A kill between the POST and the outcome entry
|
||||
leaves the attempt `requesting` with a comment on the issue. The queue
|
||||
never posts again on its own; a person resolves it.
|
||||
- **Verdict comments aren't fetched.** `review record` takes the
|
||||
reviewer's comment id on trust, as the queue takes `--by`.
|
||||
- **Commit candidates** stay checkable only while a ref keeps the commit.
|
||||
- The helper's own limits are in `helper.md`.
|
||||
|
||||
## After approval (Sage)
|
||||
|
||||
1. Apply `build.patch` to the canonical tree and check it against
|
||||
`build-manifest.sha256`. Run `scripts/test-queue.sh` and
|
||||
`node --test packages/queue/tests/`. Commit the 10 files by path.
|
||||
2. Apply `helper.patch` for the same commit (decision 39). The live
|
||||
round needs it: without it the helper refuses a raw token file.
|
||||
3. `tools-md.patch` if you want it.
|
||||
4. Row 12 lists no reviewers, so moving it to in-review now would open a
|
||||
round that posts nothing. Set them first:
|
||||
`scripts/mosaic queue set 12 reviewers filbert --op OP --by sage`, and
|
||||
add rocko if the helper counts as part of this round.
|
||||
|
||||
The live round, which is mine:
|
||||
|
||||
5. `MOSAIC_GITEA_CREDENTIAL_FILE=~/.mosaic/fleet/agents/darkwing/secrets/gitea-mosaicstack-darkwing.token
|
||||
scripts/mosaic queue move 12 in-review --candidate <D's commit> --op OP --by darkwing`.
|
||||
The queue `lstat`s the file, the helper reads it, and `GET user` must
|
||||
answer `darkwing`. The request goes on #1508.
|
||||
6. Exit 0 means posted. Exit 1 with HTTP 403 is the scope question above.
|
||||
Exit 3 means I look on #1508 for the marker and resolve, or ask you to
|
||||
abandon.
|
||||
7. Filbert posts a verdict on #1508 and runs `review record 12`. Then
|
||||
`move 12 done`, and `queue-commit.sh` for the queue ops.
|
||||
@@ -0,0 +1,184 @@
|
||||
# Raw per-seat token files in `scripts/gitea-api.sh` (row 12, #1508)
|
||||
|
||||
Darkwing, 2026-09-27, after round 2 (lead decision 38). Lead decision 37 approved this patch. It ships next
|
||||
to the Piece D candidate but is a separate patch, and Rocko reviews it
|
||||
because it handles a credential. Filbert reviews D. Sage commits both.
|
||||
Nothing is committed, staged or pushed.
|
||||
|
||||
`helper.patch` (sha256 `48edd46b93c54908e9d59737aab78a33e007ddfafa1e6ad4d34333015bb75430`)
|
||||
changes `scripts/gitea-api.sh` and adds
|
||||
`packages/ledger/tests/gitea-helper-raw.test.mjs`. It applies to HEAD
|
||||
8efc0ff3 by itself. With only this patch applied, `node --test
|
||||
packages/ledger/tests/` passes 58/58. D does not depend on it, and it does
|
||||
not depend on D.
|
||||
|
||||
## Why
|
||||
|
||||
Each seat's Gitea token is at
|
||||
`~/.mosaic/fleet/agents/<seat>/secrets/gitea-mosaicstack-<seat>.token`.
|
||||
These files hold the bare token, not the JSON `mosaic.gitea.json` shape
|
||||
that the helper reads today. Without this change the helper refuses them,
|
||||
so D's live round can't post as the seat.
|
||||
|
||||
## What changed
|
||||
|
||||
Both old node snippets, the one that printed the base URL and the one that
|
||||
wrote the curl config, are now one script, `CRED_JS`, run in two modes:
|
||||
`base` and `cfg`. Each mode runs every check before it prints anything.
|
||||
|
||||
1. `lstat`: a regular file, not a symlink, no group or other bits.
|
||||
Unchanged.
|
||||
2. Read the file. A read error exits 3. It used to be caught by the JSON
|
||||
parse's catch, with the same result.
|
||||
3. `JSON.parse`. If the text parses, the JSON path runs unchanged: `url`
|
||||
must be `https://git.mosaicstack.dev` (trailing slashes stripped), and
|
||||
`api_token` must be a nonempty string. A non-`SyntaxError`, such as
|
||||
`null.mosaicstack`, exits 3.
|
||||
4. The raw path runs only on a `SyntaxError`. It accepts the file only if
|
||||
all of these hold:
|
||||
- `stat` size is 40 or 41;
|
||||
- the byte length of the text equals the `stat` size;
|
||||
- the text matches `^[0-9a-f]{40}\n?$`.
|
||||
|
||||
Then the base URL is the fixed string `https://git.mosaicstack.dev`,
|
||||
and the token is the first 40 characters. Anything else exits 3
|
||||
before any request.
|
||||
5. The second read runs, and must succeed, before curl starts. Round 1
|
||||
ran it inside `curl -K <(…)`, where bash drops its exit status, so a
|
||||
refusal gave curl an empty config and the request still went out.
|
||||
Now `CFG="$(node -e "$CRED_JS" cfg)" || exit 3` and `[ -n "$CFG" ] ||
|
||||
exit 3` run first, and curl reads `-K <(printf '%s\n' "$CFG")`.
|
||||
`printf` is a builtin, so the token is in no argv. Both curl branches,
|
||||
with and without a body, use it.
|
||||
6. `export -n CFG` runs right after the checked assignment, before any
|
||||
child starts. A `CFG` inherited from the caller's environment keeps its
|
||||
export attribute when assigned, and `SHELLOPTS=allexport` in the
|
||||
environment exports every assignment. Either way the config would reach
|
||||
curl's environment, and the body file's `mktemp` and `chmod` too.
|
||||
|
||||
## Decision 37, condition by condition
|
||||
|
||||
1. **The JSON path is unchanged.** The last test runs a good JSON file,
|
||||
four refused ones, and content that parses as JSON but would pass the
|
||||
raw pattern: 40 decimal digits, with and without a newline. Those take
|
||||
the JSON path and refuse.
|
||||
2. **One line of token characters.** The format is 40 lowercase hex
|
||||
characters. I checked by `stat` that the real files are 40 bytes (jarvis)
|
||||
or 41 (the others), all mode 600. I then ran the patched `CRED_JS` in
|
||||
`base` mode against my own file only. It printed
|
||||
`https://git.mosaicstack.dev` and exited 0, so darkwing's file matches
|
||||
the pattern. Nothing printed the token, and I read no other seat's file.
|
||||
The other seats' files are unconfirmed beyond size and mode. If one
|
||||
doesn't match, the helper exits 3 before any request. The test refuses
|
||||
16 bad contents before curl runs: empty, 39 characters, 41 characters,
|
||||
upper case, CRLF, a trailing CR, a trailing space, a trailing tab, two
|
||||
newlines, a leading space, a trailing space at 40, a second line, a
|
||||
quote, non-hex, non-ASCII, and 80 characters.
|
||||
3. **Fixed base URL.** The raw path assigns the literal. The test sets
|
||||
`MOSAIC_GITEA_URL`, `GITEA_URL` and `MOSAIC_GITEA_BASE_URL` to another
|
||||
host, and curl still gets `https://git.mosaicstack.dev/api/v1/user`.
|
||||
4. **File checks and the config stream.** Modes 640, 604, 660, 644, 000
|
||||
and 200, a symlink, a missing file and a directory all refuse before
|
||||
curl runs. 000 and 200 pass the group and other check, and the read
|
||||
refuses them. The stub curl records its argv and its `-K` stream. The
|
||||
token is in the stream only, never in argv, stdout or stderr. Every
|
||||
token in the test is a dummy that the test writes.
|
||||
5. **Only the acting seat's file.** That is D's job, not the helper's. D
|
||||
refuses unless `MOSAIC_GITEA_CREDENTIAL_FILE` resolves to
|
||||
`…/agents/<login>/secrets/gitea-mosaicstack-<login>.token` for the
|
||||
acting seat, or jarvis when Sage acts, and before posting it checks that
|
||||
`GET user` returns that login. See `review.mjs` `credCheck` and
|
||||
`checkUser` in the D candidate.
|
||||
6. This review.
|
||||
|
||||
## Round 1 and what changed for round 2
|
||||
|
||||
Rocko's round 1 (`agents/rocko/work/gitea-helper-raw-r1-review-2026-09-27.md`)
|
||||
found one blocker: a second read that refused still let curl run with an
|
||||
empty config, and a 2xx then exited 0. My round 1 notes called that a 401,
|
||||
which the server doesn't guarantee. Sage agreed it belongs in this patch.
|
||||
Step 5 above is the fix.
|
||||
|
||||
The new test, "a file that changes between the two reads refuses before
|
||||
curl runs", uses the git stub, which runs between the two reads, to
|
||||
change a valid dummy file to invalid text, to `{}`, to mode 644, to a
|
||||
symlink, and to missing. It runs each change for `GET user` and for a POST
|
||||
with a dummy body. Every case exits 3 with no curl call and nothing on
|
||||
stdout. It also changes a valid JSON file to invalid text. As a control,
|
||||
both calls reach curl with the right config when nothing changes. The
|
||||
first test now also checks that the token is not in the environment curl
|
||||
gets.
|
||||
|
||||
Rocko's round 2 (`agents/rocko/work/gitea-helper-raw-r2-review-2026-09-27.md`)
|
||||
closed that blocker and found another: the inherited export attribute in
|
||||
step 6. Lead decision 38 took the fix, `export -n CFG`, with no round 3;
|
||||
Sage checks it with Rocko's reproducer. The test "the token reaches no
|
||||
child environment, even with an inherited CFG or SHELLOPTS=allexport" runs
|
||||
raw and JSON dummy files, GET and POST, with an exported harmless `CFG`,
|
||||
with `SHELLOPTS=allexport`, and with both. Each call must exit 0 with the
|
||||
token in curl's config stream and not in its environment, argv, stdout or
|
||||
stderr. I found the `SHELLOPTS` route while checking the fix; `export -n`
|
||||
covers it too, so I added no second line for it.
|
||||
|
||||
Not in this patch: the predictable response path
|
||||
`/tmp/gitea-api-response.$$`. Sage has it on DEFERRED as "Gitea helper
|
||||
response-file hardening".
|
||||
|
||||
## What I'd like you to look at
|
||||
|
||||
- **The second read now re-validates.** Before, `gen_curl_cfg` parsed the
|
||||
file again without the `lstat` checks. Now both reads run every check.
|
||||
So a file swapped for a symlink or a wider mode between the two reads
|
||||
refuses. On the JSON path this is a tightening.
|
||||
- **The token now sits in a shell variable** for the rest of the script.
|
||||
`export -n` keeps it out of every child's environment, and nothing
|
||||
prints it. Before, it existed only in the pipe.
|
||||
- **`lstat` then `readFile` is still not atomic.** A swap between those two
|
||||
calls inside one read is the pre-existing race Rocko noted. This patch
|
||||
doesn't claim to close it.
|
||||
- **`text.slice(0, 40)`** relies on the pattern having matched. The only
|
||||
accepted texts are the token alone or the token plus `\n`.
|
||||
|
||||
## Mutations
|
||||
|
||||
25 mutants, each run alone against `gitea-helper-raw.test.mjs`: 19 of
|
||||
`CRED_JS` and 6 of the gate and the export. 15 are killed:
|
||||
|
||||
- uppercase allowed, any trailing whitespace allowed, the `^` anchor
|
||||
dropped;
|
||||
- a raw file refused outright (the old behaviour);
|
||||
- an env override of the raw base URL;
|
||||
- the token taken with its newline;
|
||||
- the mode check dropped;
|
||||
- the JSON path's host check or token check dropped, or its `|| {}`;
|
||||
- `cfg` output in `base` mode;
|
||||
- the read's own `try` dropped (killed by the 000 and 200 modes; I added
|
||||
those after this mutant first survived);
|
||||
- both gate checks dropped, and the round 1 code (no gate, curl reading
|
||||
`<(node … cfg)`), killed by the between-reads test;
|
||||
- `export -n CFG` dropped, killed by the inherited-environment test.
|
||||
|
||||
Ten survive, and each is equivalent while the other checks stay:
|
||||
|
||||
- `\n*$` for `\n?$`: the size check caps the file at 41 bytes;
|
||||
- the size check dropped: the pattern caps the length;
|
||||
- the byte-length check dropped: the pattern is ASCII, so a match means
|
||||
bytes equal characters; the check differs only if the file changes
|
||||
between `lstat` and the read;
|
||||
- the `SyntaxError` test dropped: the only non-`SyntaxError` is JSON
|
||||
`null`, which then fails the raw pattern;
|
||||
- `text.trim()` for `text.slice(0, 40)`: same result on every accepted
|
||||
text;
|
||||
- the `isSymbolicLink` test dropped: `lstat` of a symlink is never
|
||||
`isFile`;
|
||||
- `!== "base"` for `=== "cfg"`: the script calls only these two modes;
|
||||
- `|| true` for `|| exit 3` on the `CFG` line: node prints nothing when it
|
||||
refuses, so `[ -n "$CFG" ]` refuses;
|
||||
- `[ -n "$CFG" ]` dropped: `|| exit 3` refuses first. Dropping
|
||||
`|| exit 3` outright is the same, since `set -e` exits on the failed
|
||||
assignment;
|
||||
- `export CFG=…` for `CFG=…`: `export -n` on the next line clears it. In
|
||||
round 2 this one was killed; the fix makes it equivalent.
|
||||
|
||||
I kept the size, byte-length and symlink checks anyway. Decision 37 asks
|
||||
for size plus pattern, and the other two cost nothing.
|
||||
@@ -1467,7 +1467,7 @@ cooperative trust model (J2).
|
||||
| in-progress→briefed (`release`) | claimant, or privileged | clears `claim` |
|
||||
| in-progress→in-review | claimant | once D is built, for a row with `reviewers`, this move is the review request, and it takes `--candidate` (8.9). Before D, or with no reviewers, it opens the round with `request: none` |
|
||||
| in-review→in-progress | owner, or privileged | changes requested (Sage) |
|
||||
| in-review→waiting-on-jason | owner, or privileged | — |
|
||||
| in-review→waiting-on-jason | owner, or privileged | with D built, on a comment round: refused while any attempt is unresolved, and it needs the approving receipts of every listed reviewer for that round, the same checks as in-review→done (8.9). Before D, or on a round with `request: none`: no condition |
|
||||
| waiting-on-jason→done | `jason`; or `sage` with `--evidence` citing Jason's approval | the evidence reference is logged |
|
||||
| in-review→done | the row's `gateOwner`, or privileged | refused when `gateOwner` is `jason` (that path runs through waiting-on-jason). `--evidence` must name the current round and its candidate digest: with D built, the approving receipts of every listed reviewer for that round (8.9); before D, a comment id together with the round's candidate digest, which must match. Logged (J5) |
|
||||
| any non-terminal→blocked | owner, or privileged | reason required; records `previousState`; blocked→blocked refused (update the reason with `note`) |
|
||||
@@ -1688,6 +1688,10 @@ The CLI may print the issue URL and the marker as a hint. It never decides.
|
||||
**Receipts.** `review record` is accepted from a listed reviewer, for the
|
||||
current round, citing the comment id and the candidate digest that was
|
||||
approved. A mismatch refuses. Every round is kept in `review.rounds[]`.
|
||||
On a comment round, the receipts gate both moves out of in-review toward
|
||||
done: in-review→done, and in-review→waiting-on-jason, which is the route
|
||||
for a Jason-gated row. Each needs an approval from every listed reviewer
|
||||
and no unresolved attempt.
|
||||
|
||||
**Tests** use a fake transport:
|
||||
- a kill:
|
||||
@@ -2402,3 +2406,8 @@ The round-2 modifications:
|
||||
- the guard checked as active on every invocation, including a
|
||||
`git hook run` canary (G2), checked in a scratch repository;
|
||||
- the lead's expected login is `jarvis`. 8.19 maps the findings.
|
||||
- 2026-09-27: correction from Filbert's Piece D review round 1 (C1),
|
||||
ruled by Sage. The transition table let in-review→waiting-on-jason pass
|
||||
with no reviewer approvals, so a Jason-gated row could close without its
|
||||
reviewers. That move now carries the in-review→done checks on a comment
|
||||
round; the table and 8.9's receipts paragraph say so.
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
# Queue Piece D review, round 1 (#1508, row 12)
|
||||
|
||||
Filbert, 2026-09-27. Plan: `queue-as-data-plan-2026-09-26.md` §8.9.
|
||||
A2 round 1 review: `queue-a2-review-r1-2026-09-27.md` (sha256 f167b85e).
|
||||
|
||||
## Verdict
|
||||
|
||||
**Changes requested, one item (C1).** The review covers `build.patch`
|
||||
sha256 cb7a6c6649f88496887eacab52db77ef1cc62aaf59a48c3cadcd5f8e33ae1243
|
||||
on base 8ffbd73b (it applies cleanly at 8efc0ff3), with
|
||||
`build-manifest.sha256` 5646d938…. Everything else in the candidate is
|
||||
ready. C1 is a gap in my own plan's transition table, and D implements that
|
||||
table as written. Sage can rule it a plan question and approve without it,
|
||||
but I recommend fixing it now: no semantics-2 entry is logged yet, so the
|
||||
fix needs no semantics 3.
|
||||
|
||||
I re-hashed the inputs after the review. `build.patch` and
|
||||
`build-manifest.sha256` match the hashes Darkwing sent. `build.md` changed
|
||||
from b079fbc1… to ced0946b… while I reviewed. The diff is two lines: the
|
||||
`helper.patch` hash now names helper round 2, and a helper test count went
|
||||
from 56 to 57. Both are in the helper, which is Rocko's scope, so the
|
||||
change doesn't touch this review.
|
||||
|
||||
## C1. A Jason-gated row can close without its reviewers' verdicts
|
||||
|
||||
**Where:** `packages/queue/src/queue.mjs`, `applyMove`, the branch
|
||||
`} else if (from === "in-review" && (to === "in-progress" || to === "waiting-on-jason")) {`
|
||||
(lines 766–767 in the patched file). It calls only `ownerOrPriv`.
|
||||
|
||||
**What happens.** On a semantics-2 comment round, in-review→waiting-on-jason
|
||||
checks neither the reviewers' approvals nor the unresolved attempts. The
|
||||
in-review→done path checks both, but a Jason-gated row never takes that
|
||||
path; it goes through waiting-on-jason. After that, waiting-on-jason→done
|
||||
checks only for unresolved attempts. So on a Jason-gated row with
|
||||
reviewers, the listed reviewers are advisory.
|
||||
|
||||
**Reproduced** with a temporary probe test on the `review.test.mjs`
|
||||
harness (since deleted):
|
||||
|
||||
1. `set 9 gate-owner jason` on a row with reviewers.
|
||||
2. The owner's move to in-review posts the request (comment 1000). The
|
||||
round's `receipts` is `[]`.
|
||||
3. darkwing: `move 9 waiting-on-jason` exits 0.
|
||||
4. jason: `move 9 done` exits 0.
|
||||
|
||||
No reviewer recorded a verdict at any point.
|
||||
|
||||
**Why it matters now.** Row 5 (CHAT-03 source) is next through this path.
|
||||
Its gate owner is jason and its reviewers are filbert and rocko. D exists so
|
||||
that the queue, not a seat's memory, holds "reviewed before Jason sees it".
|
||||
|
||||
**Cause.** My plan's table, line 1470, gives
|
||||
in-review→waiting-on-jason as "owner, or privileged | —". I wrote the
|
||||
approval condition only on in-review→done (line 1472). D follows the table.
|
||||
|
||||
**Fix.** When the current round is a comment round, in-review→waiting-on-jason
|
||||
runs `refuseUnresolved` and requires an approving receipt from every listed
|
||||
reviewer, the same checks as in-review→done (lines 779–787). Share them in
|
||||
one helper. In-review→in-progress stays as it is: that's the changes path,
|
||||
and the next round already refuses unresolved attempts.
|
||||
|
||||
- Scope it to `cur?.request === "comment"`. Only semantics-2 entries create
|
||||
such rounds, so v1 replay is unaffected.
|
||||
- Whether `by === "jason"` is exempt is Sage's call. I lean against it:
|
||||
an exemption is one more path to test, and the owner can't use it.
|
||||
- Add a test: the steps above refuse at step 3, then pass once every
|
||||
reviewer has recorded an approval. Add a mutant that drops the check.
|
||||
- Update the plan table line 1470 and the README's review section to match.
|
||||
I'll make the plan edit if Sage wants it in my file.
|
||||
|
||||
## What I checked
|
||||
|
||||
All of this ran in a scratch clone, `/tmp/fqd`, with push disabled, on
|
||||
frozen 0444 copies of the inputs in `/tmp/fqd-in`.
|
||||
|
||||
- **Manifest and suites.** The manifest checks 10/10 after
|
||||
`git apply build.patch`. `node --test packages/queue/tests/` passes
|
||||
141/141, and the ledger tests pass 51/51. `scripts/test-queue.sh` reports
|
||||
27/0 with its "outside the canonical root" skip line, as Sage described.
|
||||
- **The source**, read in full: `review.mjs`, and the diffs to `queue.mjs`,
|
||||
`store.mjs`, `cli.mjs`, `scripts/test-queue.sh`, the README, the tests and
|
||||
`fixtures/fake-gitea.mjs`. `tools-md.patch` matches the code; it's Sage's
|
||||
to apply.
|
||||
- **The deadline.** `timeout -s KILL` kills the helper's grandchild. I
|
||||
checked it with a helper that forks a sleeper: the grandchild is gone
|
||||
after the deadline, and `spawnSync` reports `signal: "SIGKILL"`, which
|
||||
`classifyPost` maps to uncertain.
|
||||
- **Mutations.** I wrote 24 mutants of my own, separate from Darkwing's 61,
|
||||
and the suite kills all 24. They cover:
|
||||
- a same-op retry falling through to a second POST;
|
||||
- an abandoned attempt followed by a late post, which must be a conflict;
|
||||
- resolve accepting a comment with another id, another author, or no
|
||||
round marker;
|
||||
- `credCheck` skipping the mode check or the realpath check;
|
||||
- a 201 with no comment id treated as posted;
|
||||
- other HTTP codes treated as failed instead of uncertain;
|
||||
- a killed call treated as failed;
|
||||
- done without every approval;
|
||||
- a new round, or waiting-on-jason→done, ignoring unresolved attempts;
|
||||
- a new request while one is posted;
|
||||
- resolve without the GET;
|
||||
- the body limit removed;
|
||||
- sage posting as sage instead of jarvis;
|
||||
- `verify-commit` ignoring the mode or a deletion;
|
||||
- the outcome-write failure exiting 1 instead of 3;
|
||||
- a failed pre-send still posting;
|
||||
- `next` offering a round the reviewer already answered;
|
||||
- abandon leaving `duplicateRisk` false;
|
||||
- record accepting a different candidate digest.
|
||||
|
||||
## Non-blocking
|
||||
|
||||
- **n1. Done doesn't require a posted request.** If every attempt failed
|
||||
or was abandoned, reviewers can still record approvals and the row can
|
||||
close. That's acceptable: the receipts are the evidence, and the comment
|
||||
is how reviewers are asked, not what they approve.
|
||||
- **n2. A review issue that is a pull request.** `checkComment` needs
|
||||
`issue_url` to end in `/issues/N`. I haven't checked what Gitea returns
|
||||
for a comment on a pull request. If it's anything else, resolve refuses.
|
||||
That fails closed, and our review issues are issues, so no change now.
|
||||
- **n3. Abandoning a `requesting` attempt sets `duplicateRisk`** even when
|
||||
the kill came before the POST. That's per plan: the queue can't tell
|
||||
which side of the POST the kill landed on.
|
||||
- **n4. The body travels on argv as a JSON argument.** It's capped at
|
||||
60,000 bytes, well under the per-argument limit. An E2BIG would classify
|
||||
as uncertain, which is the safe side.
|
||||
- **n5. For Rocko:** `scripts/gitea-api.sh` writes the response to the
|
||||
predictable path `/tmp/gitea-api-response.$$`. That's the helper, not D.
|
||||
|
||||
What I'd keep as it is:
|
||||
- resolve checking the comment's author against the requester's login.
|
||||
That goes beyond the plan, and it's right;
|
||||
- semantics recorded per entry, with review verbs refused on v1 entries;
|
||||
- the late-outcome rules in `outcomeState`;
|
||||
- fixed detail strings, so nothing from a response reaches the log.
|
||||
|
||||
## For Darkwing and Sage
|
||||
|
||||
Fix C1 with its test and mutant, re-send the patch and manifest, and I'll
|
||||
review round 2 against this list only. If Sage rules C1 out of D's scope,
|
||||
the candidate is approved as it stands, and C1 has to land before row 5
|
||||
moves to in-review.
|
||||
@@ -0,0 +1,90 @@
|
||||
# Queue Piece D review, round 2 (#1508, row 12)
|
||||
|
||||
Filbert, 2026-09-27. Round 1: `queue-d-review-r1-2026-09-27.md`
|
||||
(sha256 a2dc2302…). This round checks C1 only, per Sage's ruling to fix it
|
||||
in D.
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approved.** The review covers `build.patch` sha256
|
||||
8a19f7fa03e2ee6c3bd9ce7b2e296acb21655471c4cc56ba19f25464c8129209 (base
|
||||
8ffbd73b, applied at cdcedb27), with `build-manifest.sha256` 647d0170…
|
||||
and `build.md` e77c8c5e…. The C1 fix is correct. One mutant of mine
|
||||
survives: an approval from an earlier round counted in the current one.
|
||||
The code handles that case correctly; the tests just don't check it. n1
|
||||
gives a four-line test that kills the mutant. I'd add it before the
|
||||
commit, and it doesn't need a round 3. I checked it myself (below).
|
||||
|
||||
## What I checked
|
||||
|
||||
All of this ran in a scratch clone, `/tmp/fqd2`, at cdcedb27 with push
|
||||
disabled, on frozen 0444 copies of the three inputs.
|
||||
|
||||
- **Manifest and suites.** The manifest checks 10/10. `node --test
|
||||
packages/queue/tests/` passes 142/142, and the ledger tests pass 51/51
|
||||
(Darkwing's 58 includes the helper patch, which I didn't apply).
|
||||
`scripts/test-queue.sh` reports 27/0 with the "outside the canonical
|
||||
root" skip line.
|
||||
- **What changed.** I compared all ten manifest files with round 1's tree.
|
||||
Only `queue.mjs`, `README.md` and `review.test.mjs` differ, as Darkwing
|
||||
said.
|
||||
- **The fix in `queue.mjs`.**
|
||||
- `requireApprovals` is the round-1 inline check moved into a helper.
|
||||
in-review→done's messages are unchanged.
|
||||
- in-review→in-progress still checks only `ownerOrPriv`, which is right
|
||||
for the changes path.
|
||||
- in-review→waiting-on-jason runs `ownerOrPriv`, then `refuseUnresolved`,
|
||||
then `requireApprovals` when the last round is a comment round.
|
||||
`unresolvedAttempts` skips rounds that aren't comment rounds, and v1
|
||||
entries can't open one. So v1 replay is unaffected.
|
||||
- There's no exemption for jason or sage, as agreed.
|
||||
- **The tests.** The new test runs my round-1 reproduction and now
|
||||
refuses at the step that used to pass. It covers a reviewer who asked
|
||||
for changes, a second round, and reviewers cleared after the round
|
||||
opened. The rewritten unresolved test keeps waiting-on-jason→done
|
||||
covered through a late post that turns an abandoned attempt into a
|
||||
conflict. That's a better route than the old one.
|
||||
- **The README** matches the code.
|
||||
- **Mutations.** I wrote eight mutants of my own for this round. The
|
||||
suite kills seven:
|
||||
- G1: sage bypasses the approval check on waiting-on-jason;
|
||||
- G2: a `changes` verdict counts as an approval;
|
||||
- G3: the check reads the first round, not the last;
|
||||
- G4: one approval is enough;
|
||||
- G5: the owner-or-privileged check dropped from waiting-on-jason;
|
||||
- G6: the unresolved check dropped from waiting-on-jason→done;
|
||||
- G7: the approval check dropped from in-review→done.
|
||||
|
||||
G8 survives: approvals are collected from every round's receipts
|
||||
instead of the current round's. See n1.
|
||||
- **My plan.** Table line 1470 and 8.9's Receipts paragraph (plan sha256
|
||||
293747cd…) describe the code as built.
|
||||
|
||||
## Non-blocking
|
||||
|
||||
- **n1. Add a test that an earlier round's approval doesn't count (kills
|
||||
G8).** In the new test, filbert approves round 1 and rocko asks for
|
||||
changes. In round 2 both approve before anything checks, so an old
|
||||
approval counting in round 2 goes unnoticed. In
|
||||
`packages/queue/tests/review.test.mjs`, replace the round-2 `for` loop
|
||||
(three lines) with:
|
||||
|
||||
```js
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "91", "--candidate", head, "--op", "record-9-rock02"], "rocko"));
|
||||
// Filbert's round 1 approval doesn't carry into round 2.
|
||||
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 2 has no approval recorded by filbert$/m);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "90", "--candidate", head, "--op", "record-9-filb02"], "filbert"));
|
||||
```
|
||||
|
||||
I checked it in a copy of the scratch clone: `review.test.mjs` passes
|
||||
20/20 on the candidate and fails 1 under G8.
|
||||
- **n2. An owner who is also a listed reviewer blocks the row.** `set
|
||||
reviewers` and `assign` don't stop the owner from being on the list.
|
||||
`record` refuses the owner, but `requireApprovals` still waits for the
|
||||
owner's approval. The row then can't reach done or waiting-on-jason
|
||||
until a privileged actor runs `set reviewers`. That fails closed, and it
|
||||
was already true of done in round 1. The refusal names the owner as
|
||||
missing, which could confuse. Don't fix it by dropping the owner from
|
||||
the required list: with reviewers `[owner]`, that list would be empty
|
||||
and the row would pass with no review. A later fix could refuse the
|
||||
owner at `set reviewers` and `assign`. That's a follow-up, not D.
|
||||
@@ -0,0 +1,39 @@
|
||||
# Gitea raw-token helper — R1 security review
|
||||
|
||||
Verdict: **changes requested**, one blocking finding. Round 1 of two.
|
||||
|
||||
Reviewed `helper.patch`, sha256 `c5e3de137ee999620688fab6b9721fe973a9d0ab1ca98ae6bcd8cc742d5620ee`, applied alone to an isolated shared clone at `8efc0ff3`. The patch hash was checked before reading and again after testing. Scope is lead decision 37's credential-handling helper; Filbert reviews Piece D. `scripts/mosaic queue next rocko` returned `nothing`; this explicit review request supplies the assignment.
|
||||
|
||||
## 1. Blocking — second validation failure does not prevent the request
|
||||
|
||||
Candidate `scripts/gitea-api.sh:86` and `:90` invoke curl with `<(gen_curl_cfg)`. Bash does not propagate the producer's exit status to curl. Revalidating in `cfg` mode is useful, but a refusal produces an empty config stream while curl still sends the request. Lines 99–100 can then report success on a 2xx response. A 401 is not guaranteed and is not a substitute for refusing before the request.
|
||||
|
||||
Independent deterministic reproduction used only dummy credentials and stub git/curl:
|
||||
|
||||
1. The first read accepts a valid raw dummy token.
|
||||
2. The git stub, which runs between the two reads, replaces that dummy file's contents with `invalid-token`.
|
||||
3. The second validator exits 3. The curl stub still runs, observes zero config bytes, and returns 200.
|
||||
4. The helper exits 0, prints `{}` and `HTTP 200`.
|
||||
|
||||
This control-flow hole predates the patch, as helper.md correctly discloses. It directly contradicts decision 37's acceptance condition for this new raw path: invalid content must refuse **before any request**. It also defeats the newly added second-read file checks at the request boundary. It needs fixing in this patch rather than deferral.
|
||||
|
||||
**Required change:** finish credential validation/config generation and check its success before starting curl. Keep the successful config in memory and deliver the token only through the config stream, never through argv, exported environment, disk or diagnostics. Preserve the JSON validation contract. Cover both body and no-body curl branches.
|
||||
|
||||
**Required regressions:** change a valid dummy file to invalid content, a symlink and a disallowed mode between the reads (or at the equivalent validation boundary after refactoring). Assert exit 3 and zero curl invocations, including a POST with a dummy body. Do not merely assert an eventual HTTP failure. Remove the success gate as a mutation and require these tests to fail.
|
||||
|
||||
## 2. Nonblocking — predictable response path is a separate existing issue
|
||||
|
||||
Lines 86/90/95/96 use `/tmp/gitea-api-response.$$`. A pre-existing symlink can redirect curl's output and the later `cat`; predictable shared-directory names are not a safe temporary-file allocation. This does not put the raw token in that file, and this patch does not introduce the path. I would keep it out of the narrow credential patch and track a separate fix using an exclusively created private temporary response file and cleanup on all exits. No exploit against a real path was attempted.
|
||||
|
||||
## What is fine
|
||||
|
||||
- The raw branch is reached only after JSON syntax failure. JSON values such as `null` and 40 decimal digits do not become raw credentials. The JSON URL/token validation rules remain unchanged.
|
||||
- The size, byte-count and ASCII pattern checks together accept exactly 40 lowercase hex characters with at most one final LF. Rejected contents emit no token. The fixed raw URL has no environment override.
|
||||
- Both reads perform the existing regular-file, no-symlink and group/other-mode checks. This is an improvement over the old second read, subject to finding 1. It is not an atomic file-descriptor identity guarantee across `lstat` and `readFile`; that separate pre-existing race is not claimed closed here.
|
||||
- On successful validation, the raw token reaches curl through its config descriptor and not through argv or normal stdout/stderr. Tests use dummy files and stub executables.
|
||||
- The helper is intentionally generic; it does not enforce the acting seat's identity. Decision 37's own-file rule remains an integration gate in Piece D (`credCheck` and `GET user` identity verification). This helper review does not certify those separate paths or authorize another seat's credentials.
|
||||
- Mutation results are honestly separated into killed and surviving mutations. The byte-length mutation is equivalent only for a stable file; helper.md already notes the changing-file exception. The missing request-gating test above is material despite the existing suite passing.
|
||||
|
||||
## Validation
|
||||
|
||||
`node --test packages/ledger/tests/` in the isolated candidate: **56/56 passed**, no skips. The separate between-read replacement reproducer demonstrated finding 1 with helper exit 0 and an empty config received by stub curl. No real credential file was opened, printed or tested; no live API request, source edit in the shared checkout, commit or restart was made. Only this review report was added to the checkout.
|
||||
@@ -0,0 +1,31 @@
|
||||
# Gitea raw-token helper — final R2 review
|
||||
|
||||
Verdict: **changes requested**, one blocking finding. This is round 2 of 2; escalate the remaining blocker to Sage under item 27, not an automatic third round.
|
||||
|
||||
Target: `helper.patch`, sha256 `dd9e38bfa95d2e2305ad004a8583646bb90d6a5bf85cadf3140b3b688319770f`, applied alone to an isolated clone at `8efc0ff3`. Hash verified before reading and again after testing. Queue lookup returned `nothing`; the explicit review request supplies this assignment.
|
||||
|
||||
## 1. Blocking — inherited CFG retains its export attribute
|
||||
|
||||
Candidate `scripts/gitea-api.sh:74` assigns the secret configuration to `CFG` without clearing an inherited export attribute. In Bash, assigning a value to a variable imported from the environment does not make it private. The line-72 comment that CFG is not exported is therefore false when the caller already has an exported variable with that name.
|
||||
|
||||
Independent reproduction, with a dummy token and stub git/curl only:
|
||||
|
||||
- Start the helper with environment `CFG=innocent inherited value`.
|
||||
- Both validations pass, and line 74 replaces CFG with the credential config.
|
||||
- Stub curl observes the dummy token in its environment. The helper exits 0.
|
||||
|
||||
No caller needs to know or supply the credential value for this to happen. The body branch also starts external utilities after the assignment, so exposure is not limited to curl. This is newly introduced by storing the secret in a shell variable and violates decision 37's config-stream-only boundary.
|
||||
|
||||
**Required fix:** explicitly remove CFG's export attribute before launching any child with the captured secret, rather than relying on the absence of an `export CFG` statement. For example, a checked assignment followed immediately by Bash's builtin `export -n CFG`, before body-file creation or curl, addresses this inherited-attribute case. A task-specific name reduces collisions but does not replace clearing the attribute.
|
||||
|
||||
**Regression:** seed the helper environment with an exported, harmless CFG value. Exercise GET and POST using dummy raw and JSON credentials; assert that the token appears only in the config stream, never in child environment, argv or diagnostics. The current environment assertion runs without this seed, so it passes despite the bug. Removing the explicit attribute-clearing step must fail the regression.
|
||||
|
||||
## R1 disposition
|
||||
|
||||
The R1 blocker is **closed**. Configuration generation finishes synchronously and its failure prevents curl from starting in both branches. The added tests cover invalid text, invalid JSON shape, disallowed mode, symlink and missing-file changes between reads, plus a JSON-to-invalid transition. They assert exit 3 and zero curl calls rather than depending on an HTTP error. The nonempty check provides an additional guard.
|
||||
|
||||
The raw parser, fixed base URL and JSON validation rules retain the R1 behavior. The predictable response path remains a separate, acknowledged DEFERRED item and is not a blocker for this patch. Acting-seat path/login enforcement remains Piece D's integration responsibility, not certified by this helper-only review.
|
||||
|
||||
## Evidence and scope
|
||||
|
||||
The isolated R2 ledger suite passed **57/57**, no skips. Separately reproduced the inherited-export leak with the actual candidate helper and stubs, reporting only the boolean result, not credential contents. A small shell check confirmed that `export -n CFG` removes the inherited export attribute. No candidate source was changed for this review, and no real token file was opened or printed. No live API call, commit, push or restart occurred. Only this report was written in the shared checkout.
|
||||
@@ -0,0 +1,205 @@
|
||||
// scripts/gitea-api.sh with a raw per-seat token file (lead decision 37).
|
||||
// Every credential file here is a dummy written by the test. curl and git
|
||||
// are stubs: curl records its arguments and the config stream it was given,
|
||||
// and never reaches a network.
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
|
||||
const helper = fileURLToPath(new URL('../../../scripts/gitea-api.sh', import.meta.url));
|
||||
const DUMMY = '0123456789abcdef0123456789abcdef01234567';
|
||||
|
||||
function setup(t) {
|
||||
const dir = mkdtempSync(path.join(os.tmpdir(), 'gitea-helper-raw-'));
|
||||
t.after(() => rmSync(dir, { recursive: true, force: true }));
|
||||
const tool = (name, content) => { const p = path.join(dir, name); writeFileSync(p, '#!/usr/bin/env bash\n' + content); chmodSync(p, 0o755); };
|
||||
// git runs between the helper's two reads of the file, so STUB_SWAP
|
||||
// changes the file there.
|
||||
tool('git', [
|
||||
'f="$MOSAIC_GITEA_CREDENTIAL_FILE"',
|
||||
'case "${STUB_SWAP:-}" in',
|
||||
' invalid) printf "invalid-token\\n" > "$f";;',
|
||||
' json) printf "{}" > "$f";;',
|
||||
' mode) chmod 644 "$f";;',
|
||||
' symlink) mv "$f" "$f.moved"; ln -s "$f.moved" "$f";;',
|
||||
' missing) rm -f "$f";;',
|
||||
'esac',
|
||||
'printf %s https://git.mosaicstack.dev/mosaicstack/stack.git',
|
||||
'',
|
||||
].join('\n'));
|
||||
tool('curl', [
|
||||
'printf "%s\\n" "$@" > "$STUB_DIR/curl-args"',
|
||||
'env > "$STUB_DIR/curl-env"',
|
||||
'while (($#)); do case "$1" in -K) shift; cat "$1" > "$STUB_DIR/curl-cfg";; -o) shift; out="$1";; esac; shift; done',
|
||||
'printf "{}" > "$out"',
|
||||
'printf 200',
|
||||
'',
|
||||
].join('\n'));
|
||||
const cred = (content, mode = 0o600, name = 'gitea-mosaicstack-darkwing.token') => {
|
||||
const p = path.join(dir, name);
|
||||
rmSync(p, { force: true });
|
||||
writeFileSync(p, content);
|
||||
chmodSync(p, mode);
|
||||
return p;
|
||||
};
|
||||
const run = (file, extraEnv = {}, argv = ['GET', 'user']) => {
|
||||
const r = spawnSync('bash', [helper, ...argv], {
|
||||
encoding: 'utf8',
|
||||
env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, STUB_DIR: dir, MOSAIC_GITEA_CREDENTIAL_FILE: file, ...extraEnv },
|
||||
});
|
||||
const called = existsSync(path.join(dir, 'curl-args'));
|
||||
const args = called ? readFileSync(path.join(dir, 'curl-args'), 'utf8').split('\n') : null;
|
||||
const cfg = called ? readFileSync(path.join(dir, 'curl-cfg'), 'utf8') : null;
|
||||
const env = called ? readFileSync(path.join(dir, 'curl-env'), 'utf8') : null;
|
||||
for (const f of ['curl-args', 'curl-cfg', 'curl-env']) rmSync(path.join(dir, f), { force: true });
|
||||
return { status: r.status, stdout: r.stdout, stderr: r.stderr, called, args, cfg, env };
|
||||
};
|
||||
return { dir, cred, run };
|
||||
}
|
||||
|
||||
const header = (token) => `header = "Authorization: token ${token}"\nheader = "Content-Type: application/json"\n`;
|
||||
|
||||
test('a raw token file, with or without one trailing newline, reaches curl only through the config stream', t => {
|
||||
const s = setup(t);
|
||||
for (const content of [DUMMY, `${DUMMY}\n`]) {
|
||||
const r = s.run(s.cred(content));
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
assert.ok(r.called);
|
||||
assert.equal(r.cfg, header(DUMMY));
|
||||
assert.ok(r.args.includes('https://git.mosaicstack.dev/api/v1/user'), r.args.join(' '));
|
||||
assert.ok(!r.args.some((a) => a.includes(DUMMY)), 'the token is not in argv');
|
||||
assert.ok(!r.env.includes(DUMMY), 'the token is not in the environment curl gets');
|
||||
assert.ok(!r.stdout.includes(DUMMY) && !r.stderr.includes(DUMMY), 'the token is not printed');
|
||||
assert.equal(r.stdout, '{}');
|
||||
assert.match(r.stderr, /^HTTP 200$/m);
|
||||
}
|
||||
});
|
||||
|
||||
test('the raw path accepts nothing else, and refuses before curl runs', t => {
|
||||
const s = setup(t);
|
||||
const bad = [
|
||||
['empty', ''],
|
||||
['39 characters', DUMMY.slice(1)],
|
||||
['41 characters', `${DUMMY}8`],
|
||||
['upper case', DUMMY.toUpperCase().replace(/^0/, 'A')],
|
||||
['CRLF', `${DUMMY}\r\n`],
|
||||
['a trailing CR', `${DUMMY}\r`],
|
||||
['a trailing space', `${DUMMY} `],
|
||||
['a trailing tab', `${DUMMY}\t`],
|
||||
['two newlines', `${DUMMY}\n\n`],
|
||||
['leading space', ` ${DUMMY.slice(1)}`],
|
||||
['trailing space', `${DUMMY.slice(1)} `],
|
||||
['a second line', `${DUMMY}\nx`],
|
||||
['a quote', `${DUMMY.slice(2)}"\n`],
|
||||
['not hex', `${DUMMY.slice(1)}g`],
|
||||
['non-ASCII', `${DUMMY.slice(2)}é`],
|
||||
['80 characters', DUMMY + DUMMY],
|
||||
];
|
||||
for (const [what, content] of bad) {
|
||||
const r = s.run(s.cred(content));
|
||||
assert.equal(r.status, 3, `${what}: ${r.stderr}`);
|
||||
assert.equal(r.called, false, `${what}: curl ran`);
|
||||
assert.equal(r.stdout, '', what);
|
||||
}
|
||||
});
|
||||
|
||||
test('the file checks still apply on the raw path: mode, symlink, missing, directory', t => {
|
||||
const s = setup(t);
|
||||
// 000 and 200 pass the group and other check; the read then refuses.
|
||||
for (const mode of [0o640, 0o604, 0o660, 0o644, 0o000, 0o200]) {
|
||||
const r = s.run(s.cred(`${DUMMY}\n`, mode));
|
||||
assert.equal(r.status, 3, `mode ${mode.toString(8)}`);
|
||||
assert.equal(r.called, false);
|
||||
}
|
||||
const real = s.cred(`${DUMMY}\n`, 0o600, 'real.token');
|
||||
const link = path.join(s.dir, 'link.token');
|
||||
symlinkSync(real, link);
|
||||
assert.deepEqual([s.run(link).status, s.run(link).called], [3, false]);
|
||||
assert.deepEqual([s.run(path.join(s.dir, 'missing.token')).status, s.run(path.join(s.dir, 'missing.token')).called], [3, false]);
|
||||
assert.deepEqual([s.run(s.dir).status, s.run(s.dir).called], [3, false]);
|
||||
});
|
||||
|
||||
test('the raw path base URL has no override', t => {
|
||||
const s = setup(t);
|
||||
const r = s.run(s.cred(`${DUMMY}\n`), { MOSAIC_GITEA_URL: 'https://evil.example', GITEA_URL: 'https://evil.example', MOSAIC_GITEA_BASE_URL: 'https://evil.example' });
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
assert.ok(r.args.includes('https://git.mosaicstack.dev/api/v1/user'));
|
||||
assert.ok(!r.args.some((a) => a.includes('evil')));
|
||||
});
|
||||
|
||||
test('the JSON path is unchanged, and JSON never falls through to the raw path', t => {
|
||||
const s = setup(t);
|
||||
const json = (o) => s.cred(JSON.stringify(o), 0o600, 'mosaic.gitea.json');
|
||||
const good = s.run(json({ mosaicstack: { url: 'https://git.mosaicstack.dev/', api_token: 'json-dummy' } }));
|
||||
assert.equal(good.status, 0, good.stderr);
|
||||
assert.equal(good.cfg, header('json-dummy'));
|
||||
assert.ok(good.args.includes('https://git.mosaicstack.dev/api/v1/user'));
|
||||
const refused = [
|
||||
['another host', { mosaicstack: { url: 'https://evil.example', api_token: 'json-dummy' } }],
|
||||
['no token', { mosaicstack: { url: 'https://git.mosaicstack.dev' } }],
|
||||
['an empty token', { mosaicstack: { url: 'https://git.mosaicstack.dev', api_token: '' } }],
|
||||
['no mosaicstack key', { url: 'https://git.mosaicstack.dev', api_token: 'json-dummy' }],
|
||||
];
|
||||
for (const [what, o] of refused) {
|
||||
const r = s.run(json(o));
|
||||
assert.deepEqual([r.status, r.called], [3, false], what);
|
||||
}
|
||||
// Content that parses as JSON takes the JSON path even when it would pass
|
||||
// the raw pattern. A token of 40 decimal digits refuses there.
|
||||
for (const content of ['null', '"x"', '1234567890123456789012345678901234567890', '1234567890123456789012345678901234567890\n']) {
|
||||
const r = s.run(s.cred(content));
|
||||
assert.deepEqual([r.status, r.called], [3, false], JSON.stringify(content));
|
||||
}
|
||||
});
|
||||
|
||||
test('a file that changes between the two reads refuses before curl runs, with or without a body', t => {
|
||||
const s = setup(t);
|
||||
const post = ['POST', 'repos/mosaicstack/stack/issues/1508/comments', '{"body":"dummy"}'];
|
||||
// Unchanged, both calls reach curl, and the POST carries its body.
|
||||
for (const argv of [['GET', 'user'], post]) {
|
||||
const r = s.run(s.cred(`${DUMMY}\n`), {}, argv);
|
||||
assert.deepEqual([r.status, r.called, r.cfg], [0, true, header(DUMMY)], r.stderr);
|
||||
}
|
||||
assert.ok(s.run(s.cred(`${DUMMY}\n`), {}, post).args.includes('--data-binary'));
|
||||
for (const swap of ['invalid', 'json', 'mode', 'symlink', 'missing']) {
|
||||
for (const argv of [['GET', 'user'], post]) {
|
||||
const what = `${swap} ${argv[0]}`;
|
||||
const r = s.run(s.cred(`${DUMMY}\n`), { STUB_SWAP: swap }, argv);
|
||||
assert.deepEqual([r.status, r.called, r.stdout], [3, false, ''], what);
|
||||
assert.ok(!r.stderr.includes(DUMMY), what);
|
||||
}
|
||||
}
|
||||
const json = s.cred(JSON.stringify({ mosaicstack: { url: 'https://git.mosaicstack.dev', api_token: 'json-dummy' } }), 0o600, 'mosaic.gitea.json');
|
||||
const r = s.run(json, { STUB_SWAP: 'invalid' }, post);
|
||||
assert.deepEqual([r.status, r.called], [3, false], 'a JSON file that changes');
|
||||
});
|
||||
|
||||
test('the token reaches no child environment, even with an inherited CFG or SHELLOPTS=allexport', t => {
|
||||
const s = setup(t);
|
||||
const post = ['POST', 'repos/mosaicstack/stack/issues/1508/comments', '{"body":"dummy"}'];
|
||||
const files = [
|
||||
['raw', DUMMY, () => s.cred(`${DUMMY}\n`)],
|
||||
['JSON', 'json-dummy', () => s.cred(JSON.stringify({ mosaicstack: { url: 'https://git.mosaicstack.dev', api_token: 'json-dummy' } }), 0o600, 'mosaic.gitea.json')],
|
||||
];
|
||||
const seeds = [
|
||||
['an inherited CFG', { CFG: 'inherited harmless value' }],
|
||||
['SHELLOPTS=allexport', { SHELLOPTS: 'allexport' }],
|
||||
['both', { CFG: 'inherited harmless value', SHELLOPTS: 'allexport' }],
|
||||
];
|
||||
for (const [kind, token, make] of files) {
|
||||
for (const [seed, env] of seeds) {
|
||||
for (const argv of [['GET', 'user'], post]) {
|
||||
const what = `${kind}, ${seed}, ${argv[0]}`;
|
||||
const r = s.run(make(), env, argv);
|
||||
assert.deepEqual([r.status, r.called, r.cfg], [0, true, header(token)], `${what}: ${r.stderr}`);
|
||||
assert.ok(!r.env.includes(token), `${what}: the token is in curl's environment`);
|
||||
assert.ok(!r.args.some((a) => a.includes(token)), `${what}: the token is in argv`);
|
||||
assert.ok(!r.stdout.includes(token) && !r.stderr.includes(token), `${what}: the token is printed`);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
+86
-10
@@ -33,6 +33,8 @@ scripts/test-queue.sh
|
||||
| `sync [--op ID]` | yes | fsyncs and the witness; logs nothing |
|
||||
| `snapshot --out DIR` | yes | copies of both files into an empty DIR outside the repository |
|
||||
| `unlock [--check-gate]` | no; takes the unlock gate | removes a dead or mismatched lock |
|
||||
| `review request`, `review resolve`, `review abandon`, `review record` | yes | one log entry, the witness, the table; a request also posts one comment and logs its outcome |
|
||||
| `review verify-commit ID REF` | no | nothing; reads the queue and git objects |
|
||||
|
||||
Every change takes `--op ID` (8 to 72 characters of `[a-z0-9._-]`,
|
||||
starting with a letter or digit, not ending in `.outcome`) and an actor from `--by NAME` or
|
||||
@@ -52,12 +54,13 @@ or genesis from the map), it keeps the part still among the new issues, and
|
||||
the receipt ends in `(kept narrowed)`.
|
||||
|
||||
Exit codes: 0 ok; 1 the operation failed; 2 invalid data or refused;
|
||||
3 uncertain (visible or durable, not acknowledged); 4 usage.
|
||||
3 uncertain (visible or durable, not acknowledged; for a review request,
|
||||
not known to be posted); 4 usage.
|
||||
|
||||
Until piece D, `move ID in-review` needs `--candidate`: an existing file is
|
||||
read as a manifest (one `<sha256> <path>` line per file), anything else as a
|
||||
commit reachable from `refs/heads` or `refs/tags`. The candidate is frozen
|
||||
for the round.
|
||||
`move ID in-review` needs `--candidate`. An existing file is read as a
|
||||
manifest (one `<sha256> <path>` line per file), anything else as a commit
|
||||
reachable from `refs/heads` or `refs/tags`. The candidate is frozen for the
|
||||
round.
|
||||
|
||||
The review's issue follows lead decision 23. A row with no issues can't
|
||||
request review. A row with one issue uses it. A row with several needs
|
||||
@@ -66,10 +69,72 @@ unless `--issue` names another; if the row no longer lists the kept issue,
|
||||
the request refuses until `--issue` names one. Each round records the issue
|
||||
it used (`review.rounds[].issue`).
|
||||
|
||||
`move ID done` from in-review needs `--evidence
|
||||
comment=<id>,round=<n>,candidate=<digest>`. The round must be the current
|
||||
one and the digest its candidate's, so a comment from an earlier round
|
||||
can't close a later one, even when the candidate is the same.
|
||||
A row with reviewers opens a request round and posts the review request
|
||||
comment; see "Review requests". A row with no reviewers opens a round that
|
||||
posts nothing. For that kind of round, `move ID done` from in-review needs
|
||||
`--evidence comment=<id>,round=<n>,candidate=<digest>`. The round must be
|
||||
the current one and the digest its candidate's, so a comment from an
|
||||
earlier round can't close a later one, even when the candidate is the same.
|
||||
|
||||
## Review requests
|
||||
|
||||
Piece D, section 8.9 of the plan. `move ID in-review` from in-progress, on
|
||||
a row with reviewers, logs the new round and a request attempt in the `requesting`
|
||||
state, then posts one comment on the round's issue as the acting seat and
|
||||
logs what the transport said as a second entry, `<op>.outcome`. `review
|
||||
request ID` makes a new attempt in the current round after a failed or
|
||||
abandoned one.
|
||||
|
||||
- **Credentials.** The request posts with the acting seat's own token file,
|
||||
named by `MOSAIC_GITEA_CREDENTIAL_FILE`. It must end in
|
||||
`/agents/<login>/secrets/gitea-mosaicstack-<login>.token`, be a regular
|
||||
file (no symlink, and its real path ends the same way), belong to this
|
||||
user and be mode 0600. The lead's login is `jarvis`. The shared
|
||||
`~/secrets/mosaic.gitea.json` refuses. The queue checks the file with
|
||||
`lstat` and never opens it; `scripts/gitea-api.sh` reads it. Before the
|
||||
POST, `GET user` must name the seat's login.
|
||||
- **Outcome.** HTTP 201 with a comment id is `posted` (exit 0). HTTP 400,
|
||||
401, 403, 404 or 422, or a failed pre-send check, is `failed` (exit 1).
|
||||
Anything else is `uncertain` (exit 3): another status, 201 without an id,
|
||||
a transport error, or no answer before the 30-second deadline, which kills
|
||||
the helper and curl with it. The log records the status and a fixed
|
||||
detail, never the response body.
|
||||
- **Retries never post.** Running the same `--op` again prints what the log
|
||||
holds and sends nothing. A request left `requesting` (the process died
|
||||
after the log entry) or `uncertain` needs a person to look on the issue
|
||||
for a comment carrying `<!-- mosaic-queue-op: OP -->`. If it is there,
|
||||
the owner or a privileged actor runs `review resolve ID OP --comment N`.
|
||||
The queue fetches that comment and refuses unless it is on the round's
|
||||
issue, written by the requester's login, and carries both markers for
|
||||
this attempt, round and candidate. If it is not there, a privileged actor
|
||||
runs `review abandon ID OP --reason TEXT --yes`. An abandon sets the
|
||||
round's `duplicateRisk`, because the comment may still land.
|
||||
- **Late outcomes.** An outcome that arrives after a resolve with the same
|
||||
comment id keeps `posted`. One that disagrees, or a posted outcome after
|
||||
an abandon, makes the attempt `conflict`, which a resolve settles. The
|
||||
outcome entry is the one entry a done row still accepts, so a late answer
|
||||
is recorded even after the row closed.
|
||||
- **Unresolved requests** (`requesting`, `uncertain`, `conflict`) block a
|
||||
new request, a new round, a move to `waiting-on-jason` and `done`.
|
||||
- **Verdicts.** Each listed reviewer other than the owner posts a comment
|
||||
on the issue, then runs `review record ID --verdict approve|changes
|
||||
--comment N --candidate DIGEST`, once per round. DIGEST must be the
|
||||
round's candidate. On a request round, `move ID done` and
|
||||
`move ID waiting-on-jason` each need an approval recorded by every
|
||||
reviewer in the current round, so a Jason-gated row reaches Jason only
|
||||
after its reviewers approved. `done` also refuses `--evidence`. The queue
|
||||
doesn't fetch the verdict comment; `record` is cooperative like every
|
||||
other actor claim.
|
||||
- **`review verify-commit ID REF`** checks a prospective commit or tree
|
||||
against the current round's candidate. For a manifest, every listed path
|
||||
must hash to its digest. For a commit candidate, every path the candidate
|
||||
changed from its first parent must match in REF by blob and mode, and
|
||||
every path it deleted must be absent.
|
||||
|
||||
Each log entry records the `semantics` it was written under. Entries from
|
||||
semantics 1, before piece D, replay under the old rules: a move to
|
||||
in-review opens a round that posts nothing. Review entries need
|
||||
semantics 2.
|
||||
|
||||
## Where the files live
|
||||
|
||||
@@ -223,6 +288,12 @@ reporting, so a write in progress is never reported as lost history.
|
||||
witness continuity. Canonical `verify`, under the lock, does that.
|
||||
- **Commit candidates** stay retrievable only while some ref keeps the
|
||||
commit. The queue keeps the manifest text, not the source bytes.
|
||||
`review verify-commit` on a commit candidate needs that commit.
|
||||
- **Post to outcome.** A kill after the POST and before the outcome entry
|
||||
leaves the attempt `requesting` with a comment on the issue. The queue
|
||||
never posts on its own again; `review resolve` records the comment.
|
||||
- **Verdict comments** are not fetched. `review record` trusts the
|
||||
reviewer's comment id the way the queue trusts `--by`.
|
||||
- **Locks** are never removed for their age. `unknown` and `invalid` locks
|
||||
wait for a person. A host rename makes old locks `unknown`.
|
||||
- `queue unlock --check-gate` classifies a stale unlock gate. Remove the
|
||||
@@ -264,7 +335,7 @@ then `queue render`.
|
||||
`node --test packages/queue/tests/` runs everything in scratch repositories
|
||||
under the system temp directory; nothing touches this checkout's `.git`.
|
||||
Faults reach the code only through options the tests pass in (`io`, `proc`,
|
||||
`hook`, `now`, `readOrder`, `lockWaitMs`); the CLI passes none.
|
||||
`hook`, `now`, `readOrder`, `lockWaitMs`, `deadlineMs`); the CLI passes none.
|
||||
|
||||
- `data.test.mjs`: serialization, replay, the transition matrix, `next`
|
||||
ordering, render.
|
||||
@@ -280,3 +351,8 @@ Faults reach the code only through options the tests pass in (`io`, `proc`,
|
||||
`map-check.mjs` reports each kind of drift.
|
||||
- `commit.test.mjs`: `queue-commit.sh` and the guard, through PATH shims
|
||||
that run an action at an exact point in the procedure.
|
||||
- `review.test.mjs`: review requests against `fixtures/fake-gitea.mjs`,
|
||||
installed as the scratch repository's `scripts/gitea-api.sh`, with dummy
|
||||
token files. Every transport answer, the deadline, kills at each step,
|
||||
a held lock at the outcome, late outcomes, resolve checks, credential
|
||||
checks, verdicts across rounds, `verify-commit` and semantics 1 replay.
|
||||
|
||||
@@ -8,17 +8,22 @@
|
||||
// release ID | assign ID SEAT | note ID TEXT | set ID FIELD VALUE [--reason TEXT]
|
||||
// genesis --root PATH --branch NAME --map PATH
|
||||
// accept-history --reason TEXT --yes
|
||||
// review request ID | review resolve ID ATTEMPT --comment N
|
||||
// review abandon ID ATTEMPT --reason TEXT --yes
|
||||
// review record ID --verdict approve|changes --comment N --candidate DIGEST
|
||||
// Each needs --op ID and an actor (--by NAME or $MOSAIC_AGENT_NAME).
|
||||
// Read only: review verify-commit ID REF
|
||||
// No log: render [--check] | verify [--current] | verify --snapshot DIR (--base-file F | --base-absent)
|
||||
// sync [--op ID] | snapshot --out DIR | unlock [--check-gate]
|
||||
//
|
||||
// Exit codes: 0 ok; 1 operation failed; 2 invalid data or refused;
|
||||
// 3 uncertain (visible or durable, not acknowledged); 4 usage.
|
||||
// 3 uncertain (visible or durable, not acknowledged; for a request, not
|
||||
// known to be posted); 4 usage.
|
||||
import { realpathSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { QueueError } from "./errors.mjs";
|
||||
import { SET_FIELDS } from "./queue.mjs";
|
||||
import { list, mutate, next, renderView, show, snapshot, sync, unlock, verify, verifySnapshot } from "./store.mjs";
|
||||
import { list, mutate, next, renderView, show, snapshot, sync, unlock, verify, verifyCommit, verifySnapshot } from "./store.mjs";
|
||||
|
||||
const USAGE = [
|
||||
"usage: queue list | show ID | next [SEAT]",
|
||||
@@ -28,6 +33,10 @@ const USAGE = [
|
||||
` queue set ID FIELD VALUE --op ID [--reason TEXT] (fields: ${SET_FIELDS.join(", ")})`,
|
||||
" queue genesis --op ID --root PATH --branch NAME --map PATH",
|
||||
" queue accept-history --op ID --reason TEXT --yes",
|
||||
" queue review request ID --op ID | review resolve ID ATTEMPT --comment N --op ID",
|
||||
" queue review abandon ID ATTEMPT --reason TEXT --yes --op ID",
|
||||
" queue review record ID --verdict approve|changes --comment N --candidate DIGEST --op ID",
|
||||
" queue review verify-commit ID REF",
|
||||
" queue render [--check] | verify [--current] | verify --snapshot DIR (--base-file F | --base-absent)",
|
||||
" queue sync [--op ID] | snapshot --out DIR | unlock [--check-gate]",
|
||||
" every change takes --by NAME, else $MOSAIC_AGENT_NAME",
|
||||
@@ -36,6 +45,7 @@ const USAGE = [
|
||||
const VALUE_FLAGS = new Set([
|
||||
"--op", "--by", "--piece", "--gate", "--brief", "--issue", "--note", "--owner", "--gate-owner", "--after", "--reviewer",
|
||||
"--reason", "--candidate", "--evidence", "--root", "--branch", "--map", "--snapshot", "--base-file", "--out",
|
||||
"--verdict", "--comment",
|
||||
]);
|
||||
const REPEATED = new Set(["--issue", "--after", "--reviewer"]);
|
||||
const BOOL_FLAGS = new Set(["--required", "--yes", "--check", "--current", "--base-absent", "--check-gate"]);
|
||||
@@ -80,6 +90,38 @@ function intArg(v, what) {
|
||||
return Number(v);
|
||||
}
|
||||
|
||||
// Gitea comment ids outgrow a row id's seven digits.
|
||||
function commentArg(v) {
|
||||
if (v === null) throw usage("--comment N is required");
|
||||
if (!/^[1-9][0-9]{0,15}$/.test(v) || !Number.isSafeInteger(Number(v))) throw usage(`--comment must be a comment id: ${JSON.stringify(v)}`);
|
||||
return Number(v);
|
||||
}
|
||||
|
||||
function review(pos, flags, f, change, opts) {
|
||||
const [sub, ...rest] = pos;
|
||||
switch (sub) {
|
||||
case "request":
|
||||
allow(flags, CHANGE); positional(rest, 1, "review request ID");
|
||||
return change("review-request", { id: intArg(rest[0], "ID") });
|
||||
case "resolve":
|
||||
allow(flags, [...CHANGE, "--comment"]); positional(rest, 2, "review resolve ID ATTEMPT");
|
||||
return change("review-resolve", { id: intArg(rest[0], "ID"), attempt: rest[1], comment: commentArg(f("--comment")) });
|
||||
case "abandon":
|
||||
allow(flags, [...CHANGE, "--reason", "--yes"]); positional(rest, 2, "review abandon ID ATTEMPT");
|
||||
if (f("--reason") === null) throw usage("review abandon needs --reason");
|
||||
return change("review-abandon", { id: intArg(rest[0], "ID"), attempt: rest[1], reason: f("--reason") }, { yes: flags.has("--yes") });
|
||||
case "record":
|
||||
allow(flags, [...CHANGE, "--verdict", "--comment", "--candidate"]); positional(rest, 1, "review record ID");
|
||||
for (const k of ["--verdict", "--candidate"]) if (f(k) === null) throw usage(`review record needs ${k}`);
|
||||
return change("review-record", { id: intArg(rest[0], "ID"), verdict: f("--verdict"), comment: commentArg(f("--comment")), candidate: f("--candidate") });
|
||||
case "verify-commit":
|
||||
allow(flags, []); positional(rest, 2, "review verify-commit ID REF");
|
||||
return verifyCommit(opts, intArg(rest[0], "ID"), rest[1]);
|
||||
default:
|
||||
throw usage("review takes request, resolve, abandon, record or verify-commit");
|
||||
}
|
||||
}
|
||||
|
||||
function afterArg(v) {
|
||||
const m = /^([1-9][0-9]{0,6})(?::(done|settled))?$/.exec(v);
|
||||
if (!m) throw usage(`--after takes ID or ID:settled: ${JSON.stringify(v)}`);
|
||||
@@ -161,6 +203,8 @@ export function run(argv, opts = {}) {
|
||||
allow(flags, [...CHANGE, "--reason", "--yes"]); positional(pos, 0, "only options");
|
||||
if (f("--reason") === null) throw usage("accept-history needs --reason");
|
||||
return change("accept-history", { reason: f("--reason") }, { yes: flags.has("--yes") });
|
||||
case "review":
|
||||
return review(pos, flags, f, change, opts);
|
||||
case "render":
|
||||
allow(flags, ["--check"]); positional(pos, 0, "no arguments");
|
||||
return renderView(opts, { check: flags.has("--check") });
|
||||
|
||||
+323
-20
@@ -6,19 +6,29 @@ import { createHash } from "node:crypto";
|
||||
import { QueueError } from "./errors.mjs";
|
||||
|
||||
export const VERSION = 1;
|
||||
export const SEMANTICS = 1;
|
||||
// Semantics 2 is Piece D: a move into review on a row with reviewers opens
|
||||
// a request round, and the review-* verbs exist. Each entry replays under
|
||||
// the semantics it was written with.
|
||||
export const SEMANTICS = 2;
|
||||
export const STATES = ["queued", "briefed", "in-progress", "in-review", "waiting-on-jason", "done", "blocked", "parked"];
|
||||
const NON_TERMINAL = new Set(["queued", "briefed", "in-progress", "in-review", "waiting-on-jason"]);
|
||||
const CLAIM_KEPT = new Set(["in-progress", "in-review", "waiting-on-jason"]);
|
||||
export const PRIVILEGED = new Set(["jason", "sage"]);
|
||||
export const VERBS = ["genesis", "add", "move", "release", "assign", "note", "set", "accept-history"];
|
||||
export const REVIEW_VERBS = ["review-request", "review-outcome", "review-resolve", "review-abandon", "review-record"];
|
||||
export const VERBS = ["genesis", "add", "move", "release", "assign", "note", "set", "accept-history", ...REVIEW_VERBS];
|
||||
export const ATTEMPT_STATES = ["requesting", "posted", "failed", "uncertain", "abandoned", "conflict"];
|
||||
// Attempts that block a new request on their row (8.9).
|
||||
export const UNRESOLVED_STATES = ["requesting", "uncertain", "conflict"];
|
||||
const UNRESOLVED = new Set(UNRESOLVED_STATES);
|
||||
// The endpoint answered and refused: nothing was posted (8.9).
|
||||
export const FAILED_CODES = [400, 401, 403, 404, 422];
|
||||
export const SET_FIELDS = ["piece", "gate", "gate-owner", "after", "reviewers", "issues", "closes", "brief", "required"];
|
||||
|
||||
const NAME_RE = /^[a-z][a-z0-9-]{0,31}$/;
|
||||
export const CALLER_OP_RE = /^[a-z0-9][a-z0-9._-]{7,71}$/;
|
||||
// Room for `<op>.outcome`, which only an op id the CLI derives may use. No
|
||||
// verb derives one before Piece D, so replay applies CALLER_OP_RE to every
|
||||
// entry, genesis included.
|
||||
// Room for `<op>.outcome`, the op id of the entry that records a review
|
||||
// attempt's transport outcome (8.9). Only that verb uses it; every other
|
||||
// entry's op, genesis included, is a caller's op.
|
||||
export const LOG_OP_RE = /^[a-z0-9][a-z0-9._-]{7,79}$/;
|
||||
const ISO_RE = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/;
|
||||
const DATE_RE = /^\d{4}-\d{2}-\d{2}$/;
|
||||
@@ -187,15 +197,109 @@ export function parseManifest(text) {
|
||||
return lines.length;
|
||||
}
|
||||
|
||||
// A checked manifest's lines as {digest, path}.
|
||||
export function manifestEntries(text) {
|
||||
parseManifest(text);
|
||||
return text.slice(0, -1).split("\n").map((line) => ({ digest: line.slice(0, 64), path: line.slice(66) }));
|
||||
}
|
||||
|
||||
// The request round an attempt belongs to, or null.
|
||||
export function attemptOf(row, op) {
|
||||
for (const r of row.review?.rounds ?? []) {
|
||||
if (r.request !== "comment") continue;
|
||||
const a = r.attempts.find((x) => x.op === op);
|
||||
if (a) return { round: r, attempt: a };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function checkCommentId(v, what = "comment id") {
|
||||
if (!Number.isSafeInteger(v) || v < 1) throw refuse(`${what} must be a positive integer: ${JSON.stringify(v)}`);
|
||||
return v;
|
||||
}
|
||||
|
||||
function checkDigest(v, what) {
|
||||
if (typeof v !== "string" || !(BLOB_RE.test(v) || SHA256_RE.test(v))) throw refuse(`${what} must be a 40-hex commit id or a 64-hex SHA-256`);
|
||||
return v;
|
||||
}
|
||||
|
||||
// What the transport reported for one attempt (8.9 step 3). `status` is
|
||||
// null when no HTTP status came back, which for `failed` means a pre-send
|
||||
// failure.
|
||||
export function checkTransport(t) {
|
||||
keysExactly(t, ["outcome", "status", "comment", "detail"], "transport outcome");
|
||||
if (!["posted", "failed", "uncertain"].includes(t.outcome)) throw refuse("transport outcome must be posted, failed or uncertain");
|
||||
if (t.status !== null && (!Number.isInteger(t.status) || t.status < 100 || t.status > 599)) throw refuse("transport status must be an HTTP status or null");
|
||||
if (t.comment !== null) checkCommentId(t.comment, "transport comment id");
|
||||
if ((t.outcome === "posted") !== (t.comment !== null) || (t.outcome === "posted" && t.status !== 201)) throw refuse("a transport outcome is posted exactly when HTTP 201 returned a comment id");
|
||||
if (t.outcome === "failed" && t.status !== null && !FAILED_CODES.includes(t.status)) throw refuse(`a failed outcome's status is one of ${FAILED_CODES.join(", ")}, or null before the POST`);
|
||||
checkText(t.detail, "transport detail", { max: 200 });
|
||||
return t;
|
||||
}
|
||||
|
||||
function checkAttempt(a) {
|
||||
keysExactly(a, ["op", "by", "at", "state", "comment", "transport", "resolutions"], "review attempt");
|
||||
checkCallerOpId(a.op, "review attempt op");
|
||||
checkName(a.by, "review attempt by");
|
||||
checkTime(a.at, "review attempt at");
|
||||
if (!ATTEMPT_STATES.includes(a.state)) throw refuse(`review attempt state ${JSON.stringify(a.state)} is not one of ${ATTEMPT_STATES.join(", ")}`);
|
||||
if (a.comment !== null) checkCommentId(a.comment);
|
||||
if ((a.state === "posted") !== (a.comment !== null)) throw refuse("a review attempt carries a comment id exactly when it is posted");
|
||||
if (a.transport !== null) checkTransport(a.transport);
|
||||
if (!Array.isArray(a.resolutions) || a.resolutions.length > 2) throw refuse("a review attempt holds at most two resolutions");
|
||||
for (const r of a.resolutions) {
|
||||
keysExactly(r, ["verb", "op", "by", "at", "comment", "reason"], "review resolution");
|
||||
checkCallerOpId(r.op, "review resolution op");
|
||||
checkName(r.by, "review resolution by");
|
||||
checkTime(r.at, "review resolution at");
|
||||
if (r.verb === "resolve") {
|
||||
checkCommentId(r.comment);
|
||||
if (r.reason !== null) throw refuse("a resolve carries no reason");
|
||||
} else if (r.verb === "abandon") {
|
||||
if (r.comment !== null) throw refuse("an abandon carries no comment id");
|
||||
checkText(r.reason, "abandon reason");
|
||||
} else {
|
||||
throw refuse("a review resolution is resolve or abandon");
|
||||
}
|
||||
}
|
||||
if (a.state === "requesting" && (a.transport !== null || a.resolutions.length > 0)) throw refuse("a requesting attempt has no outcome and no resolution");
|
||||
if (["failed", "uncertain", "conflict"].includes(a.state) && a.transport === null) throw refuse(`a ${a.state} attempt needs its transport outcome`);
|
||||
}
|
||||
|
||||
function checkReceipt(r) {
|
||||
keysExactly(r, ["reviewer", "op", "at", "verdict", "comment", "candidate"], "review receipt");
|
||||
checkName(r.reviewer, "review receipt reviewer");
|
||||
checkCallerOpId(r.op, "review receipt op");
|
||||
checkTime(r.at, "review receipt at");
|
||||
if (r.verdict !== "approve" && r.verdict !== "changes") throw refuse("a verdict is approve or changes");
|
||||
checkCommentId(r.comment, "review receipt comment id");
|
||||
checkDigest(r.candidate, "review receipt candidate");
|
||||
}
|
||||
|
||||
const ROUND_KEYS = ["n", "op", "by", "at", "issue", "candidate", "request"];
|
||||
|
||||
// A round is `request: "none"` (opened before Piece D, or on a row with no
|
||||
// reviewers) or `request: "comment"`: the move posts a request comment, and
|
||||
// the round keeps its attempts and the reviewers' receipts (8.9).
|
||||
function checkRound(v, n) {
|
||||
keysExactly(v, ["n", "op", "by", "at", "issue", "candidate", "request"], "review round");
|
||||
keysExactly(v, isObj(v) && v.request === "comment" ? [...ROUND_KEYS, "attempts", "duplicateRisk", "receipts"] : ROUND_KEYS, "review round");
|
||||
if (v.n !== n) throw refuse(`review rounds must be numbered from 1; expected ${n}`);
|
||||
checkCallerOpId(v.op, "review round op");
|
||||
checkName(v.by, "review round by");
|
||||
checkTime(v.at, "review round at");
|
||||
checkId(v.issue, "review round issue");
|
||||
checkCandidate(v.candidate);
|
||||
if (v.request !== "none") throw refuse("review round request must be none before Piece D");
|
||||
if (v.request === "none") return;
|
||||
if (v.request !== "comment") throw refuse("review round request must be none or comment");
|
||||
if (!Array.isArray(v.attempts) || v.attempts.length === 0) throw refuse("a request round needs at least one attempt");
|
||||
v.attempts.forEach(checkAttempt);
|
||||
if (v.attempts[0].op !== v.op) throw refuse("a request round's first attempt is the move that opened it");
|
||||
if (new Set(v.attempts.map((a) => a.op)).size !== v.attempts.length) throw refuse("a request round names an attempt twice");
|
||||
const abandoned = v.attempts.some((a) => a.resolutions.some((r) => r.verb === "abandon"));
|
||||
if (v.duplicateRisk !== abandoned) throw refuse("a request round's duplicateRisk is true exactly when an attempt was abandoned");
|
||||
if (!Array.isArray(v.receipts)) throw refuse("a request round's receipts must be a list");
|
||||
v.receipts.forEach(checkReceipt);
|
||||
if (new Set(v.receipts.map((r) => r.reviewer)).size !== v.receipts.length) throw refuse("a reviewer has one receipt per round");
|
||||
}
|
||||
|
||||
export function validateRow(row) {
|
||||
@@ -377,6 +481,19 @@ export function canonArgs(verb, a) {
|
||||
}
|
||||
case "accept-history":
|
||||
return { reason: checkText(a.reason, "reason") };
|
||||
case "review-request":
|
||||
return { id: checkId(a.id) };
|
||||
case "review-outcome": {
|
||||
const t = checkTransport({ outcome: a.outcome, status: a.status, comment: a.comment, detail: a.detail });
|
||||
return { id: checkId(a.id), attempt: checkCallerOpId(a.attempt, "attempt"), ...t };
|
||||
}
|
||||
case "review-resolve":
|
||||
return { id: checkId(a.id), attempt: checkCallerOpId(a.attempt, "attempt"), comment: checkCommentId(a.comment) };
|
||||
case "review-abandon":
|
||||
return { id: checkId(a.id), attempt: checkCallerOpId(a.attempt, "attempt"), reason: checkText(a.reason, "reason") };
|
||||
case "review-record":
|
||||
if (a.verdict !== "approve" && a.verdict !== "changes") throw refuse("--verdict must be approve or changes");
|
||||
return { id: checkId(a.id), verdict: a.verdict, comment: checkCommentId(a.comment), candidate: checkDigest(a.candidate, "candidate") };
|
||||
default:
|
||||
throw refuse(`unknown verb ${JSON.stringify(verb)}`);
|
||||
}
|
||||
@@ -392,8 +509,8 @@ const isPriv = (by) => PRIVILEGED.has(by);
|
||||
|
||||
function ownerOrPriv(row, by, doing) {
|
||||
if (by === row.owner || isPriv(by)) return;
|
||||
if (row.claim !== null) throw refuse(`row ${row.id} is claimed by ${row.claim.seat}; ${by} cannot ${doing}`);
|
||||
throw refuse(`only the owner (${row.owner}) or a privileged actor may ${doing} row ${row.id}`);
|
||||
const claimed = row.claim !== null ? `row ${row.id} is claimed by ${row.claim.seat}; ` : "";
|
||||
throw refuse(`${claimed}only the owner (${row.owner}) or a privileged actor may ${doing} row ${row.id}`);
|
||||
}
|
||||
|
||||
function requirePriv(by, doing) {
|
||||
@@ -442,6 +559,146 @@ function reviewIssue(row, issue) {
|
||||
return row.issues[0];
|
||||
}
|
||||
|
||||
// Attempts still waiting on an outcome, a resolve or an abandon. A new
|
||||
// request on the row, and closing it, wait for them (8.9).
|
||||
function unresolvedAttempts(row) {
|
||||
const out = [];
|
||||
for (const r of row.review?.rounds ?? []) {
|
||||
if (r.request !== "comment") continue;
|
||||
for (const a of r.attempts) if (UNRESOLVED.has(a.state)) out.push(`${a.op} (round ${r.n}, ${a.state})`);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function refuseUnresolved(row, doing) {
|
||||
const open = unresolvedAttempts(row);
|
||||
if (open.length) throw refuse(`row ${row.id} has an unresolved review request: ${open.join(", ")}; resolve or abandon it before ${doing}`);
|
||||
}
|
||||
|
||||
// A comment round leaves in-review, for done or for Jason's gate, only on
|
||||
// an approval recorded by every listed reviewer (8.9).
|
||||
function requireApprovals(row, cur, to) {
|
||||
if (row.reviewers.length === 0) throw refuse(`row ${row.id} lists no reviewers now; a privileged actor sets them before it ${to === "done" ? "closes" : `moves to ${to}`}`);
|
||||
const approved = new Set(cur.receipts.filter((r) => r.verdict === "approve").map((r) => r.reviewer));
|
||||
const missing = row.reviewers.filter((s) => !approved.has(s));
|
||||
if (missing.length) throw refuse(`row ${row.id} round ${cur.n} has no approval recorded by ${missing.join(", ")}`);
|
||||
}
|
||||
|
||||
// The request round an attempt belongs to, and the attempt.
|
||||
function findAttempt(row, op) {
|
||||
for (const r of row.review?.rounds ?? []) {
|
||||
if (r.request !== "comment") continue;
|
||||
const i = r.attempts.findIndex((x) => x.op === op);
|
||||
if (i >= 0) return { round: r, index: i, attempt: r.attempts[i] };
|
||||
}
|
||||
throw refuse(`row ${row.id} has no review request ${op}`);
|
||||
}
|
||||
|
||||
// A copy of `row` with one attempt of one round replaced.
|
||||
function withAttempt(row, found, attempt, roundExtra = {}) {
|
||||
const rounds = row.review.rounds.map((r) => {
|
||||
if (r.n !== found.round.n) return r;
|
||||
return { ...r, ...roundExtra, attempts: r.attempts.map((x, i) => (i === found.index ? attempt : x)) };
|
||||
});
|
||||
return { ...row, review: { rounds } };
|
||||
}
|
||||
|
||||
// The current round, which review-request and review-record act on.
|
||||
function currentCommentRound(row, doing) {
|
||||
if (row.state !== "in-review") throw refuse(`row ${row.id} is ${row.state}; ${doing} applies to in-review rows`);
|
||||
const cur = row.review.rounds.at(-1);
|
||||
if (cur.request !== "comment") throw refuse(`row ${row.id} round ${cur.n} posts no request (the row had no reviewers when it opened); ${doing} does not apply`);
|
||||
return cur;
|
||||
}
|
||||
|
||||
function newAttempt(entry) {
|
||||
return { op: entry.op, by: entry.by, at: entry.at, state: "requesting", comment: null, transport: null, resolutions: [] };
|
||||
}
|
||||
|
||||
// review-outcome: what the transport reported, against the attempt's state
|
||||
// by then (8.9 step 3). A resolve or abandon may have come first.
|
||||
function outcomeState(attempt, t) {
|
||||
if (attempt.state === "requesting") return t.outcome;
|
||||
if (attempt.state === "posted") {
|
||||
if (t.outcome === "uncertain") return "posted";
|
||||
return t.outcome === "posted" && t.comment === attempt.comment ? "posted" : "conflict";
|
||||
}
|
||||
if (attempt.state === "abandoned") return t.outcome === "posted" ? "conflict" : "abandoned";
|
||||
throw refuse(`attempt ${attempt.op} is ${attempt.state}; it already has its outcome`);
|
||||
}
|
||||
|
||||
function applyReview(rows, row, entry) {
|
||||
const a = entry.args;
|
||||
const by = entry.by;
|
||||
if (!row.review && entry.verb !== "review-request" && entry.verb !== "review-record") throw refuse(`row ${row.id} has no review request ${a.attempt}`);
|
||||
switch (entry.verb) {
|
||||
case "review-request": {
|
||||
ownerOrPriv(row, by, "request review of");
|
||||
const cur = currentCommentRound(row, "review request");
|
||||
const live = cur.attempts.filter((x) => x.state !== "failed" && x.state !== "abandoned");
|
||||
refuseUnresolved(row, "a new request");
|
||||
if (live.length) throw refuse(`row ${row.id} round ${cur.n} already has a posted request (${live[0].op}, comment ${live[0].comment})`);
|
||||
const rounds = row.review.rounds.map((r) => (r.n === cur.n ? { ...r, attempts: [...r.attempts, newAttempt(entry)] } : r));
|
||||
return {
|
||||
row: { ...row, review: { rounds } },
|
||||
result: { row: row.id, round: cur.n, attempt: entry.op, state: "requesting" },
|
||||
text: `row ${row.id} round ${cur.n} request ${entry.op} requesting`,
|
||||
};
|
||||
}
|
||||
case "review-outcome": {
|
||||
if (entry.op !== `${a.attempt}.outcome`) throw refuse(`an outcome's op is its attempt's op plus .outcome (${a.attempt}.outcome)`);
|
||||
const found = findAttempt(row, a.attempt);
|
||||
const att = found.attempt;
|
||||
if (by !== att.by) throw refuse(`only ${att.by}, who made request ${att.op}, records its outcome`);
|
||||
if (att.transport !== null) throw refuse(`request ${att.op} already has its outcome`);
|
||||
const t = { outcome: a.outcome, status: a.status, comment: a.comment, detail: a.detail };
|
||||
const state = outcomeState(att, t);
|
||||
const comment = state === "posted" ? (att.comment ?? t.comment) : null;
|
||||
const next = withAttempt(row, found, { ...att, state, comment, transport: t });
|
||||
const shown = state === t.outcome ? state : `${state} (transport ${t.outcome})`;
|
||||
return {
|
||||
row: next,
|
||||
result: { row: row.id, round: found.round.n, attempt: att.op, state },
|
||||
text: `row ${row.id} round ${found.round.n} request ${att.op} ${shown}${comment !== null ? ` comment ${comment}` : ""}`,
|
||||
};
|
||||
}
|
||||
case "review-resolve":
|
||||
case "review-abandon": {
|
||||
const resolve = entry.verb === "review-resolve";
|
||||
if (row.state === "done") throw refuse(`row ${row.id} is done; done rows never change`);
|
||||
if (resolve) ownerOrPriv(row, by, "resolve a request on");
|
||||
else requirePriv(by, "abandon a review request");
|
||||
const found = findAttempt(row, a.attempt);
|
||||
const att = found.attempt;
|
||||
if (!UNRESOLVED.has(att.state)) throw refuse(`request ${att.op} is ${att.state}; only a requesting, uncertain or conflict request can be ${resolve ? "resolved" : "abandoned"}`);
|
||||
const res = { verb: resolve ? "resolve" : "abandon", op: entry.op, by, at: entry.at, comment: resolve ? a.comment : null, reason: resolve ? null : a.reason };
|
||||
const state = resolve ? "posted" : "abandoned";
|
||||
const next = withAttempt(row, found, { ...att, state, comment: resolve ? a.comment : null, resolutions: [...att.resolutions, res] }, resolve ? {} : { duplicateRisk: true });
|
||||
return {
|
||||
row: next,
|
||||
result: { row: row.id, round: found.round.n, attempt: att.op, state },
|
||||
text: `row ${row.id} round ${found.round.n} request ${att.op} ${att.state}→${state}${resolve ? ` comment ${a.comment}` : "; a duplicate request comment may exist"}`,
|
||||
};
|
||||
}
|
||||
case "review-record": {
|
||||
if (!row.reviewers.includes(by) || by === row.owner) throw refuse(`only a listed reviewer other than the owner may record a verdict on row ${row.id} (reviewers: ${fmt(row.reviewers)})`);
|
||||
if (!row.review) throw refuse(`row ${row.id} has no review round`);
|
||||
const cur = currentCommentRound(row, "review record");
|
||||
if (a.candidate !== cur.candidate.digest) throw refuse(`candidate ${a.candidate} is not round ${cur.n}'s candidate ${cur.candidate.digest}`);
|
||||
if (cur.receipts.some((r) => r.reviewer === by)) throw refuse(`${by} already recorded a verdict for row ${row.id} round ${cur.n}`);
|
||||
const rec = { reviewer: by, op: entry.op, at: entry.at, verdict: a.verdict, comment: a.comment, candidate: a.candidate };
|
||||
const rounds = row.review.rounds.map((r) => (r.n === cur.n ? { ...r, receipts: [...r.receipts, rec] } : r));
|
||||
return {
|
||||
row: { ...row, review: { rounds } },
|
||||
result: { row: row.id, round: cur.n, verdict: a.verdict },
|
||||
text: `row ${row.id} round ${cur.n} ${a.verdict} by ${by} (comment ${a.comment})`,
|
||||
};
|
||||
}
|
||||
default:
|
||||
throw refuse(`unknown verb ${entry.verb}`);
|
||||
}
|
||||
}
|
||||
|
||||
function touch(row, entry) {
|
||||
return { ...row, updatedAt: entry.at, updatedBy: entry.by };
|
||||
}
|
||||
@@ -469,6 +726,8 @@ function applyMove(rows, row, entry, resolved) {
|
||||
const { to, reason, candidate, evidence, issue } = entry.args;
|
||||
const by = entry.by;
|
||||
const from = row.state;
|
||||
const v2 = entry.semantics >= 2;
|
||||
let request = null;
|
||||
const illegal = () => refuse(`row ${row.id}: ${from}→${to} is not a transition`);
|
||||
if (from === "done") throw refuse(`row ${row.id} is done; done rows never change`);
|
||||
if (reason !== null && to !== "blocked") throw refuse("--reason applies only to a move to blocked");
|
||||
@@ -505,12 +764,23 @@ function applyMove(rows, row, entry, resolved) {
|
||||
cand = checkCandidate(resolved.candidate);
|
||||
const rounds = row.review ? row.review.rounds : [];
|
||||
round = rounds.length + 1;
|
||||
next.review = {
|
||||
rounds: [...rounds, { n: round, op: entry.op, by, at: entry.at, issue: revIssue, candidate: cand, request: "none" }],
|
||||
};
|
||||
} else if (from === "in-review" && (to === "in-progress" || to === "waiting-on-jason")) {
|
||||
let opened = { n: round, op: entry.op, by, at: entry.at, issue: revIssue, candidate: cand, request: "none" };
|
||||
// Semantics 2: a row with reviewers asks them in a comment (8.9).
|
||||
if (v2 && row.reviewers.length > 0) {
|
||||
refuseUnresolved(row, "a new round");
|
||||
opened = { ...opened, request: "comment", attempts: [newAttempt(entry)], duplicateRisk: false, receipts: [] };
|
||||
request = `; request ${entry.op} requesting`;
|
||||
}
|
||||
next.review = { rounds: [...rounds, opened] };
|
||||
} else if (from === "in-review" && to === "in-progress") {
|
||||
ownerOrPriv(row, by, `move to ${to}`);
|
||||
} else if (from === "in-review" && to === "waiting-on-jason") {
|
||||
ownerOrPriv(row, by, `move to ${to}`);
|
||||
refuseUnresolved(row, "moving it to waiting-on-jason");
|
||||
const cur = row.review?.rounds.at(-1);
|
||||
if (cur?.request === "comment") requireApprovals(row, cur, to);
|
||||
} else if (from === "waiting-on-jason" && to === "done") {
|
||||
refuseUnresolved(row, "closing it");
|
||||
if (by === "sage") {
|
||||
if (evidence === null) throw refuse("sage closes a waiting-on-jason row only with --evidence citing Jason's approval");
|
||||
} else {
|
||||
@@ -520,11 +790,18 @@ function applyMove(rows, row, entry, resolved) {
|
||||
} else if (from === "in-review" && to === "done") {
|
||||
if (row.gateOwner === "jason") throw refuse(`row ${row.id}'s gate is Jason's; it goes through waiting-on-jason`);
|
||||
if (by !== row.gateOwner && !isPriv(by)) throw refuse(`only the gate owner (${row.gateOwner}) or a privileged actor may close row ${row.id}`);
|
||||
const ev = parseReviewEvidence(evidence);
|
||||
refuseUnresolved(row, "closing it");
|
||||
const cur = row.review?.rounds.at(-1);
|
||||
if (cur?.request === "comment") {
|
||||
// The reviewers' receipts are the evidence (8.9).
|
||||
if (evidence !== null) throw refuse(`row ${row.id} round ${cur.n} closes on its recorded verdicts; drop --evidence`);
|
||||
requireApprovals(row, cur, to);
|
||||
} else {
|
||||
const ev = parseReviewEvidence(evidence);
|
||||
if (!cur) throw refuse(`row ${row.id} has no review round to cite`);
|
||||
if (ev.round !== cur.n) throw refuse(`evidence names round ${ev.round}; row ${row.id} is in round ${cur.n}`);
|
||||
if (ev.candidate !== cur.candidate.digest) throw refuse(`evidence candidate ${ev.candidate} is not round ${cur.n}'s candidate ${cur.candidate.digest}`);
|
||||
}
|
||||
round = cur.n;
|
||||
next.claim = null;
|
||||
} else if ((from === "queued" || from === "briefed") && to === "parked") {
|
||||
@@ -536,7 +813,7 @@ function applyMove(rows, row, entry, resolved) {
|
||||
throw illegal();
|
||||
}
|
||||
next = touch(next, entry);
|
||||
return { row: next, result: { row: row.id, from, to, round, issue: revIssue, candidate: cand } };
|
||||
return { row: next, result: { row: row.id, from, to, round, issue: revIssue, candidate: cand }, request };
|
||||
}
|
||||
|
||||
function applySet(rows, row, entry, resolved) {
|
||||
@@ -637,7 +914,22 @@ export function applyEntry(state, entry, resolved) {
|
||||
const out = applyMove(rows, row, entry, resolved);
|
||||
rows.set(row.id, out.row);
|
||||
const r = out.result;
|
||||
result = { ...r, receipt: receipt(entry, rev, `row ${row.id} ${r.from}→${r.to}${r.round ? ` round ${r.round}` : ""}${r.issue ? ` on #${r.issue}` : ""}`) };
|
||||
result = { ...r, receipt: receipt(entry, rev, `row ${row.id} ${r.from}→${r.to}${r.round ? ` round ${r.round}` : ""}${r.issue ? ` on #${r.issue}` : ""}${out.request ?? ""}`) };
|
||||
break;
|
||||
}
|
||||
case "review-request":
|
||||
case "review-outcome":
|
||||
case "review-resolve":
|
||||
case "review-abandon":
|
||||
case "review-record": {
|
||||
if (entry.semantics < 2) throw refuse(`${entry.verb} needs semantics 2`);
|
||||
const row = getRow(rows, a.id);
|
||||
// review-outcome is the one entry a done row takes: a transport can
|
||||
// answer after the row closed (8.9).
|
||||
if (row.state === "done" && entry.verb !== "review-outcome") throw refuse(`row ${row.id} is done; done rows never change`);
|
||||
const out = applyReview(rows, row, entry);
|
||||
rows.set(row.id, touch(out.row, entry));
|
||||
result = { ...out.result, receipt: receipt(entry, rev, out.text) };
|
||||
break;
|
||||
}
|
||||
case "release": {
|
||||
@@ -795,11 +1087,16 @@ export function rowsArray(state) {
|
||||
function checkEntryShape(e, i) {
|
||||
keysExactly(e, ENTRY_KEYS, `log entry ${i}`);
|
||||
if (e.rev !== i) throw refuse(`log entry ${i} has rev ${e.rev}`);
|
||||
checkCallerOpId(e.op, `log entry ${i} op`);
|
||||
if (!VERBS.includes(e.verb)) throw refuse(`log entry ${i} verb ${JSON.stringify(e.verb)} is unknown`);
|
||||
if (e.verb === "review-outcome") {
|
||||
if (typeof e.op !== "string" || !LOG_OP_RE.test(e.op) || !isObj(e.args) || e.op !== `${e.args.attempt}.outcome`) throw refuse(`log entry ${i} op must be its attempt's op plus .outcome`);
|
||||
} else {
|
||||
checkCallerOpId(e.op, `log entry ${i} op`);
|
||||
}
|
||||
checkName(e.by, `log entry ${i} by`);
|
||||
checkTime(e.at, `log entry ${i} at`);
|
||||
if (e.semantics !== SEMANTICS) throw refuse(`log entry ${i} semantics ${e.semantics} is not ${SEMANTICS}`);
|
||||
if (!Number.isInteger(e.semantics) || e.semantics < 1 || e.semantics > SEMANTICS) throw refuse(`log entry ${i} semantics ${e.semantics} is not 1 to ${SEMANTICS}`);
|
||||
if (REVIEW_VERBS.includes(e.verb) && e.semantics < 2) throw refuse(`log entry ${i} ${e.verb} needs semantics 2`);
|
||||
if (typeof e.viewSha !== "string" || !SHA256_RE.test(e.viewSha)) throw refuse(`log entry ${i} viewSha is not a SHA-256`);
|
||||
}
|
||||
|
||||
@@ -867,7 +1164,9 @@ export function nextFor(rows, seat, briefMatches) {
|
||||
const byId = new Map(list.map((r) => [r.id, r]));
|
||||
const resume = list.find((r) => r.state === "in-progress" && r.claim?.seat === seat);
|
||||
if (resume) return { action: "resume", row: resume };
|
||||
const review = list.find((r) => r.state === "in-review" && r.reviewers.includes(seat) && r.owner !== seat);
|
||||
// A reviewer who recorded a verdict on the current round is done with it.
|
||||
const recorded = (r) => r.review.rounds.at(-1).receipts?.some((x) => x.reviewer === seat) ?? false;
|
||||
const review = list.find((r) => r.state === "in-review" && r.reviewers.includes(seat) && r.owner !== seat && !(r.review && recorded(r)));
|
||||
if (review) return { action: "review", row: review };
|
||||
const start = list.find((r) => r.state === "briefed" && r.owner === seat && afterSatisfied(byId, r).length === 0);
|
||||
if (start) return { action: "start", row: start, briefDiffers: !briefMatches(start) };
|
||||
@@ -885,7 +1184,11 @@ function cell(text) {
|
||||
function stateCell(r) {
|
||||
let s = r.state;
|
||||
if (r.state === "blocked") s = `blocked (from ${r.previousState}): ${r.blockedReason}`;
|
||||
else if (r.state === "in-review" && r.review) s = `in-review, round ${r.review.rounds.length}`;
|
||||
else if (r.state === "in-review" && r.review) {
|
||||
const cur = r.review.rounds.at(-1);
|
||||
s = `in-review, round ${cur.n}`;
|
||||
if (cur.request === "comment") s += `, request ${cur.attempts.at(-1).state}`;
|
||||
}
|
||||
return r.required ? `required; ${s}` : s;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
// Piece D (8.9): the request comment and its transport. The queue never
|
||||
// reads a token. It checks the acting seat's credential file with lstat
|
||||
// only, and `scripts/gitea-api.sh`, the one reader, sends the token to curl
|
||||
// through a config stream. Every call runs under a hard deadline.
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { lstatSync, realpathSync } from "node:fs";
|
||||
import { isAbsolute, join, resolve } from "node:path";
|
||||
import { FAILED_CODES } from "./queue.mjs";
|
||||
|
||||
export const REPO_API = "repos/mosaicstack/stack";
|
||||
export const DEFAULT_DEADLINE_MS = 30000;
|
||||
const LOGIN_RE = /^[a-z0-9][a-z0-9._-]{0,38}$/;
|
||||
const MAX_BODY = 60000;
|
||||
|
||||
// The Gitea account a seat posts as. The lead's is jarvis (lead decision 37).
|
||||
export function loginFor(actor) {
|
||||
return actor === "sage" ? "jarvis" : actor;
|
||||
}
|
||||
|
||||
// The acting seat's own token file, checked without opening it. Returns
|
||||
// null when it passes, else the reason.
|
||||
export function credCheck(env, actor) {
|
||||
const login = loginFor(actor);
|
||||
const p = env.MOSAIC_GITEA_CREDENTIAL_FILE;
|
||||
if (p === undefined || p === "") return `MOSAIC_GITEA_CREDENTIAL_FILE is not set; a request posts only with ${login}'s own token file`;
|
||||
if (!isAbsolute(p)) return "MOSAIC_GITEA_CREDENTIAL_FILE must be an absolute path";
|
||||
if (env.HOME && resolve(p) === resolve(env.HOME, "secrets/mosaic.gitea.json")) return "MOSAIC_GITEA_CREDENTIAL_FILE names the shared default file; a request posts only with the seat's own token file";
|
||||
const want = `/agents/${login}/secrets/gitea-mosaicstack-${login}.token`;
|
||||
if (!resolve(p).endsWith(want)) return `MOSAIC_GITEA_CREDENTIAL_FILE is not ${login}'s token file (…${want})`;
|
||||
let st;
|
||||
try {
|
||||
st = lstatSync(p);
|
||||
} catch {
|
||||
return `${login}'s token file does not exist`;
|
||||
}
|
||||
if (st.isSymbolicLink() || !st.isFile()) return `${login}'s token file must be a regular file, not a symlink`;
|
||||
// A linked directory must not lead to another seat's file.
|
||||
if (!realpathSync(p).endsWith(want)) return `MOSAIC_GITEA_CREDENTIAL_FILE resolves outside ${login}'s secrets directory`;
|
||||
if (typeof process.getuid === "function" && st.uid !== process.getuid()) return `${login}'s token file is not owned by this user`;
|
||||
if ((st.mode & 0o777) !== 0o600) return `${login}'s token file must be mode 0600`;
|
||||
return null;
|
||||
}
|
||||
|
||||
// The two markers `review resolve` checks in a comment it is shown.
|
||||
export function markers(op, row, round, digest) {
|
||||
return [`<!-- mosaic-queue-op: ${op} -->`, `<!-- mosaic-queue-round: row=${row} round=${round} candidate=${digest} -->`];
|
||||
}
|
||||
|
||||
function fence(text) {
|
||||
const longest = Math.max(0, ...(text.match(/`+/g) ?? []).map((s) => s.length));
|
||||
return "`".repeat(Math.max(3, longest + 1));
|
||||
}
|
||||
|
||||
// The request comment for one attempt.
|
||||
export function requestBody(row, round, op) {
|
||||
const c = round.candidate;
|
||||
const lines = [
|
||||
...markers(op, row.id, round.n, c.digest),
|
||||
"",
|
||||
`Review request for queue row ${row.id}, round ${round.n}: ${row.piece}`,
|
||||
"",
|
||||
`- Owner: ${row.owner}`,
|
||||
`- Reviewers: ${row.reviewers.join(", ")}`,
|
||||
`- Gate: ${row.gate} (${row.gateOwner})`,
|
||||
`- Brief: ${row.brief ? `\`${row.brief.path}\` § ${row.brief.anchor} @${row.brief.blob.slice(0, 12)}` : "none"}`,
|
||||
`- Candidate: ${c.kind} \`${c.digest}\``,
|
||||
"",
|
||||
];
|
||||
if (c.kind === "manifest") {
|
||||
const f = fence(c.text);
|
||||
lines.push("The manifest:", "", `${f}text`, c.text.replace(/\n$/, ""), f, "");
|
||||
lines.push(`Check a tree against it with \`scripts/mosaic queue review verify-commit ${row.id} REF\`.`, "");
|
||||
} else {
|
||||
lines.push(`Check a prospective commit against it with \`scripts/mosaic queue review verify-commit ${row.id} REF\`.`, "");
|
||||
}
|
||||
lines.push(
|
||||
"Post your verdict as a comment here, then record it:",
|
||||
"",
|
||||
"```",
|
||||
`scripts/mosaic queue review record ${row.id} --verdict approve|changes --comment COMMENT_ID --candidate ${c.digest} --op OP --by SEAT`,
|
||||
"```",
|
||||
);
|
||||
return `${lines.join("\n")}\n`;
|
||||
}
|
||||
|
||||
export function bodyTooLong(body) {
|
||||
return Buffer.byteLength(body) > MAX_BODY;
|
||||
}
|
||||
|
||||
// One helper call under the deadline. `timeout -s KILL` kills the helper's
|
||||
// whole process group, curl included, so nothing runs on after it.
|
||||
export function callTool(ctx, top, args) {
|
||||
const tool = join(top, "scripts/gitea-api.sh");
|
||||
const secs = String(ctx.deadlineMs / 1000);
|
||||
const r = spawnSync("timeout", ["-s", "KILL", secs, tool, ...args], { cwd: top, env: ctx.env, encoding: "utf8", maxBuffer: 16 << 20 });
|
||||
const m = /^HTTP (\d{3})$/m.exec(r.stderr ?? "");
|
||||
return {
|
||||
spawnFailed: r.error?.code === "ENOENT",
|
||||
killed: r.signal === "SIGKILL" || r.status === 137,
|
||||
error: r.error ? true : false,
|
||||
exit: r.status,
|
||||
http: m ? Number(m[1]) : null,
|
||||
requestFailed: /^gitea-api: request failed$/m.test(r.stderr ?? ""),
|
||||
stdout: r.stdout ?? "",
|
||||
};
|
||||
}
|
||||
|
||||
function jsonOrNull(text) {
|
||||
try { return JSON.parse(text); } catch { return null; }
|
||||
}
|
||||
|
||||
// The POST's outcome. Details are fixed text: nothing from the response is
|
||||
// recorded or echoed.
|
||||
export function classifyPost(r) {
|
||||
const out = (outcome, status, comment, detail) => ({ outcome, status, comment, detail });
|
||||
if (r.spawnFailed) return out("failed", null, null, "pre-send: the timeout command could not run");
|
||||
if (r.killed) return out("uncertain", null, null, "no answer before the deadline");
|
||||
if (r.error) return out("uncertain", r.http, null, "the helper call failed");
|
||||
if (r.http === 201) {
|
||||
const j = jsonOrNull(r.stdout);
|
||||
const id = j && typeof j === "object" ? j.id : undefined;
|
||||
if (Number.isSafeInteger(id) && id > 0) return out("posted", 201, id, "created");
|
||||
return out("uncertain", 201, null, "HTTP 201 without a comment id");
|
||||
}
|
||||
if (r.http !== null && FAILED_CODES.includes(r.http)) return out("failed", r.http, null, `refused with HTTP ${r.http}`);
|
||||
if (r.http !== null) return out("uncertain", r.http, null, `unexpected HTTP ${r.http}`);
|
||||
if (r.requestFailed) return out("uncertain", null, null, "the request failed in transit");
|
||||
return out("uncertain", null, null, "no HTTP status came back");
|
||||
}
|
||||
|
||||
// GET user: the token must belong to the acting seat's login. Returns null
|
||||
// or the reason, which is safe to print.
|
||||
export function checkUser(r, login) {
|
||||
if (r.spawnFailed) return "the timeout command could not run";
|
||||
if (r.killed) return "GET user had no answer before the deadline";
|
||||
if (r.error || r.http === null) return "GET user failed";
|
||||
if (r.http !== 200) return `GET user answered HTTP ${r.http}`;
|
||||
const j = jsonOrNull(r.stdout);
|
||||
const got = j && typeof j === "object" ? j.login : undefined;
|
||||
if (got === login) return null;
|
||||
const shown = typeof got === "string" && LOGIN_RE.test(got) ? got : "an unexpected value";
|
||||
return `the token belongs to ${shown}, not ${login}`;
|
||||
}
|
||||
|
||||
// GET issues/comments/ID for `review resolve`: the comment must be on the
|
||||
// round's issue and carry both of the attempt's markers.
|
||||
export function checkComment(r, { id, issue, op, row, round, digest, author }) {
|
||||
if (r.spawnFailed || r.killed || r.error || r.http === null) return { code: 1, reason: `GET comment ${id} failed or had no answer before the deadline` };
|
||||
if (r.http === 404) return { code: 2, reason: `comment ${id} does not exist` };
|
||||
if (r.http !== 200) return { code: 1, reason: `GET comment ${id} answered HTTP ${r.http}` };
|
||||
const j = jsonOrNull(r.stdout);
|
||||
if (!j || typeof j !== "object") return { code: 1, reason: `GET comment ${id} did not answer JSON` };
|
||||
const wrong = [];
|
||||
if (j.id !== id) wrong.push("its id");
|
||||
if (!j.user || j.user.login !== author) wrong.push(`its author (want ${author})`);
|
||||
if (typeof j.issue_url !== "string" || !j.issue_url.endsWith(`/issues/${issue}`)) wrong.push(`the issue (want #${issue})`);
|
||||
const body = typeof j.body === "string" ? j.body.split("\n") : [];
|
||||
const [mOp, mRound] = markers(op, row, round, digest);
|
||||
if (!body.includes(mOp)) wrong.push(`the op marker for ${op}`);
|
||||
if (!body.includes(mRound)) wrong.push(`the round marker (row ${row} round ${round} candidate ${digest})`);
|
||||
return wrong.length ? { code: 2, reason: `comment ${id} does not match request ${op}: ${wrong.join(", ")}` } : null;
|
||||
}
|
||||
@@ -11,10 +11,15 @@ import { QueueError } from "./errors.mjs";
|
||||
import { checkPlatform, errno, fsyncFile, lstatOrNull, readOrNull, realIo, unlinkQuiet, writeTemp } from "./io.mjs";
|
||||
import { acquire, checkGate, realProc, releaseOrWarn, unlock as unlockLock } from "./lock.mjs";
|
||||
import {
|
||||
PRIVILEGED, SEMANTICS, VERSION, applyEntry, buildDoc, canonArgs, checkCallerOpId, checkName, classifyView, countHeading,
|
||||
describeUnshown, genesisReceipt, genesisRows, gitBlobId, loadDoc, logDigest, nextFor, parseBriefSpec, parseManifest,
|
||||
parseMigrationMap, render, rowsArray, sameJson, serialize, sha256, splitView,
|
||||
LOG_OP_RE, PRIVILEGED, SEMANTICS, UNRESOLVED_STATES, VERSION, applyEntry, attemptOf, buildDoc, canonArgs, checkCallerOpId,
|
||||
checkName, classifyView, countHeading, describeUnshown, genesisReceipt, genesisRows, gitBlobId, loadDoc, logDigest,
|
||||
manifestEntries, nextFor, parseBriefSpec, parseManifest, parseMigrationMap, render, rowsArray, sameJson, serialize, sha256,
|
||||
splitView,
|
||||
} from "./queue.mjs";
|
||||
import {
|
||||
DEFAULT_DEADLINE_MS, REPO_API, bodyTooLong, callTool, checkComment, checkUser, classifyPost, credCheck, loginFor, markers,
|
||||
requestBody,
|
||||
} from "./review.mjs";
|
||||
|
||||
export const QUEUE_REL = "docs/plans/queue.json";
|
||||
export const VIEW_REL = "docs/plans/QUEUE.md";
|
||||
@@ -37,6 +42,7 @@ function makeCtx(opts = {}) {
|
||||
readOrder: opts.readOrder ?? "witness-first",
|
||||
lockWaitMs: opts.lockWaitMs ?? 10000,
|
||||
lockStepMs: opts.lockStepMs ?? 100,
|
||||
deadlineMs: opts.deadlineMs ?? DEFAULT_DEADLINE_MS,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -458,13 +464,17 @@ function resolveFor(ctx, loc, cur, verb, args, cmp) {
|
||||
return {};
|
||||
}
|
||||
|
||||
// Every logged verb except genesis. `yes` is accept-history's confirmation;
|
||||
// it is not part of the op's identity.
|
||||
// Every logged verb except genesis and review-outcome, which only a
|
||||
// request writes. `yes` confirms accept-history and review-abandon; it is
|
||||
// not part of the op's identity.
|
||||
export function mutate(opts, { verb, op, args, by, yes = false }) {
|
||||
const ctx = makeCtx(opts);
|
||||
const mismatch = actorMismatch(ctx, by);
|
||||
try {
|
||||
const res = mutateAs(ctx, { verb, op, args, by, yes });
|
||||
if (verb === "review-outcome") throw refuse("a request records its own outcome; resolve or abandon the attempt instead");
|
||||
if (verb === "review-resolve") checkResolve(ctx, { op, args, by });
|
||||
let res = mutateAs(ctx, { verb, op, args, by, yes });
|
||||
res = verb === "move" || verb === "review-request" ? requestStep(ctx, res) : strip(res);
|
||||
if (mismatch) res.err.unshift(mismatch);
|
||||
return res;
|
||||
} catch (err) {
|
||||
@@ -473,8 +483,13 @@ export function mutate(opts, { verb, op, args, by, yes = false }) {
|
||||
}
|
||||
}
|
||||
|
||||
function mutateAs(ctx, { verb, op, args, by, yes }) {
|
||||
// `derived` is the outcome write: its op is the attempt's op plus .outcome.
|
||||
function mutateAs(ctx, { verb, op, args, by, yes = false, derived = false }) {
|
||||
if (derived) {
|
||||
if (!LOG_OP_RE.test(op) || op !== `${args.attempt}.outcome`) throw refuse(`outcome op ${op} is not its attempt's op plus .outcome`);
|
||||
} else {
|
||||
checkCallerOp(op);
|
||||
}
|
||||
const actor = actorOf(ctx, by);
|
||||
const cargs = canonArgs(verb, args);
|
||||
if (verb === "genesis") return genesis(ctx, op, cargs, actor);
|
||||
@@ -495,8 +510,12 @@ function mutateAs(ctx, { verb, op, args, by, yes }) {
|
||||
if (view.state === "stale") res.err.push(`warning: ${staleMessage(view, cur.doc.log)}`);
|
||||
if (view.state === "unknown") res.err.push(`warning: ${unknownMessage(view)}`);
|
||||
res.out.push(`${prior.result.receipt} (already recorded at rev ${prior.rev})`);
|
||||
Object.assign(res, { fresh: false, entry: prior, rows: cur.state.rows, top: loc.top });
|
||||
return;
|
||||
}
|
||||
if (verb === "review-abandon" && !yes) {
|
||||
throw refuse("abandoning a request means a request comment may exist twice on the issue; re-run with --yes to record that");
|
||||
}
|
||||
if (verb === "accept-history") {
|
||||
if (cmp.state !== "lost" && cmp.state !== "absent") throw refuse("history is not lost and the witness is present; accept-history has nothing to accept");
|
||||
const range = `${lostMessage(cmp, cur.doc)}. ops in that range are no longer deduplicated`;
|
||||
@@ -527,9 +546,159 @@ function mutateAs(ctx, { verb, op, args, by, yes }) {
|
||||
const warn = commitWrite(ctx, loc, cur, doc, bytes, op, { bytes: viewRead.bytes, parts: view.parts }, body, false);
|
||||
if (warn) res.err.push(`warning: ${warn}`);
|
||||
res.out.push(entry.result.receipt);
|
||||
Object.assign(res, { fresh: true, entry, rows: applied.state.rows, top: loc.top });
|
||||
});
|
||||
}
|
||||
|
||||
// --- the request comment (8.9) ---
|
||||
|
||||
const STATE_CODE = { posted: 0, failed: 1, abandoned: 1 };
|
||||
|
||||
function stateCode(state) {
|
||||
return STATE_CODE[state] ?? 3;
|
||||
}
|
||||
|
||||
function strip(res) {
|
||||
return { out: res.out, err: res.err, code: res.code };
|
||||
}
|
||||
|
||||
function retryHint(row, round, attempt) {
|
||||
const where = `row ${row.id} round ${round.n} on #${round.issue}`;
|
||||
if (attempt.state === "posted") return `request ${attempt.op} is posted (${where}), comment ${attempt.comment}; nothing was sent again`;
|
||||
if (!UNRESOLVED_STATES.includes(attempt.state)) return `request ${attempt.op} is ${attempt.state} (${where}); nothing was sent again`;
|
||||
return `request ${attempt.op} is ${attempt.state} (${where}); nothing was sent again.\n${settleHint(row, round, attempt)}`;
|
||||
}
|
||||
|
||||
// What to do about a request that may or may not be on the issue.
|
||||
function settleHint(row, round, attempt) {
|
||||
return [
|
||||
`Look on #${round.issue} for a comment carrying ${markers(attempt.op, row.id, round.n, round.candidate.digest)[0]}.`,
|
||||
`If it is there: ${FIX} review resolve ${row.id} ${attempt.op} --comment ID --op OP. If not: a privileged actor runs ${FIX} review abandon ${row.id} ${attempt.op} --reason TEXT --yes --op OP.`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
// After a move or review-request is logged: post the request comment once,
|
||||
// then log what the transport said. A retried op never posts again.
|
||||
function requestStep(ctx, res) {
|
||||
const e = res.entry;
|
||||
const row = res.rows.get(e.args.id);
|
||||
const found = row ? attemptOf(row, e.op) : null;
|
||||
if (!found) return strip(res);
|
||||
const { round, attempt } = found;
|
||||
if (!res.fresh) {
|
||||
res.err.push(retryHint(row, round, attempt));
|
||||
return { ...strip(res), code: stateCode(attempt.state) };
|
||||
}
|
||||
const actor = e.by;
|
||||
ctx.hook("pre-send");
|
||||
const body = requestBody(row, round, e.op);
|
||||
let why = credCheck(ctx.env, actor);
|
||||
if (why === null) why = checkUser(callTool(ctx, res.top, ["GET", "user"]), loginFor(actor));
|
||||
let t;
|
||||
if (why !== null) {
|
||||
res.err.push(`request ${e.op} not sent: ${why}`);
|
||||
t = { outcome: "failed", status: null, comment: null, detail: "pre-send: the credential or account check failed" };
|
||||
} else if (bodyTooLong(body)) {
|
||||
res.err.push(`request ${e.op} not sent: the comment would be longer than the limit`);
|
||||
t = { outcome: "failed", status: null, comment: null, detail: "pre-send: the request comment is too long" };
|
||||
} else {
|
||||
const r = callTool(ctx, res.top, ["POST", `${REPO_API}/issues/${round.issue}/comments`, JSON.stringify({ body })]);
|
||||
ctx.hook("posted");
|
||||
t = classifyPost(r);
|
||||
}
|
||||
ctx.hook("outcome");
|
||||
let out;
|
||||
try {
|
||||
out = mutateAs(ctx, { verb: "review-outcome", op: `${e.op}.outcome`, args: { id: row.id, attempt: e.op, ...t }, by: actor, derived: true });
|
||||
} catch (err) {
|
||||
const said = t.outcome === "posted" ? `posted comment ${t.comment}` : `${t.outcome} (${t.detail})`;
|
||||
throw new QueueError(`${[...res.out, ...res.err].join("\n")}\nuncertain ${e.op}: the transport said ${said}; that outcome is not recorded: ${err.message}`, 3);
|
||||
}
|
||||
const now = attemptOf(out.rows.get(row.id), e.op);
|
||||
const err = [...res.err, ...out.err];
|
||||
if (UNRESOLVED_STATES.includes(now.attempt.state)) err.push(settleHint(out.rows.get(row.id), now.round, now.attempt));
|
||||
return { out: [...res.out, ...out.out], err, code: stateCode(now.attempt.state) };
|
||||
}
|
||||
|
||||
// review resolve checks the comment it names before logging anything: on
|
||||
// the round's issue, by the requester's account, with both markers.
|
||||
function checkResolve(ctx, { op, args, by }) {
|
||||
checkCallerOp(op);
|
||||
const actor = actorOf(ctx, by);
|
||||
const a = canonArgs("review-resolve", args);
|
||||
const st = readStateWith(ctx);
|
||||
if (st.doc.log.some((e) => e.op === op)) return;
|
||||
const row = st.state.rows.get(a.id);
|
||||
const found = row ? attemptOf(row, a.attempt) : null;
|
||||
// Anything the log would refuse anyway is refused there, with no request.
|
||||
if (!found || !UNRESOLVED_STATES.includes(found.attempt.state) || row.state === "done") return;
|
||||
if (actor !== row.owner && !PRIVILEGED.has(actor)) return;
|
||||
const why = credCheck(ctx.env, actor);
|
||||
if (why) throw refuse(`cannot check comment ${a.comment}: ${why}`);
|
||||
const r = callTool(ctx, st.loc.top, ["GET", `${REPO_API}/issues/comments/${a.comment}`]);
|
||||
const { round } = found;
|
||||
const bad = checkComment(r, {
|
||||
id: a.comment, issue: round.issue, op: a.attempt, row: row.id, round: round.n, digest: round.candidate.digest, author: loginFor(found.attempt.by),
|
||||
});
|
||||
if (bad) throw new QueueError(bad.reason, bad.code);
|
||||
}
|
||||
|
||||
// review verify-commit: does REF's tree hold exactly the candidate?
|
||||
export function verifyCommit(opts, id, ref) {
|
||||
const st = readState(opts);
|
||||
const { ctx, loc } = st;
|
||||
const row = st.state.rows.get(id);
|
||||
if (!row) throw refuse(`no row ${id}`);
|
||||
const cur = row.review?.rounds.at(-1);
|
||||
if (!cur) throw refuse(`row ${id} has no review round`);
|
||||
const tree = git(ctx, loc.top, ["rev-parse", "--verify", "--quiet", "--end-of-options", `${ref}^{tree}`], { allowFail: true });
|
||||
if (tree === null) throw refuse(`${ref} is not a commit or tree here`);
|
||||
const have = lsTree(ctx, loc.top, tree.toString().trim());
|
||||
const c = cur.candidate;
|
||||
const bad = [];
|
||||
let count = 0;
|
||||
if (c.kind === "manifest") {
|
||||
for (const { digest, path } of manifestEntries(c.text)) {
|
||||
count++;
|
||||
const h = have.get(path);
|
||||
if (!h || h.type !== "blob") bad.push(`${path}: missing`);
|
||||
else if (sha256(git(ctx, loc.top, ["cat-file", "blob", h.oid])) !== digest) bad.push(`${path}: content differs`);
|
||||
}
|
||||
} else {
|
||||
if (git(ctx, loc.top, ["cat-file", "-e", `${c.digest}^{commit}`], { allowFail: true }) === null) {
|
||||
throw refuse(`candidate commit ${c.digest} is no longer in this repository; a commit candidate stays checkable only while a ref keeps it`);
|
||||
}
|
||||
const parent = git(ctx, loc.top, ["rev-parse", "--verify", "--quiet", `${c.digest}^1`], { allowFail: true });
|
||||
const base = parent === null ? git(ctx, loc.top, ["hash-object", "-t", "tree", "/dev/null"]).toString().trim() : parent.toString().trim();
|
||||
const raw = git(ctx, loc.top, ["diff-tree", "-r", "--no-renames", "-z", base, c.digest]).toString().split("\0");
|
||||
for (let i = 0; i + 1 < raw.length; i += 2) {
|
||||
const m = /^:(\d{6}) (\d{6}) ([0-9a-f]{40}) ([0-9a-f]{40}) ([A-Z])/.exec(raw[i]);
|
||||
if (!m) throw new QueueError(`git diff-tree printed a line this check does not read: ${JSON.stringify(raw[i].slice(0, 80))}`, 1);
|
||||
const path = raw[i + 1];
|
||||
count++;
|
||||
const h = have.get(path);
|
||||
if (m[5] === "D") {
|
||||
if (h) bad.push(`${path}: deleted in the candidate, present here`);
|
||||
} else if (!h) {
|
||||
bad.push(`${path}: missing`);
|
||||
} else if (h.oid !== m[4] || h.mode !== m[2]) {
|
||||
bad.push(`${path}: ${h.oid !== m[4] ? "content" : "mode"} differs`);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (bad.length) throw refuse(`${ref} does not match row ${id} round ${cur.n}'s candidate:\n${bad.join("\n")}`);
|
||||
return { out: [`ok row ${id} round ${cur.n}: ${ref} matches the ${c.kind} candidate (${count} paths)`], err: st.notes, code: 0 };
|
||||
}
|
||||
|
||||
function lsTree(ctx, top, tree) {
|
||||
const map = new Map();
|
||||
for (const e of git(ctx, top, ["ls-tree", "-r", "-z", "--full-tree", tree]).toString().split("\0")) {
|
||||
const m = /^(\d{6}) (\w+) ([0-9a-f]{40})\t(.*)$/s.exec(e);
|
||||
if (m) map.set(m[4], { mode: m[1], type: m[2], oid: m[3] });
|
||||
}
|
||||
return map;
|
||||
}
|
||||
|
||||
// log[0] (8.2). Runs before canonicalRoot exists; its arguments are checked
|
||||
// against this checkout instead.
|
||||
function genesis(ctx, op, args, actor) {
|
||||
@@ -627,7 +796,10 @@ function viewNotes(ctx, loc, log) {
|
||||
}
|
||||
|
||||
function readState(opts) {
|
||||
const ctx = makeCtx(opts);
|
||||
return readStateWith(makeCtx(opts));
|
||||
}
|
||||
|
||||
function readStateWith(ctx) {
|
||||
const loc = locate(ctx);
|
||||
const r = readUnlocked(ctx, loc);
|
||||
return { ctx, loc, ...r, notes: [...r.notes, ...viewNotes(ctx, loc, r.doc.log)] };
|
||||
|
||||
+52
@@ -0,0 +1,52 @@
|
||||
// A stand-in for scripts/gitea-api.sh in the review tests: same argv
|
||||
// (METHOD PATH [BODY]), same output (body on stdout, "HTTP <code>" on
|
||||
// stderr, exit 1 on a non-2xx code). It reads no credential file; the login
|
||||
// comes from the credential path's agents/<login>/ segment. Every call is
|
||||
// appended to $FAKE_GITEA_LOG, and posted comments are served back from it.
|
||||
// $FAKE_GITEA_SCENARIO names a JSON file of rules that override the
|
||||
// defaults: {rules: [{method, path (regex), http, body, sleepMs, fail}]}.
|
||||
import { appendFileSync, existsSync, readFileSync } from "node:fs";
|
||||
|
||||
const [method, path, body] = process.argv.slice(2);
|
||||
const env = process.env;
|
||||
const read = (f) => (f && existsSync(f) ? readFileSync(f, "utf8") : null);
|
||||
const scenario = JSON.parse(read(env.FAKE_GITEA_SCENARIO) ?? "{}");
|
||||
const prior = (read(env.FAKE_GITEA_LOG) ?? "").split("\n").filter(Boolean).map((l) => JSON.parse(l));
|
||||
const cred = env.MOSAIC_GITEA_CREDENTIAL_FILE ?? "";
|
||||
const loginOf = (p) => /\/agents\/([^/]+)\/secrets\//.exec(p)?.[1] ?? null;
|
||||
const login = loginOf(cred);
|
||||
appendFileSync(env.FAKE_GITEA_LOG, `${JSON.stringify({ method, path, body: body === undefined ? null : JSON.parse(body), cred })}\n`);
|
||||
|
||||
function answer(http, obj) {
|
||||
if (obj !== undefined) process.stdout.write(typeof obj === "string" ? obj : JSON.stringify(obj));
|
||||
process.stderr.write(`HTTP ${http}\n`);
|
||||
if (http < 200 || http > 299) {
|
||||
process.stderr.write(`gitea-api: ${method} ${path} failed (HTTP ${http})\n`);
|
||||
process.exit(1);
|
||||
}
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const issueUrl = (n) => `https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/${n}`;
|
||||
const rule = (scenario.rules ?? []).find((r) => r.method === method && new RegExp(r.path).test(path));
|
||||
if (rule) {
|
||||
if (rule.sleepMs) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, rule.sleepMs);
|
||||
if (rule.fail) {
|
||||
process.stderr.write("gitea-api: request failed\n");
|
||||
process.exit(1);
|
||||
}
|
||||
if (rule.http) answer(rule.http, rule.body);
|
||||
}
|
||||
let m;
|
||||
if (method === "GET" && path === "user") answer(200, { login });
|
||||
if (method === "POST" && (m = /^repos\/mosaicstack\/stack\/issues\/(\d+)\/comments$/.exec(path))) {
|
||||
const id = 1000 + prior.filter((p) => p.method === "POST").length;
|
||||
answer(201, { id, issue_url: issueUrl(m[1]), body: JSON.parse(body).body, user: { login } });
|
||||
}
|
||||
if (method === "GET" && (m = /^repos\/mosaicstack\/stack\/issues\/comments\/(\d+)$/.exec(path))) {
|
||||
const id = Number(m[1]);
|
||||
const post = prior.filter((p) => p.method === "POST")[id - 1000];
|
||||
if (post) answer(200, { id, issue_url: issueUrl(/issues\/(\d+)\/comments$/.exec(post.path)[1]), body: post.body.body, user: { login: loginOf(post.cred) } });
|
||||
answer(404, { message: "not found" });
|
||||
}
|
||||
answer(404, { message: "no route" });
|
||||
+6
-3
@@ -1,10 +1,13 @@
|
||||
// Child process for the SIGKILL tests. argv: SRC_DIR STEP REQUEST_JSON.
|
||||
// Runs one mutation from the working directory and kills itself with
|
||||
// SIGKILL when the write path reaches STEP (8.5).
|
||||
// SIGKILL when the write path reaches STEP (8.5). STEP#N kills at the Nth
|
||||
// time the step is reached: a request's outcome write is its second lock.
|
||||
import { join } from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
|
||||
const [src, step, json] = process.argv.slice(2);
|
||||
const [src, spec, json] = process.argv.slice(2);
|
||||
const [step, nth = "1"] = spec.split("#");
|
||||
let seen = 0;
|
||||
const store = await import(pathToFileURL(join(src, "store.mjs")).href);
|
||||
store.mutate({ hook: (name) => { if (name === step) process.kill(process.pid, "SIGKILL"); } }, JSON.parse(json));
|
||||
store.mutate({ hook: (name) => { if (name === step && ++seen === Number(nth)) process.kill(process.pid, "SIGKILL"); } }, JSON.parse(json));
|
||||
process.stdout.write("finished without reaching the step\n");
|
||||
|
||||
@@ -0,0 +1,664 @@
|
||||
// Piece D (8.9): the request comment, its outcome, resolve and abandon,
|
||||
// recorded verdicts and verify-commit. Every test posts to
|
||||
// fixtures/fake-gitea.mjs, installed as the scratch repository's
|
||||
// scripts/gitea-api.sh. The token files are dummies; nothing reads them.
|
||||
import assert from "node:assert/strict";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { chmodSync, existsSync, mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { test } from "node:test";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { cli, genesisCommitted, load, opts, scratchRepo } from "./helpers.mjs";
|
||||
|
||||
const HERE = new URL(".", import.meta.url).pathname;
|
||||
const FAKE = join(HERE, "fixtures", "fake-gitea.mjs");
|
||||
const LOGINS = ["darkwing", "dewey", "filbert", "rocko", "jarvis", "sage"];
|
||||
|
||||
async function ready(t, { start = true } = {}) {
|
||||
const repo = scratchRepo(t);
|
||||
genesisCommitted(repo);
|
||||
mkdirSync(join(repo.root, "scripts"), { recursive: true });
|
||||
writeFileSync(join(repo.root, "scripts/gitea-api.sh"), `#!/bin/sh\nexec "${process.execPath}" "${FAKE}" "$@"\n`, { mode: 0o755 });
|
||||
const logFile = join(repo.base, "gitea.log");
|
||||
const scenarioFile = join(repo.base, "scenario.json");
|
||||
repo.env = { ...repo.env, FAKE_GITEA_LOG: logFile, FAKE_GITEA_SCENARIO: scenarioFile };
|
||||
const tokens = {};
|
||||
for (const login of LOGINS) {
|
||||
const dir = join(repo.base, "fleet/agents", login, "secrets");
|
||||
mkdirSync(dir, { recursive: true });
|
||||
tokens[login] = join(dir, `gitea-mosaicstack-${login}.token`);
|
||||
writeFileSync(tokens[login], `${"0".repeat(40)}\n`, { mode: 0o600 });
|
||||
}
|
||||
const m = await load(repo);
|
||||
const cred = (seat) => ({ MOSAIC_GITEA_CREDENTIAL_FILE: tokens[seat === "sage" ? "jarvis" : seat] });
|
||||
const s = {
|
||||
repo, m, tokens, cred,
|
||||
run: (args, by, env = {}) => cli(repo, args, { by, env: { ...cred(by), ...env } }),
|
||||
mut: (req, extra = {}) => m.store.mutate(opts(repo, { ...extra, env: { ...repo.env, ...cred(req.by), ...(extra.env ?? {}) } }), req),
|
||||
calls: () => (existsSync(logFile) ? readFileSync(logFile, "utf8").split("\n").filter(Boolean).map((l) => JSON.parse(l)) : []),
|
||||
posts: () => s.calls().filter((c) => c.method === "POST"),
|
||||
rules: (rules) => writeFileSync(scenarioFile, JSON.stringify({ rules })),
|
||||
doc: () => JSON.parse(readFileSync(repo.queuePath, "utf8")),
|
||||
row: (id) => s.doc().rows.find((r) => r.id === id),
|
||||
round: (id, n) => s.row(id).review.rounds[n - 1],
|
||||
head: () => repo.g("rev-parse", "HEAD").trim(),
|
||||
};
|
||||
if (start) {
|
||||
ok(s.run(["move", "6", "blocked", "--reason", "paused", "--op", "block-6-00001"], "darkwing"));
|
||||
ok(s.run(["move", "9", "in-progress", "--op", "start-9-00001"], "darkwing"));
|
||||
}
|
||||
return s;
|
||||
}
|
||||
|
||||
function ok(r, re) {
|
||||
assert.equal(r.code, 0, `exit ${r.code}: ${r.out}${r.err}`);
|
||||
if (re) assert.match(r.out, re);
|
||||
return r;
|
||||
}
|
||||
|
||||
function no(r, code, re) {
|
||||
assert.equal(r.code, code, `expected exit ${code}, got ${r.code}: ${r.out}${r.err}`);
|
||||
if (re) assert.match(r.err, re);
|
||||
return r;
|
||||
}
|
||||
|
||||
function throwsCode(fn, code, re) {
|
||||
assert.throws(fn, (err) => {
|
||||
assert.equal(err.code, code, err.message);
|
||||
if (re) assert.match(err.message, re);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
const move9 = (op = "review-9-00001") => ["move", "9", "in-review", "--candidate", "HEAD", "--op", op];
|
||||
const request = (op, id = 9) => ["review", "request", String(id), "--op", op];
|
||||
|
||||
// A commit on refs/heads/NAME: PARENT plus CHANGES ({path: text or null}),
|
||||
// built in a private index so the working tree never moves.
|
||||
function commitOn(repo, name, parent, changes) {
|
||||
const env = { ...repo.env, GIT_INDEX_FILE: join(repo.base, `index-${name}`) };
|
||||
const g = (args, input) => {
|
||||
const r = spawnSync("git", ["-C", repo.root, ...args], { env, input, encoding: "utf8" });
|
||||
if (r.status !== 0) throw new Error(`git ${args.join(" ")}: ${r.stderr}`);
|
||||
return r.stdout.trim();
|
||||
};
|
||||
g(["read-tree", parent]);
|
||||
for (const [path, text] of Object.entries(changes)) {
|
||||
if (text === null) g(["update-index", "--force-remove", path]);
|
||||
else g(["update-index", "--add", "--cacheinfo", `100644,${g(["hash-object", "-w", "--stdin"], text)},${path}`]);
|
||||
}
|
||||
const c = g(["commit-tree", g(["write-tree"]), "-p", parent, "-m", name]);
|
||||
g(["update-ref", `refs/heads/${name}`, c]);
|
||||
return c;
|
||||
}
|
||||
|
||||
test("a request posts once as the requester; a retry sends nothing", async (t) => {
|
||||
const s = await ready(t);
|
||||
const head = s.head();
|
||||
const r = ok(s.run(move9(), "darkwing"), /^ok review-9-00001 rev 3 row 9 in-progress→in-review round 1 on #1508; request review-9-00001 requesting$/m);
|
||||
assert.match(r.out, /^ok review-9-00001\.outcome rev 4 row 9 round 1 request review-9-00001 posted comment 1000$/m);
|
||||
const calls = s.calls();
|
||||
assert.deepEqual(calls.map((c) => `${c.method} ${c.path}`), ["GET user", "POST repos/mosaicstack/stack/issues/1508/comments"]);
|
||||
assert.ok(calls.every((c) => c.cred === s.tokens.darkwing));
|
||||
const body = calls[1].body.body;
|
||||
assert.ok(body.startsWith(`<!-- mosaic-queue-op: review-9-00001 -->\n<!-- mosaic-queue-round: row=9 round=1 candidate=${head} -->\n`), body);
|
||||
assert.match(body, new RegExp(`queue review record 9 --verdict approve\\|changes --comment COMMENT_ID --candidate ${head}`));
|
||||
assert.match(body, /- Reviewers: filbert\n/);
|
||||
const round = s.round(9, 1);
|
||||
assert.deepEqual(Object.keys(round), ["n", "op", "by", "at", "issue", "candidate", "request", "attempts", "duplicateRisk", "receipts"]);
|
||||
assert.equal(round.request, "comment");
|
||||
assert.deepEqual(round.attempts.map((a) => [a.op, a.by, a.state, a.comment, a.transport]), [
|
||||
["review-9-00001", "darkwing", "posted", 1000, { outcome: "posted", status: 201, comment: 1000, detail: "created" }],
|
||||
]);
|
||||
assert.equal(s.doc().log.at(-1).op, "review-9-00001.outcome");
|
||||
assert.match(readFileSync(s.repo.viewPath, "utf8"), /; in-review, round 1, request posted \|/);
|
||||
const again = ok(s.run(move9(), "darkwing"), /\(already recorded at rev 3\)$/m);
|
||||
assert.match(again.err, /request review-9-00001 is posted \(row 9 round 1 on #1508\), comment 1000; nothing was sent again/);
|
||||
assert.equal(s.calls().length, 2);
|
||||
no(s.run(request("request-9-0002"), "darkwing"), 2, /round 1 already has a posted request \(review-9-00001, comment 1000\)/);
|
||||
no(s.run(["review", "outcome", "9"], "darkwing"), 4, /review takes request, resolve, abandon, record or verify-commit/);
|
||||
throwsCode(() => s.mut({ verb: "review-outcome", op: "review-9-00001.outcome", args: { id: 9, attempt: "review-9-00001", outcome: "failed", status: 403, comment: null, detail: "x" }, by: "darkwing" }), 2, /records its own outcome/);
|
||||
});
|
||||
|
||||
test("each transport answer maps to posted, failed or uncertain (8.9 step 3)", async (t) => {
|
||||
const s = await ready(t);
|
||||
s.rules([{ method: "POST", path: "comments$", http: 403, body: { message: "forbidden" } }]);
|
||||
const first = no(s.run(move9(), "darkwing"), 1);
|
||||
assert.match(first.out, /^ok review-9-00001\.outcome rev 4 row 9 round 1 request review-9-00001 failed$/m);
|
||||
let n = 1;
|
||||
const cases = [
|
||||
[{ http: 400 }, 1, "failed", 400], [{ http: 401 }, 1, "failed", 401], [{ http: 404 }, 1, "failed", 404], [{ http: 422 }, 1, "failed", 422],
|
||||
[{ http: 500 }, 3, "uncertain", 500], [{ http: 201, body: { nope: true } }, 3, "uncertain", 201], [{ http: 201, body: "not json" }, 3, "uncertain", 201],
|
||||
[{ http: 200, body: { id: 5 } }, 3, "uncertain", 200], [{ fail: true }, 3, "uncertain", null],
|
||||
];
|
||||
for (const [rule, code, state, status] of cases) {
|
||||
s.rules([{ method: "POST", path: "comments$", ...rule }]);
|
||||
const op = `request-9-${String(++n).padStart(4, "0")}`;
|
||||
const r = no(s.run(request(op), "darkwing"), code);
|
||||
assert.match(r.out, new RegExp(`request ${op} ${state}$`, "m"));
|
||||
const a = s.round(9, 1).attempts.at(-1);
|
||||
assert.deepEqual([a.op, a.state, a.comment, a.transport.status], [op, state, null, status]);
|
||||
assert.ok(!JSON.stringify(a.transport).includes("forbidden"));
|
||||
if (state === "uncertain") {
|
||||
no(s.run(["review", "abandon", "9", op, "--reason", "not posted", "--op", `abandon-${op}`], "sage"), 2, /re-run with --yes/);
|
||||
no(s.run(["review", "abandon", "9", op, "--reason", "not posted", "--yes", "--op", `abandon-${op}`], "darkwing"), 2, /only a privileged actor/);
|
||||
ok(s.run(["review", "abandon", "9", op, "--reason", "not posted", "--yes", "--op", `abandon-${op}`], "sage"), /uncertain→abandoned; a duplicate request comment may exist$/m);
|
||||
}
|
||||
}
|
||||
assert.equal(s.round(9, 1).duplicateRisk, true);
|
||||
s.rules([{ method: "POST", path: "comments$", sleepMs: 5000 }]);
|
||||
const t0 = Date.now();
|
||||
const late = s.mut({ verb: "review-request", op: "request-9-late1", args: { id: 9 }, by: "darkwing" }, { deadlineMs: 300 });
|
||||
assert.ok(Date.now() - t0 < 4000, "the deadline killed the helper");
|
||||
assert.equal(late.code, 3);
|
||||
assert.deepEqual(s.round(9, 1).attempts.at(-1).transport, { outcome: "uncertain", status: null, comment: null, detail: "no answer before the deadline" });
|
||||
ok(s.run(["review", "abandon", "9", "request-9-late1", "--reason", "timed out", "--yes", "--op", "abandon-late1"], "sage"));
|
||||
s.rules([]);
|
||||
ok(s.run(request("request-9-final"), "darkwing"), /request request-9-final posted comment \d+$/m);
|
||||
assert.match(readFileSync(s.repo.viewPath, "utf8"), /; in-review, round 1, request posted \|/);
|
||||
});
|
||||
|
||||
test("the pre-send checks: GET user must name the requester, under the deadline", async (t) => {
|
||||
const s = await ready(t);
|
||||
s.rules([{ method: "GET", path: "^user$", http: 200, body: { login: "dewey" } }]);
|
||||
const r = no(s.run(move9(), "darkwing"), 1, /request review-9-00001 not sent: the token belongs to dewey, not darkwing/);
|
||||
assert.match(r.out, /request review-9-00001 failed$/m);
|
||||
assert.deepEqual(s.round(9, 1).attempts[0].transport, { outcome: "failed", status: null, comment: null, detail: "pre-send: the credential or account check failed" });
|
||||
s.rules([{ method: "GET", path: "^user$", http: 200, body: { login: "Bad Name\n" } }]);
|
||||
no(s.run(request("request-9-0002"), "darkwing"), 1, /the token belongs to an unexpected value, not darkwing/);
|
||||
s.rules([{ method: "GET", path: "^user$", http: 401, body: { message: "bad token" } }]);
|
||||
const unauth = no(s.run(request("request-9-0003"), "darkwing"), 1, /GET user answered HTTP 401/);
|
||||
assert.ok(!unauth.err.includes("bad token"));
|
||||
s.rules([{ method: "GET", path: "^user$", sleepMs: 5000 }]);
|
||||
const slow = s.mut({ verb: "review-request", op: "request-9-0004", args: { id: 9 }, by: "darkwing" }, { deadlineMs: 300 });
|
||||
assert.equal(slow.code, 1);
|
||||
assert.match(slow.err.join("\n"), /GET user had no answer before the deadline/);
|
||||
assert.equal(s.posts().length, 0);
|
||||
// The lead posts as jarvis, with jarvis's file; a token for "sage" is refused.
|
||||
s.rules([]);
|
||||
ok(s.run(request("request-9-lead1"), "sage"), /request request-9-lead1 posted comment 1000$/m);
|
||||
assert.equal(s.posts()[0].cred, s.tokens.jarvis);
|
||||
assert.equal(s.calls().at(-2).cred, s.tokens.jarvis);
|
||||
assert.equal(s.round(9, 1).attempts.at(-1).by, "sage");
|
||||
});
|
||||
|
||||
test("the lead's request refuses a token for login sage", async (t) => {
|
||||
const s = await ready(t);
|
||||
s.rules([{ method: "POST", path: "comments$", http: 403 }]);
|
||||
no(s.run(move9(), "darkwing"), 1);
|
||||
s.rules([{ method: "GET", path: "^user$", http: 200, body: { login: "sage" } }]);
|
||||
no(s.run(request("request-9-0002"), "sage"), 1, /the token belongs to sage, not jarvis/);
|
||||
no(s.run(request("request-9-0003"), "sage", { MOSAIC_GITEA_CREDENTIAL_FILE: s.tokens.sage }), 1, /not jarvis's token file/);
|
||||
assert.equal(s.posts().length, 1);
|
||||
});
|
||||
|
||||
test("the credential file: the seat's own, 0600, no symlink, never the shared default", async (t) => {
|
||||
const s = await ready(t);
|
||||
const { credCheck, loginFor } = await import(pathToFileURL(join(s.repo.root, "packages/queue/src/review.mjs")).href);
|
||||
assert.equal(loginFor("sage"), "jarvis");
|
||||
assert.equal(loginFor("dewey"), "dewey");
|
||||
const env = (p) => ({ HOME: s.repo.home, MOSAIC_GITEA_CREDENTIAL_FILE: p });
|
||||
assert.equal(credCheck(env(s.tokens.darkwing), "darkwing"), null);
|
||||
assert.equal(credCheck(env(s.tokens.jarvis), "sage"), null);
|
||||
assert.match(credCheck({ HOME: s.repo.home }, "darkwing"), /is not set/);
|
||||
assert.match(credCheck(env("fleet/agents/darkwing/secrets/gitea-mosaicstack-darkwing.token"), "darkwing"), /absolute path/);
|
||||
const shared = join(s.repo.home, "secrets/mosaic.gitea.json");
|
||||
mkdirSync(join(s.repo.home, "secrets"));
|
||||
writeFileSync(shared, "{}", { mode: 0o600 });
|
||||
assert.match(credCheck(env(shared), "darkwing"), /shared default file/);
|
||||
assert.match(credCheck(env(s.tokens.dewey), "darkwing"), /not darkwing's token file/);
|
||||
assert.match(credCheck(env(s.tokens.sage), "sage"), /not jarvis's token file/);
|
||||
assert.match(credCheck(env(s.tokens.darkwing.replace("darkwing.token", "darkwing.tokenx")), "darkwing"), /not darkwing's token file/);
|
||||
const gone = join(s.repo.base, "x/agents/darkwing/secrets/gitea-mosaicstack-darkwing.token");
|
||||
assert.match(credCheck(env(gone), "darkwing"), /does not exist/);
|
||||
chmodSync(s.tokens.rocko, 0o640);
|
||||
assert.match(credCheck(env(s.tokens.rocko), "rocko"), /mode 0600/);
|
||||
chmodSync(s.tokens.rocko, 0o400);
|
||||
assert.match(credCheck(env(s.tokens.rocko), "rocko"), /mode 0600/);
|
||||
// A symlinked file, and a symlinked directory leading to another seat's.
|
||||
const link = join(s.repo.base, "l1/agents/filbert/secrets");
|
||||
mkdirSync(link, { recursive: true });
|
||||
symlinkSync(s.tokens.filbert, join(link, "gitea-mosaicstack-filbert.token"));
|
||||
assert.match(credCheck(env(join(link, "gitea-mosaicstack-filbert.token")), "filbert"), /regular file, not a symlink/);
|
||||
mkdirSync(join(s.repo.base, "l2/agents/filbert"), { recursive: true });
|
||||
symlinkSync(join(s.repo.base, "fleet/agents/dewey/secrets"), join(s.repo.base, "l2/agents/filbert/secrets"));
|
||||
writeFileSync(join(s.repo.base, "fleet/agents/dewey/secrets/gitea-mosaicstack-filbert.token"), "x", { mode: 0o600 });
|
||||
assert.match(credCheck(env(join(s.repo.base, "l2/agents/filbert/secrets/gitea-mosaicstack-filbert.token")), "filbert"), /resolves outside filbert's secrets directory/);
|
||||
// End to end: a refused file makes no call at all.
|
||||
chmodSync(s.tokens.darkwing, 0o644);
|
||||
no(s.run(move9(), "darkwing"), 1, /not sent: darkwing's token file must be mode 0600/);
|
||||
no(s.run(request("request-9-0002"), "darkwing", { MOSAIC_GITEA_CREDENTIAL_FILE: s.tokens.dewey }), 1, /not darkwing's token file/);
|
||||
no(s.run(request("request-9-0003"), "darkwing", { MOSAIC_GITEA_CREDENTIAL_FILE: "" }), 1, /is not set/);
|
||||
assert.deepEqual(s.calls(), []);
|
||||
});
|
||||
|
||||
test("an unresolved request blocks a new request, a new round, waiting-on-jason and done", async (t) => {
|
||||
const s = await ready(t);
|
||||
s.rules([{ method: "POST", path: "comments$", http: 502 }]);
|
||||
no(s.run(move9(), "darkwing"), 3, /^Look on #1508 for a comment carrying <!-- mosaic-queue-op: review-9-00001 -->\.$/m);
|
||||
s.rules([]);
|
||||
const head = s.head();
|
||||
no(s.run(request("request-9-0002"), "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\)/);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "77", "--candidate", head, "--op", "record-9-filbert1"], "filbert"));
|
||||
no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\); resolve or abandon it before closing it/);
|
||||
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\); resolve or abandon it before moving it to waiting-on-jason/);
|
||||
assert.equal(s.posts().length, 1);
|
||||
|
||||
// A POST that lands after the row reached waiting-on-jason: abandoned,
|
||||
// approved and moved while it was in flight. Jason's close then refuses.
|
||||
const s3 = await ready(t);
|
||||
const head3 = s3.head();
|
||||
const hook = (n) => {
|
||||
if (n !== "posted") return;
|
||||
s3.mut({ verb: "review-abandon", op: "abandon-9-0001", args: { id: 9, attempt: "review-9-00001", reason: "gave up" }, by: "sage", yes: true });
|
||||
s3.mut({ verb: "review-record", op: "record-9-filb01", args: { id: 9, verdict: "approve", comment: 88, candidate: head3 }, by: "filbert" });
|
||||
s3.mut({ verb: "move", op: "wait-9-000001", args: { id: 9, to: "waiting-on-jason" }, by: "darkwing" });
|
||||
};
|
||||
assert.equal(s3.mut({ verb: "move", op: "review-9-00001", args: { id: 9, to: "in-review", candidate: "HEAD" }, by: "darkwing" }, { hook }).code, 3);
|
||||
assert.deepEqual([s3.row(9).state, s3.round(9, 1).attempts[0].state], ["waiting-on-jason", "conflict"]);
|
||||
no(s3.run(["move", "9", "done", "--op", "done-9-000002"], "jason"), 2, /unresolved review request: review-9-00001 \(round 1, conflict\); resolve or abandon it before closing it/);
|
||||
|
||||
// Back to in-progress is allowed; the next round is not.
|
||||
const s2 = await ready(t);
|
||||
s2.rules([{ method: "POST", path: "comments$", http: 502 }]);
|
||||
no(s2.run(move9(), "darkwing"), 3);
|
||||
s2.rules([]);
|
||||
ok(s2.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing"));
|
||||
no(s2.run(["move", "9", "in-review", "--candidate", "HEAD", "--op", "review-9-00002"], "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\); resolve or abandon it before a new round/);
|
||||
assert.equal(s2.posts().length, 1);
|
||||
});
|
||||
|
||||
test("a same-op retry after a kill sends nothing, even with a stale view", async (t) => {
|
||||
for (const step of ["pre-send", "posted", "outcome", "locked#2"]) {
|
||||
const s = await ready(t);
|
||||
const viewBefore = readFileSync(s.repo.viewPath);
|
||||
const req = { verb: "move", op: "review-9-00001", args: { id: 9, to: "in-review", candidate: "HEAD" }, by: "darkwing" };
|
||||
const r = spawnSync(process.execPath, [join(HERE, "fixtures", "kill-at.mjs"), join(s.repo.root, "packages/queue/src"), step, JSON.stringify(req)], {
|
||||
cwd: s.repo.root, env: { ...s.repo.env, ...s.cred("darkwing") }, encoding: "utf8",
|
||||
});
|
||||
assert.equal(r.signal, "SIGKILL", `child did not die at ${step}: ${r.stdout}${r.stderr}`);
|
||||
if (step === "locked#2") ok(cli(s.repo, ["unlock"]), /^removed queue lock \(dead/);
|
||||
const posted = step === "pre-send" ? 0 : 1;
|
||||
assert.equal(s.posts().length, posted, step);
|
||||
assert.equal(s.round(9, 1).attempts[0].state, "requesting");
|
||||
writeFileSync(s.repo.viewPath, viewBefore);
|
||||
const calls = s.calls().length;
|
||||
const again = no(s.run(move9(), "darkwing"), 3);
|
||||
assert.match(again.out, /\(already recorded at rev 3\)$/m);
|
||||
assert.match(again.err, /warning: view stale/);
|
||||
assert.match(again.err, /request review-9-00001 is requesting \(row 9 round 1 on #1508\); nothing was sent again\.\nLook on #1508 for a comment carrying <!-- mosaic-queue-op: review-9-00001 -->\./);
|
||||
assert.equal(s.calls().length, calls, step);
|
||||
ok(cli(s.repo, ["render"]));
|
||||
if (posted) {
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "1000", "--op", "resolve-9-00001"], "darkwing"), /requesting→posted comment 1000$/m);
|
||||
assert.equal(s.calls().at(-1).path, "repos/mosaicstack/stack/issues/comments/1000");
|
||||
} else {
|
||||
ok(s.run(["review", "abandon", "9", "review-9-00001", "--reason", "never sent", "--yes", "--op", "abandon-9-0001"], "sage"));
|
||||
ok(s.run(request("request-9-0002"), "darkwing"), /posted comment 1000$/m);
|
||||
}
|
||||
assert.equal(s.posts().length, 1);
|
||||
}
|
||||
});
|
||||
|
||||
test("a held lock at the outcome exits 3 and names what the transport said", async (t) => {
|
||||
const s = await ready(t);
|
||||
let handle = null;
|
||||
const hook = (n) => {
|
||||
if (n === "outcome") handle = s.m.lock.acquire({ gitDir: s.repo.gitDir, io: s.m.io.realIo, op: "holder-op-1", verb: "move" });
|
||||
};
|
||||
throwsCode(() => s.mut({ verb: "move", op: "review-9-00001", args: { id: 9, to: "in-review", candidate: "HEAD" }, by: "darkwing" }, { hook, lockWaitMs: 200, lockStepMs: 50 }), 3,
|
||||
/^ok review-9-00001 rev 3 row 9 in-progress→in-review round 1 on #1508; request review-9-00001 requesting\nuncertain review-9-00001: the transport said posted comment 1000; that outcome is not recorded: queue lock held by move holder-op-1/);
|
||||
s.m.lock.releaseOrWarn(handle, s.m.io.realIo);
|
||||
assert.equal(s.round(9, 1).attempts[0].state, "requesting");
|
||||
no(s.run(move9(), "darkwing"), 3, /nothing was sent again/);
|
||||
assert.equal(s.posts().length, 1);
|
||||
});
|
||||
|
||||
test("late outcomes: after an abandon, and after a resolve with the same or another id", async (t) => {
|
||||
const s = await ready(t);
|
||||
const head = s.head();
|
||||
const mv = (op) => ({ verb: "move", op, args: { id: 9, to: "in-review", candidate: "HEAD" }, by: "darkwing" });
|
||||
// Abandoned while the POST was in flight, then the POST lands: conflict.
|
||||
let hook = (n) => { if (n === "posted") s.mut({ verb: "review-abandon", op: "abandon-9-0001", args: { id: 9, attempt: "review-9-00001", reason: "gave up" }, by: "sage", yes: true }); };
|
||||
const r1 = s.mut(mv("review-9-00001"), { hook });
|
||||
assert.equal(r1.code, 3);
|
||||
assert.match(r1.out.at(-1), /request review-9-00001 conflict \(transport posted\)$/);
|
||||
let a = s.round(9, 1).attempts[0];
|
||||
assert.deepEqual([a.state, a.comment, a.transport.comment, a.resolutions.map((x) => x.verb)], ["conflict", null, 1000, ["abandon"]]);
|
||||
assert.equal(s.round(9, 1).duplicateRisk, true);
|
||||
no(s.run(request("request-9-0002"), "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, conflict\)/);
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "1000", "--op", "resolve-9-00001"], "darkwing"), /conflict→posted comment 1000$/m);
|
||||
a = s.round(9, 1).attempts[0];
|
||||
assert.deepEqual([a.state, a.comment, a.resolutions.map((x) => x.verb)], ["posted", 1000, ["abandon", "resolve"]]);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "1000", "--op", "resolve-9-00002"], "darkwing"), 2, /is posted; only a requesting, uncertain or conflict request can be resolved/);
|
||||
|
||||
// Round 2: resolved with the id the POST returns, then the outcome agrees.
|
||||
ok(s.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing"));
|
||||
hook = (n) => { if (n === "posted") s.mut({ verb: "review-resolve", op: "resolve-9-r2-01", args: { id: 9, attempt: "review-9-00002", comment: 1001 }, by: "darkwing" }); };
|
||||
const r2 = s.mut(mv("review-9-00002"), { hook });
|
||||
assert.equal(r2.code, 0);
|
||||
assert.match(r2.out.at(-1), /request review-9-00002 posted comment 1001$/);
|
||||
assert.deepEqual(s.round(9, 2).attempts[0].transport, { outcome: "posted", status: 201, comment: 1001, detail: "created" });
|
||||
|
||||
// Round 3: resolved with another comment that carries the markers, then
|
||||
// the POST returns a different id: conflict.
|
||||
ok(s.run(["move", "9", "in-progress", "--op", "changes-9-0002"], "darkwing"));
|
||||
const marked = `<!-- mosaic-queue-op: review-9-00003 -->\n<!-- mosaic-queue-round: row=9 round=3 candidate=${head} -->\nposted by hand\n`;
|
||||
s.rules([{ method: "GET", path: "comments/5555$", http: 200, body: { id: 5555, issue_url: "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/1508", body: marked, user: { login: "darkwing" } } }]);
|
||||
hook = (n) => { if (n === "posted") s.mut({ verb: "review-resolve", op: "resolve-9-r3-01", args: { id: 9, attempt: "review-9-00003", comment: 5555 }, by: "darkwing" }); };
|
||||
const r3 = s.mut(mv("review-9-00003"), { hook });
|
||||
assert.equal(r3.code, 3);
|
||||
a = s.round(9, 3).attempts[0];
|
||||
assert.deepEqual([a.state, a.comment, a.transport.comment, a.resolutions.map((x) => [x.verb, x.comment])], ["conflict", null, 1002, [["resolve", 5555]]]);
|
||||
assert.equal(s.round(9, 3).duplicateRisk, false);
|
||||
// A retry of the logged resolve answers from the log and makes no call,
|
||||
// though the attempt is unresolved again.
|
||||
const before = s.calls().length;
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00003", "--comment", "5555", "--op", "resolve-9-r3-01"], "darkwing"), /already recorded/);
|
||||
assert.equal(s.calls().length, before);
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00003", "--comment", "1002", "--op", "resolve-9-r3-02"], "darkwing"), /conflict→posted comment 1002$/m);
|
||||
|
||||
// Rounds 4 and 5: resolved by hand, then the POST answers 500 (the
|
||||
// resolve stands) or 422 (a refusal contradicts it: conflict).
|
||||
for (const [n, comment, http, code, state] of [[4, 5556, 500, 0, "posted"], [5, 5557, 422, 3, "conflict"]]) {
|
||||
ok(s.run(["move", "9", "in-progress", "--op", `changes-9-000${n}`], "darkwing"));
|
||||
const op = `review-9-0000${n}`;
|
||||
const body = `<!-- mosaic-queue-op: ${op} -->\n<!-- mosaic-queue-round: row=9 round=${n} candidate=${head} -->\nposted by hand\n`;
|
||||
s.rules([
|
||||
{ method: "GET", path: `comments/${comment}$`, http: 200, body: { id: comment, issue_url: "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/1508", body, user: { login: "darkwing" } } },
|
||||
{ method: "POST", path: "comments$", http },
|
||||
]);
|
||||
hook = (step) => { if (step === "posted") s.mut({ verb: "review-resolve", op: `resolve-9-r${n}-01`, args: { id: 9, attempt: op, comment }, by: "darkwing" }); };
|
||||
const r = s.mut(mv(op), { hook });
|
||||
assert.equal(r.code, code, `round ${n}`);
|
||||
a = s.round(9, n).attempts[0];
|
||||
assert.deepEqual([a.state, a.comment, a.transport.status], [state, state === "posted" ? comment : null, http], `round ${n}`);
|
||||
}
|
||||
assert.equal(s.posts().length, 5);
|
||||
});
|
||||
|
||||
test("resolve checks the comment: issue, markers, round, candidate and author", async (t) => {
|
||||
const s = await ready(t);
|
||||
const head = s.head();
|
||||
s.rules([{ method: "POST", path: "comments$", http: 500 }]);
|
||||
no(s.run(move9(), "darkwing"), 3);
|
||||
const good = { id: 7001, issue_url: "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/1508",
|
||||
body: `intro\n<!-- mosaic-queue-op: review-9-00001 -->\n<!-- mosaic-queue-round: row=9 round=1 candidate=${head} -->\n`, user: { login: "darkwing" } };
|
||||
const bad = [
|
||||
[{ issue_url: good.issue_url.replace("1508", "1509") }, 2, /does not match request review-9-00001: the issue \(want #1508\)/],
|
||||
[{ issue_url: `${good.issue_url}0` }, 2, /the issue/],
|
||||
[{ body: good.body.replace("review-9-00001", "review-9-00009") }, 2, /the op marker for review-9-00001$/m],
|
||||
[{ body: good.body.replace("round=1", "round=2") }, 2, /the round marker/],
|
||||
[{ body: good.body.replace(head, "f".repeat(40)) }, 2, /the round marker \(row 9 round 1 candidate/],
|
||||
[{ body: good.body.replace("-->\n<!--", "--> <!--") }, 2, /the op marker for review-9-00001, the round marker/],
|
||||
[{ body: `x${good.body.replace("\n<!-- mosaic-queue-round", " <!-- mosaic-queue-round")}` }, 2, /the op marker for review-9-00001, the round marker/],
|
||||
[{ user: { login: "dewey" } }, 2, /its author \(want darkwing\)/],
|
||||
[{ id: 7002 }, 2, /its id/],
|
||||
];
|
||||
for (const [patch, code, re] of bad) {
|
||||
s.rules([{ method: "GET", path: "comments/7001$", http: 200, body: { ...good, ...patch } }]);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), code, re);
|
||||
}
|
||||
s.rules([{ method: "GET", path: "comments/7001$", http: 404 }]);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), 2, /comment 7001 does not exist/);
|
||||
s.rules([{ method: "GET", path: "comments/7001$", http: 500 }]);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), 1, /answered HTTP 500/);
|
||||
s.rules([{ method: "GET", path: "comments/7001$", fail: true }]);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), 1, /failed or had no answer/);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing", { MOSAIC_GITEA_CREDENTIAL_FILE: s.tokens.dewey }), 2, /cannot check comment 7001: .*not darkwing's token file/);
|
||||
const before = s.calls().length;
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "rocko"), 2, /^queue: row 9 is claimed by darkwing; only the owner \(darkwing\) or a privileged actor may resolve a request on row 9$/m);
|
||||
assert.equal(s.calls().length, before, "a refused actor makes no request");
|
||||
assert.equal(s.doc().revision, 4);
|
||||
s.rules([{ method: "GET", path: "comments/7001$", http: 200, body: good }]);
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), /uncertain→posted comment 7001$/m);
|
||||
const calls = s.calls().length;
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), /already recorded/);
|
||||
assert.equal(s.calls().length, calls, "a recorded resolve checks nothing again");
|
||||
const res = s.round(9, 1).attempts[0].resolutions;
|
||||
assert.deepEqual(res.map((x) => [x.verb, x.by, x.comment, x.reason]), [["resolve", "darkwing", 7001, null]]);
|
||||
});
|
||||
|
||||
test("the lead resolves a seat's request: the comment's author is the requester, fetched with the lead's token", async (t) => {
|
||||
const s = await ready(t);
|
||||
const head = s.head();
|
||||
s.rules([{ method: "POST", path: "comments$", fail: true }]);
|
||||
no(s.run(move9(), "darkwing"), 3);
|
||||
const body = `<!-- mosaic-queue-op: review-9-00001 -->\n<!-- mosaic-queue-round: row=9 round=1 candidate=${head} -->\n`;
|
||||
const comment = (login) => ({ id: 7001, issue_url: "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/1508", body, user: { login } });
|
||||
s.rules([{ method: "GET", path: "comments/7001$", http: 200, body: comment("jarvis") }]);
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "sage"), 2, /its author \(want darkwing\)/);
|
||||
s.rules([{ method: "GET", path: "comments/7001$", http: 200, body: comment("darkwing") }]);
|
||||
ok(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "sage"), /uncertain→posted comment 7001$/m);
|
||||
assert.equal(s.calls().at(-1).cred, s.tokens.jarvis);
|
||||
assert.deepEqual(s.round(9, 1).attempts[0].resolutions.map((x) => [x.by, x.comment]), [["sage", 7001]]);
|
||||
});
|
||||
|
||||
test("validateRow checks a request round's shape, which every replayed entry must keep", async (t) => {
|
||||
const s = await ready(t);
|
||||
ok(s.run(move9(), "darkwing"));
|
||||
const { queue } = s.m;
|
||||
const d = s.doc();
|
||||
const row9 = d.rows.find((r) => r.id === 9);
|
||||
queue.validateRow(row9);
|
||||
const forge = (fn) => {
|
||||
const copy = structuredClone(row9);
|
||||
fn(copy.review.rounds[0]);
|
||||
return () => queue.validateRow(copy);
|
||||
};
|
||||
assert.throws(forge((r) => { r.duplicateRisk = true; }), /duplicateRisk is true exactly when an attempt was abandoned/);
|
||||
assert.throws(forge((r) => { r.attempts[0].op = "someone-else-1"; }), /first attempt is the move that opened it/);
|
||||
const rec = { reviewer: "filbert", op: "record-9-filb01", at: "2026-09-27T00:00:00.000Z", verdict: "approve", comment: 88, candidate: d.rows.find((r) => r.id === 9).review.rounds[0].candidate.digest };
|
||||
queue.validateRow({ ...row9, review: { rounds: [{ ...row9.review.rounds[0], receipts: [rec] }] } });
|
||||
assert.throws(forge((r) => { r.receipts = [rec, { ...rec, op: "record-9-filb02" }]; }), /a reviewer has one receipt per round/);
|
||||
assert.throws(forge((r) => { r.attempts.push({ ...r.attempts[0] }); }), /a request round names an attempt twice/);
|
||||
});
|
||||
|
||||
test("request, changes, a new candidate, approval: every round pinned; no review files", async (t) => {
|
||||
const s = await ready(t);
|
||||
const c1 = s.head();
|
||||
ok(s.run(move9(), "darkwing"), /posted comment 1000$/m);
|
||||
ok(cli(s.repo, ["next", "filbert"]), /^review row 9: Queue as data/m);
|
||||
no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "88", "--candidate", c1, "--op", "record-9-dark01"], "darkwing"), 2, /only a listed reviewer other than the owner/);
|
||||
no(s.run(["review", "record", "9", "--verdict", "maybe", "--comment", "88", "--candidate", c1, "--op", "record-9-filb01"], "filbert"), 2, /approve or changes/);
|
||||
no(s.run(["review", "record", "9", "--verdict", "changes", "--candidate", c1, "--op", "record-9-filb01"], "filbert"), 4, /--comment N is required/);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "changes", "--comment", "88", "--candidate", c1, "--op", "record-9-filb01"], "filbert"), /row 9 round 1 changes by filbert \(comment 88\)$/m);
|
||||
no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, /filbert already recorded a verdict for row 9 round 1/);
|
||||
ok(cli(s.repo, ["next", "filbert"]), /^nothing$/m);
|
||||
no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /round 1 has no approval recorded by filbert/);
|
||||
ok(s.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing"));
|
||||
no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, /is in-progress; review record applies to in-review rows/);
|
||||
writeFileSync(join(s.repo.root, "packages/fix.mjs"), "export {};\n");
|
||||
s.repo.g("add", "packages/fix.mjs");
|
||||
s.repo.g("commit", "-q", "-m", "fix", "--", "packages/fix.mjs");
|
||||
const c2 = s.head();
|
||||
ok(s.run(move9("review-9-00002"), "darkwing"), /round 2 on #1508; request review-9-00002 requesting$/m);
|
||||
ok(cli(s.repo, ["next", "filbert"]), /^review row 9/m);
|
||||
no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, new RegExp(`candidate ${c1} is not round 2's candidate ${c2}`));
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c2, "--op", "record-9-filb02"], "filbert"));
|
||||
no(s.run(["move", "9", "done", "--evidence", `comment=89,round=2,candidate=${c2}`, "--op", "done-9-000001"], "filbert"), 2, /closes on its recorded verdicts; drop --evidence/);
|
||||
no(s.run(["move", "9", "done", "--op", "done-9-000001"], "rocko"), 2, /only the gate owner \(filbert\) or a privileged actor/);
|
||||
ok(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), /in-review→done round 2$/m);
|
||||
const rounds = s.row(9).review.rounds;
|
||||
assert.deepEqual(rounds.map((r) => [r.n, r.candidate.digest, r.attempts.map((a) => [a.op, a.state, a.comment]), r.receipts.map((x) => [x.reviewer, x.verdict, x.comment, x.candidate])]), [
|
||||
[1, c1, [["review-9-00001", "posted", 1000]], [["filbert", "changes", 88, c1]]],
|
||||
[2, c2, [["review-9-00002", "posted", 1001]], [["filbert", "approve", 89, c2]]],
|
||||
]);
|
||||
no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "90", "--candidate", c2, "--op", "record-9-filb03"], "filbert"), 2, /row 9 is done; done rows never change/);
|
||||
const untracked = s.repo.g("ls-files", "--others", "--exclude-standard").split("\n").filter(Boolean);
|
||||
assert.deepEqual(untracked.filter((p) => p.startsWith("docs/plans/reviews/") || /^agents\/[^/]+\/work\//.test(p)), []);
|
||||
assert.deepEqual(untracked, ["scripts/gitea-api.sh"]);
|
||||
});
|
||||
|
||||
test("a row with no reviewers opens a round that posts nothing", async (t) => {
|
||||
const s = await ready(t);
|
||||
ok(s.run(["set", "9", "reviewers", "none", "--op", "reviewers-9-none"], "sage"));
|
||||
const r = ok(s.run(move9(), "darkwing"), /in-progress→in-review round 1 on #1508$/m);
|
||||
assert.doesNotMatch(r.out, /request/);
|
||||
assert.deepEqual(Object.keys(s.round(9, 1)), ["n", "op", "by", "at", "issue", "candidate", "request"]);
|
||||
assert.equal(s.round(9, 1).request, "none");
|
||||
no(s.run(request("request-9-0002"), "darkwing"), 2, /posts no request \(the row had no reviewers when it opened\)/);
|
||||
assert.deepEqual(s.calls(), []);
|
||||
|
||||
// The other way round: a request round whose reviewers are then removed
|
||||
// doesn't close on no approvals.
|
||||
const s2 = await ready(t);
|
||||
ok(s2.run(move9(), "darkwing"));
|
||||
ok(s2.run(["set", "9", "reviewers", "none", "--op", "reviewers-9-none"], "sage"));
|
||||
no(s2.run(["move", "9", "done", "--op", "done-9-000001"], "sage"), 2, /row 9 lists no reviewers now; a privileged actor sets them before it closes/);
|
||||
});
|
||||
|
||||
test("verify-commit: a prospective tree must hold exactly the candidate's paths", async (t) => {
|
||||
const s = await ready(t);
|
||||
const base = s.head();
|
||||
const cand = commitOn(s.repo, "cand", base, { "packages/x.mjs": "export const x = 1;\n", "agents/rocko/.keep": null });
|
||||
ok(s.run(["move", "9", "in-review", "--candidate", "cand", "--op", "review-9-00001"], "darkwing"));
|
||||
const other = commitOn(s.repo, "other", base, { "docs/other.md": "other\n" });
|
||||
const good = commitOn(s.repo, "good", other, { "packages/x.mjs": "export const x = 1;\n", "agents/rocko/.keep": null });
|
||||
ok(cli(s.repo, ["review", "verify-commit", "9", "good"]), /^ok row 9 round 1: good matches the commit candidate \(2 paths\)$/m);
|
||||
const changed = commitOn(s.repo, "changed", good, { "packages/x.mjs": "export const x = 2;\n" });
|
||||
no(cli(s.repo, ["review", "verify-commit", "9", "changed"]), 2, /changed does not match row 9 round 1's candidate:\npackages\/x.mjs: content differs$/m);
|
||||
const exec = spawnSync("git", ["-C", s.repo.root, "commit-tree", s.repo.g("rev-parse", "good^{tree}").trim(), "-p", good, "-m", "exec"], { env: s.repo.env, encoding: "utf8" });
|
||||
assert.equal(exec.status, 0, exec.stderr);
|
||||
const execEnv = { ...s.repo.env, GIT_INDEX_FILE: join(s.repo.base, "index-exec") };
|
||||
const gx = (...a) => { const r = spawnSync("git", ["-C", s.repo.root, ...a], { env: execEnv, encoding: "utf8" }); assert.equal(r.status, 0, r.stderr); return r.stdout.trim(); };
|
||||
gx("read-tree", good);
|
||||
const blob = gx("rev-parse", "good:packages/x.mjs");
|
||||
gx("update-index", "--cacheinfo", `100755,${blob},packages/x.mjs`);
|
||||
gx("update-ref", "refs/heads/execbit", gx("commit-tree", gx("write-tree"), "-p", good, "-m", "execbit"));
|
||||
no(cli(s.repo, ["review", "verify-commit", "9", "execbit"]), 2, /^packages\/x.mjs: mode differs$/m);
|
||||
const kept = commitOn(s.repo, "kept", good, { "agents/rocko/.keep": "" });
|
||||
no(cli(s.repo, ["review", "verify-commit", "9", "kept"]), 2, /agents\/rocko\/.keep: deleted in the candidate, present here/);
|
||||
no(cli(s.repo, ["review", "verify-commit", "9", "other"]), 2, /packages\/x.mjs: missing\nagents\/rocko\/.keep: deleted|agents\/rocko\/.keep: deleted in the candidate, present here\npackages\/x.mjs: missing/);
|
||||
no(cli(s.repo, ["review", "verify-commit", "9", "nope"]), 2, /nope is not a commit or tree here/);
|
||||
no(cli(s.repo, ["review", "verify-commit", "11", "good"]), 2, /row 11 has no review round/);
|
||||
// A manifest candidate on row 6.
|
||||
ok(s.run(["move", "6", "in-progress", "--op", "unblock-6-0001"], "darkwing"));
|
||||
const digest = spawnSync("sha256sum", { input: "export const x = 1;\n", encoding: "utf8" }).stdout.slice(0, 64);
|
||||
const manifest = join(s.repo.base, "cand.sha256");
|
||||
writeFileSync(manifest, `${digest} packages/x.mjs\n`);
|
||||
ok(s.run(["move", "6", "in-review", "--candidate", manifest, "--op", "review-6-00001"], "darkwing"), /on #1511; request review-6-00001 requesting$/m);
|
||||
assert.match(s.posts().at(-1).body.body, new RegExp(`\`\`\`text\n${digest} packages/x.mjs\n\`\`\``));
|
||||
ok(cli(s.repo, ["review", "verify-commit", "6", "good"]), /matches the manifest candidate \(1 paths\)/);
|
||||
no(cli(s.repo, ["review", "verify-commit", "6", "changed"]), 2, /packages\/x.mjs: content differs/);
|
||||
no(cli(s.repo, ["review", "verify-commit", "6", "HEAD"]), 2, /packages\/x.mjs: missing/);
|
||||
});
|
||||
|
||||
test("semantics: v1 entries replay as before; review entries need v2", async (t) => {
|
||||
const s = await ready(t);
|
||||
const { queue } = s.m;
|
||||
ok(s.run(move9(), "darkwing"));
|
||||
const d = s.doc();
|
||||
queue.loadDoc(Buffer.from(queue.serialize(d)));
|
||||
// The same move under semantics 1 opens a round with no request.
|
||||
const mv = d.log.find((e) => e.op === "review-9-00001");
|
||||
const pre = queue.replay(d.log.slice(0, mv.rev));
|
||||
const v1 = queue.applyEntry(pre, { ...mv, semantics: 1 }, queue.resolvedFromResult("move", mv.args, mv.result));
|
||||
assert.equal(v1.state.rows.get(9).review.rounds[0].request, "none");
|
||||
assert.equal(v1.result.receipt, "ok review-9-00001 rev 3 row 9 in-progress→in-review round 1 on #1508");
|
||||
// A log written under semantics 1 with no review entries still loads.
|
||||
const old = { ...d, revision: 2, rows: queue.rowsArray(pre), log: d.log.slice(0, 3).map((e) => ({ ...e, semantics: 1 })) };
|
||||
queue.loadDoc(Buffer.from(queue.serialize(old)));
|
||||
// Relabelling the request move as semantics 1 breaks the replay.
|
||||
const forged = { ...d, log: d.log.map((e) => (e.op === mv.op ? { ...e, semantics: 1 } : e)) };
|
||||
assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize(forged))), /does not replay|differs from its replay|needs semantics 2|rows do not equal/);
|
||||
const outcome = d.log.at(-1);
|
||||
assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: [...d.log.slice(0, -1), { ...outcome, semantics: 1 }] }))), /review-outcome needs semantics 2/);
|
||||
assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: [...d.log.slice(0, -1), { ...outcome, op: "review-9-00002.outcome" }] }))), /op must be its attempt's op plus \.outcome/);
|
||||
assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: d.log.map((e, i) => (i === 1 ? { ...e, semantics: 3 } : e)) }))), /semantics 3 is not 1 to 2/);
|
||||
// An attempt takes one outcome, even one that agrees with it.
|
||||
const full = queue.replay(d.log);
|
||||
const again = { ...outcome, rev: full.revision + 1 };
|
||||
assert.throws(() => queue.applyEntry(full, again, {}), /request review-9-00001 already has its outcome/);
|
||||
// review-outcome is the one entry a done row takes.
|
||||
const at = "2026-09-27T00:00:00.000Z";
|
||||
// The request was abandoned and the row closed before the POST's answer came.
|
||||
const entry = (st, verb, op, args, by = "darkwing") => ({ rev: st.revision + 1, op, verb, args: queue.canonArgs(verb, args), by, at, semantics: 2, result: null, viewSha: null });
|
||||
const mid = queue.replay(d.log.slice(0, -1));
|
||||
assert.equal(mid.rows.get(9).review.rounds[0].attempts[0].state, "requesting");
|
||||
const gaveUp = queue.applyEntry(mid, entry(mid, "review-abandon", "abandon-9-0001", { id: 9, attempt: "review-9-00001", reason: "no answer" }, "sage"), {}).state;
|
||||
const done = new Map(gaveUp.rows);
|
||||
done.set(9, { ...gaveUp.rows.get(9), state: "done", claim: null });
|
||||
const st = { ...gaveUp, rows: done };
|
||||
const late = queue.applyEntry(st, entry(st, "review-outcome", "review-9-00001.outcome", { id: 9, attempt: "review-9-00001", outcome: "posted", status: 201, comment: 1000, detail: "created" }), {});
|
||||
const a = late.state.rows.get(9).review.rounds[0].attempts[0];
|
||||
assert.deepEqual([late.state.rows.get(9).state, a.state, a.transport.comment], ["done", "conflict", 1000]);
|
||||
const lateOk = (verb, op, args, by) => queue.applyEntry(st, entry(st, verb, op, args, by), {});
|
||||
assert.throws(() => lateOk("review-resolve", "resolve-9-00001", { id: 9, attempt: "review-9-00001", comment: 5 }), /done rows never change/);
|
||||
assert.throws(() => queue.applyEntry(mid, entry(mid, "review-outcome", "review-9-00001.outcome", { id: 9, attempt: "review-9-00001", outcome: "failed", status: 403, comment: null, detail: "x" }, "sage"), {}), /only darkwing, who made request review-9-00001, records its outcome/);
|
||||
assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "failed", status: 500, comment: null, detail: "x" }), /a failed outcome's status is one of 400, 401, 403, 404, 422/);
|
||||
assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "posted", status: 201, comment: null, detail: "x" }), /posted exactly when HTTP 201 returned a comment id/);
|
||||
assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "posted", status: 200, comment: 4, detail: "x" }), /posted exactly when HTTP 201/);
|
||||
});
|
||||
|
||||
test("the owner records no verdict, even as a listed reviewer", async (t) => {
|
||||
const s = await ready(t);
|
||||
ok(s.run(move9(), "darkwing"));
|
||||
ok(s.run(["set", "9", "reviewers", "filbert,darkwing", "--op", "reviewers-9-self"], "sage"));
|
||||
no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "1000", "--candidate", s.head(), "--op", "record-9-self-01"], "darkwing"), 2, /only a listed reviewer other than the owner may record a verdict on row 9/);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "1000", "--candidate", s.head(), "--op", "record-9-filbert"], "filbert"));
|
||||
});
|
||||
|
||||
test("a request comment over the length limit is not sent", async (t) => {
|
||||
const s = await ready(t);
|
||||
ok(s.run(["move", "6", "in-progress", "--op", "unblock-6-0001"], "darkwing"));
|
||||
const lines = Array.from({ length: 700 }, (_, i) => `${"a".repeat(64)} packages/generated/file-${String(i).padStart(4, "0")}-${"x".repeat(20)}.mjs`);
|
||||
const manifest = join(s.repo.base, "big.sha256");
|
||||
writeFileSync(manifest, `${lines.join("\n")}\n`);
|
||||
const r = no(s.run(["move", "6", "in-review", "--candidate", manifest, "--op", "review-6-00001"], "darkwing"), 1, /request review-6-00001 not sent: the comment would be longer than the limit/);
|
||||
assert.match(r.out, /request review-6-00001 failed$/m);
|
||||
assert.equal(s.round(6, 1).attempts[0].transport.detail, "pre-send: the request comment is too long");
|
||||
assert.equal(s.posts().length, 0);
|
||||
});
|
||||
|
||||
test("a late POST on a closed row leaves a conflict nothing can resolve, and resolve asks nothing", async (t) => {
|
||||
const s = await ready(t);
|
||||
const head = s.head();
|
||||
// Abandoned, approved and closed while the POST was in flight.
|
||||
const hook = (n) => {
|
||||
if (n !== "posted") return;
|
||||
s.mut({ verb: "review-abandon", op: "abandon-9-0001", args: { id: 9, attempt: "review-9-00001", reason: "gave up" }, by: "sage", yes: true });
|
||||
s.mut({ verb: "review-record", op: "record-9-filb01", args: { id: 9, verdict: "approve", comment: 88, candidate: head }, by: "filbert" });
|
||||
s.mut({ verb: "move", op: "done-9-000001", args: { id: 9, to: "done" }, by: "filbert" });
|
||||
};
|
||||
const r = s.mut({ verb: "move", op: "review-9-00001", args: { id: 9, to: "in-review", candidate: "HEAD" }, by: "darkwing" }, { hook });
|
||||
assert.equal(r.code, 3);
|
||||
const a = s.round(9, 1).attempts[0];
|
||||
assert.deepEqual([s.row(9).state, a.state, a.transport.comment], ["done", "conflict", 1000]);
|
||||
const calls = s.calls().length;
|
||||
no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "1000", "--op", "resolve-9-00001"], "darkwing"), 2, /row 9 is done; done rows never change/);
|
||||
assert.equal(s.calls().length, calls, "a done row's resolve fetches nothing");
|
||||
assert.equal(s.posts().length, 1);
|
||||
});
|
||||
|
||||
test("a Jason-gated row reaches waiting-on-jason only on every reviewer's approval", async (t) => {
|
||||
const s = await ready(t);
|
||||
const head = s.head();
|
||||
ok(s.run(["set", "9", "gate-owner", "jason", "--op", "gate-9-jason"], "sage"));
|
||||
ok(s.run(["set", "9", "reviewers", "filbert,rocko", "--op", "reviewers-9-two"], "sage"));
|
||||
ok(s.run(move9(), "darkwing"), /posted comment 1000$/m);
|
||||
assert.deepEqual(s.round(9, 1).receipts, []);
|
||||
no(s.run(["move", "9", "done", "--op", "done-9-000001"], "darkwing"), 2, /gate is Jason's; it goes through waiting-on-jason/);
|
||||
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 1 has no approval recorded by filbert, rocko$/m);
|
||||
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "sage"), 2, /no approval recorded by filbert, rocko$/m);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "88", "--candidate", head, "--op", "record-9-filb01"], "filbert"));
|
||||
ok(s.run(["review", "record", "9", "--verdict", "changes", "--comment", "89", "--candidate", head, "--op", "record-9-rock01"], "rocko"));
|
||||
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /no approval recorded by rocko$/m);
|
||||
// Changes go back to in-progress, as before.
|
||||
ok(s.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing"));
|
||||
ok(s.run(move9("review-9-00002"), "darkwing"), /posted comment 1001$/m);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "91", "--candidate", head, "--op", "record-9-rock02"], "rocko"));
|
||||
// Filbert's round 1 approval doesn't carry into round 2.
|
||||
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 2 has no approval recorded by filbert$/m);
|
||||
ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "90", "--candidate", head, "--op", "record-9-filb02"], "filbert"));
|
||||
ok(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), /in-review→waiting-on-jason$/m);
|
||||
ok(s.run(["move", "9", "done", "--op", "done-9-000001"], "jason"), /waiting-on-jason→done$/m);
|
||||
|
||||
// Reviewers removed after the round opened: waiting-on-jason refuses
|
||||
// until a privileged actor sets them.
|
||||
const s2 = await ready(t);
|
||||
ok(s2.run(["set", "9", "gate-owner", "jason", "--op", "gate-9-jason"], "sage"));
|
||||
ok(s2.run(move9(), "darkwing"));
|
||||
ok(s2.run(["set", "9", "reviewers", "none", "--op", "reviewers-9-none"], "sage"));
|
||||
no(s2.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 lists no reviewers now; a privileged actor sets them before it moves to waiting-on-jason/);
|
||||
});
|
||||
@@ -183,17 +183,20 @@ test("a retried add returns the id it first allocated, after reassignment and af
|
||||
|
||||
test("Rocko's S4 schedule: a lost result, another writer, then the retry opens no second round", (t) => {
|
||||
const repo = ready(t);
|
||||
// No reviewers, so the round posts no request; review.test.mjs covers that path.
|
||||
ok(cli(repo, ["set", "9", "reviewers", "none", "--op", "reviewers-9-none"], { by: "sage" }));
|
||||
ok(cli(repo, ["move", "6", "blocked", "--reason", "paused", "--op", "block-6-00001"], { by: "darkwing" }));
|
||||
ok(cli(repo, ["move", "9", "in-progress", "--op", "start-9-00001"], { by: "darkwing" }));
|
||||
const review = ["move", "9", "in-review", "--candidate", "HEAD", "--op", "review-9-00001"];
|
||||
cli(repo, review, { by: "darkwing" }); // result lost
|
||||
ok(cli(repo, ["note", "9", "looking now", "--op", "note-9-000001"], { by: "filbert" }));
|
||||
ok(cli(repo, review, { by: "darkwing" }), /round 1 on #1508 \(already recorded at rev 3\)/);
|
||||
ok(cli(repo, ["note", "9", "looking now", "--op", "note-9-000001"], { by: "sage" }));
|
||||
ok(cli(repo, review, { by: "darkwing" }), /round 1 on #1508 \(already recorded at rev 4\)/);
|
||||
assert.equal(row(repo, 9).review.rounds.length, 1);
|
||||
});
|
||||
|
||||
test("the review issue and the evidence round through the CLI (lead decision 23, 8.7)", (t) => {
|
||||
const repo = ready(t);
|
||||
ok(cli(repo, ["set", "9", "reviewers", "none", "--op", "reviewers-9-none"], { by: "sage" }));
|
||||
ok(cli(repo, ["move", "6", "blocked", "--reason", "paused", "--op", "block-6-00001"], { by: "darkwing" }));
|
||||
ok(cli(repo, ["set", "9", "issues", "1495,1508", "--op", "issues-9-0001"], { by: "sage" }));
|
||||
ok(cli(repo, ["move", "9", "in-progress", "--op", "start-9-00001"], { by: "darkwing" }));
|
||||
|
||||
+45
-20
@@ -7,6 +7,11 @@
|
||||
# Security:
|
||||
# - Reads credentials from ~/secrets/mosaic.gitea.json (or
|
||||
# MOSAIC_GITEA_CREDENTIAL_FILE); file must be 0600, non-symlink.
|
||||
# - A file that parses as JSON must hold mosaicstack.url and api_token.
|
||||
# A file that does not parse is a raw per-seat token file: exactly 40
|
||||
# lowercase hex characters, optionally followed by one newline. Its base
|
||||
# URL is fixed at https://git.mosaicstack.dev, with no override. Any other
|
||||
# content refuses (exit 3) before a request is made.
|
||||
# - Token is passed to curl via a config stream (never argv, never disk,
|
||||
# never stdout/stderr).
|
||||
# - Prints the response body on stdout and "HTTP <code>" on stderr.
|
||||
@@ -23,35 +28,55 @@ command -v node >/dev/null || { echo "gitea-api: node not found" >&2; exit 1; }
|
||||
|
||||
export MOSAIC_GITEA_CREDENTIAL_FILE="${MOSAIC_GITEA_CREDENTIAL_FILE:-$HOME/secrets/mosaic.gitea.json}"
|
||||
|
||||
# Validate credential file; emit only the non-secret base URL on stdout.
|
||||
BASE="$(node -e '
|
||||
# Validate the credential file. `base` prints only the non-secret base URL;
|
||||
# `cfg` prints the curl config stream. Both run every check, so the second
|
||||
# read refuses a file that changed after the first.
|
||||
CRED_JS='
|
||||
const fs = require("fs");
|
||||
const p = process.env.MOSAIC_GITEA_CREDENTIAL_FILE;
|
||||
let s;
|
||||
try { s = fs.lstatSync(p); } catch { process.exit(3); }
|
||||
if (!s.isFile() || s.isSymbolicLink() || (s.mode & 0o077) !== 0) process.exit(3);
|
||||
let e;
|
||||
try { e = JSON.parse(fs.readFileSync(p, "utf8")).mosaicstack || {}; } catch { process.exit(3); }
|
||||
const base = String(e.url || "").replace(/\/+$/, "");
|
||||
if (!/^https:\/\/git\.mosaicstack\.dev$/.test(base)) process.exit(3);
|
||||
if (typeof e.api_token !== "string" || e.api_token.length === 0) process.exit(3);
|
||||
process.stdout.write(base);
|
||||
')"
|
||||
let text;
|
||||
try { text = fs.readFileSync(p, "utf8"); } catch { process.exit(3); }
|
||||
let base, token;
|
||||
let e = null;
|
||||
try { e = JSON.parse(text).mosaicstack || {}; } catch (err) { if (!(err instanceof SyntaxError)) process.exit(3); }
|
||||
if (e !== null) {
|
||||
base = String(e.url || "").replace(/\/+$/, "");
|
||||
if (!/^https:\/\/git\.mosaicstack\.dev$/.test(base)) process.exit(3);
|
||||
if (typeof e.api_token !== "string" || e.api_token.length === 0) process.exit(3);
|
||||
token = e.api_token;
|
||||
} else {
|
||||
const n = Buffer.byteLength(text);
|
||||
if ((s.size !== 40 && s.size !== 41) || n !== s.size || !/^[0-9a-f]{40}\n?$/.test(text)) process.exit(3);
|
||||
base = "https://git.mosaicstack.dev";
|
||||
token = text.slice(0, 40);
|
||||
}
|
||||
if (process.argv[1] === "cfg") {
|
||||
process.stdout.write("header = \"Authorization: token " + token + "\"\n");
|
||||
process.stdout.write("header = \"Content-Type: application/json\"\n");
|
||||
} else {
|
||||
process.stdout.write(base);
|
||||
}
|
||||
'
|
||||
BASE="$(node -e "$CRED_JS" base)"
|
||||
|
||||
# Repo path from the configured origin remote (never from credentials).
|
||||
REMOTE_URL="$(git remote get-url origin)"
|
||||
REPO_PATH="${REMOTE_URL#https://git.mosaicstack.dev/}"
|
||||
REPO_PATH="${REPO_PATH%.git}"
|
||||
|
||||
# curl config stream: auth header via fd, never argv.
|
||||
gen_curl_cfg() {
|
||||
node -e '
|
||||
const fs = require("fs");
|
||||
const e = JSON.parse(fs.readFileSync(process.env.MOSAIC_GITEA_CREDENTIAL_FILE, "utf8")).mosaicstack || {};
|
||||
process.stdout.write("header = \"Authorization: token " + e.api_token + "\"\n");
|
||||
process.stdout.write("header = \"Content-Type: application/json\"\n");
|
||||
'
|
||||
}
|
||||
# curl config stream: auth header via fd, never argv. The second read
|
||||
# finishes, and must succeed, before curl starts: a refusal inside <(...)
|
||||
# would lose its exit status and leave curl an empty config. A CFG
|
||||
# inherited from the environment keeps its export attribute on assignment,
|
||||
# and SHELLOPTS=allexport exports every assignment, so export -n clears the
|
||||
# attribute before any child starts. printf is a builtin, so
|
||||
# the token is in no argv.
|
||||
CFG="$(node -e "$CRED_JS" cfg)" || exit 3
|
||||
export -n CFG
|
||||
[ -n "$CFG" ] || exit 3
|
||||
|
||||
BODY_FILE=""
|
||||
cleanup() { if [ -n "$BODY_FILE" ]; then rm -f "$BODY_FILE"; fi; }
|
||||
@@ -64,11 +89,11 @@ fi
|
||||
|
||||
URL="$BASE/api/v1/$API_PATH"
|
||||
if [ -n "$BODY_FILE" ]; then
|
||||
HTTP_CODE="$(curl -sS -K <(gen_curl_cfg) -o /tmp/gitea-api-response.$$ \
|
||||
HTTP_CODE="$(curl -sS -K <(printf '%s\n' "$CFG") -o /tmp/gitea-api-response.$$ \
|
||||
-w '%{http_code}' -X "$METHOD" "$URL" --data-binary @"$BODY_FILE")" || {
|
||||
echo "gitea-api: request failed" >&2; exit 1; }
|
||||
else
|
||||
HTTP_CODE="$(curl -sS -K <(gen_curl_cfg) -o /tmp/gitea-api-response.$$ \
|
||||
HTTP_CODE="$(curl -sS -K <(printf '%s\n' "$CFG") -o /tmp/gitea-api-response.$$ \
|
||||
-w '%{http_code}' -X "$METHOD" "$URL")" || {
|
||||
echo "gitea-api: request failed" >&2; exit 1; }
|
||||
fi
|
||||
|
||||
+17
-5
@@ -9,8 +9,10 @@
|
||||
# Once HEAD holds docs/plans/queue.json (the genesis commit), the suite also
|
||||
# runs `verify` and `render --check` on the live queue through
|
||||
# `scripts/mosaic queue`. Each takes the queue lock for a moment and writes
|
||||
# nothing. Before genesis it only checks that `scripts/mosaic queue help`
|
||||
# answers, which reads no queue file and takes no lock.
|
||||
# nothing. Before genesis, or in a checkout that is not the queue's
|
||||
# canonicalRoot (an export or a review clone), it skips them and says why;
|
||||
# `scripts/mosaic queue help` still runs, reading no queue file and taking no
|
||||
# lock.
|
||||
set -uo pipefail
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
@@ -54,13 +56,23 @@ check "node --test packages/queue/tests/" "$rc"
|
||||
# --- the dispatch, then the live queue once genesis is committed ---
|
||||
scripts/mosaic queue help > "$SANDBOX/help.out" 2>&1 && grep -q '^usage: queue list' "$SANDBOX/help.out"
|
||||
check "scripts/mosaic queue help" $?
|
||||
if git cat-file -e HEAD:docs/plans/queue.json 2>/dev/null; then
|
||||
if ! git cat-file -e HEAD:docs/plans/queue.json 2>/dev/null; then
|
||||
echo "skip queue verify and render --check: HEAD has no docs/plans/queue.json (before the genesis commit)"
|
||||
else
|
||||
CANON="$(git show HEAD:docs/plans/queue.json | node -e 'let s="";process.stdin.on("data",(d)=>{s+=d}).on("end",()=>{const r=JSON.parse(s).canonicalRoot;if(typeof r==="string")process.stdout.write(r)})' 2>/dev/null)"
|
||||
TOP="$(realpath "$(git rev-parse --show-toplevel)")"
|
||||
if [ -z "$CANON" ]; then
|
||||
check "HEAD's docs/plans/queue.json names a canonicalRoot" 1
|
||||
elif [ "$TOP" != "$CANON" ]; then
|
||||
# The queue refuses outside its canonical root (8.3), so an export or
|
||||
# a review clone can't run these two. Say so instead of failing them.
|
||||
echo "skip queue verify and render --check: this checkout ($TOP) is not the queue's canonical root ($CANON)"
|
||||
else
|
||||
scripts/mosaic queue verify
|
||||
check "scripts/mosaic queue verify (live queue)" $?
|
||||
scripts/mosaic queue render --check
|
||||
check "scripts/mosaic queue render --check (live queue)" $?
|
||||
else
|
||||
echo "skip queue verify and render --check: HEAD has no docs/plans/queue.json (before the genesis commit)"
|
||||
fi
|
||||
fi
|
||||
|
||||
echo
|
||||
|
||||
Reference in New Issue
Block a user