darkwing: row 60 round 2 review, approve, comment 27575 (#1549)

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-11 19:36:08 -05:00
parent 74e6be7067
commit a030b895fc
4 changed files with 188 additions and 0 deletions
@@ -0,0 +1,40 @@
+#!/bin/bash
+# Darkwing's own row 60 mutants. Each edits one source file in the review worktree, runs one
+# test file (or the bus suite), then restores the file from the worktree index (the candidate).
+export TMPDIR=~/darkwing-scratch/rv60r2/tmp
+cd ~/darkwing-scratch/rv60r2/wt
+M=~/darkwing-scratch/rv59/mut.mjs
+B=packages/bus/src/broker.mjs; R=packages/bus/src/runtime.mjs; V=packages/bus/src/views.mjs; S=packages/webui/src/serve.mjs
+BT=packages/bus/tests/credential-status.test.mjs; WE=packages/webui/tests/bus.test.mjs; ALL='packages/bus/tests/*.test.mjs'
+run() { # name file from to test
+  node $M "$2" "$3" "$4" || { echo "$1: MUTATION DID NOT APPLY"; return; }
+  env -u NODE_TEST_CONTEXT node --test --test-timeout=60000 "$5" > $TMPDIR/mut-$1.log 2>&1
+  echo "$1: rc=$? $(grep -E '^ℹ (pass|fail) ' $TMPDIR/mut-$1.log | tr '\n' ' ')"
+  grep -E '^✖ ' $TMPDIR/mut-$1.log | sed 's/ ([0-9.]*ms)$//; s/^/    /' | sort -u | head -4
+  git checkout -q -- "$2"
+}
+run reader-refused $B "'trail', 'decisions', 'credentials'].includes" "'trail', 'decisions'].includes" $BT
+run role-allowed $B "if (!s.human && !s.reader) fail('human-required');
+        keys(a, [], []);" "keys(a, [], []);" $BT
+run args-accepted $B "fail('human-required');
+        keys(a, [], []);" "fail('human-required');" $BT
+run prefix-no-slash $B "const prefix = s.business + '/';" "const prefix = s.business;" $BT
+run no-filter $B '.filter((c) => c.instance.startsWith(prefix))' '.filter(() => true)' $BT
+run sync-label-raw $B "role: sync ? 'tracker sync' : role," "role," $BT
+run sync-no-bot $B 'sync ? b.tracker?.sync?.bot :' 'sync ? null :' $BT
+run gitea-gets-bot $B "service === 'vikunja' ? b.roles?.[role]?.tracker?.bot : null" 'b.roles?.[role]?.tracker?.bot' $BT
+run datekind-fixed $B "dateKind: service === 'gitea' ? 'rotateBy' : 'expires'," "dateKind: 'expires'," $BT
+run account-undefined $B 'account: bot ?? null,' 'account: bot,' $BT
+run leak-instance $B "            return {
+              service," "            return {
+              instance,
+              service," $BT
+run status-cached $B 'this.#credentialStatus = credentialStatus;' 'const cached = credentialStatus(); this.#credentialStatus = () => cached;' $BT
+run no-result-check $B "const result = this.#dispatch(s, request.verb, args);
+        this.#secretCheck(result);" "const result = this.#dispatch(s, request.verb, args);" $BT
+run runtime-no-status $R "      credentialStatus: () => credentials.status(),
+" '' $BT
+run views-no-verb $V "    credentials: () => client.call('credentials'),
+" '' $WE
+run serve-no-verb $S "'decisions', 'credentials'];" "'decisions'];" $WE
+git diff --quiet -- $B $R $V $S && echo "restored: sources match the candidate index" || echo "NOT RESTORED"
@@ -0,0 +1,57 @@
+reader-refused: rc=1 ℹ pass 0 ℹ fail 4 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ credentials reads the status on each request, and a broker without one lists none
+    ✖ credentials refuses a role session and any argument
+    ✖ failing tests:
+role-allowed: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials refuses a role session and any argument
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+args-accepted: rc=1 ℹ pass 3 ℹ fail 1 
+    ✖ credentials refuses a role session and any argument
+    ✖ failing tests:
+prefix-no-slash: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+no-filter: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+sync-label-raw: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+sync-no-bot: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+gitea-gets-bot: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+datekind-fixed: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+account-undefined: rc=0 ℹ pass 4 ℹ fail 0 
+leak-instance: rc=1 ℹ pass 2 ℹ fail 2 
+    ✖ credentials gives human and reader sessions service, account, role, date and state, and nothing else
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+status-cached: rc=1 ℹ pass 3 ℹ fail 1 
+    ✖ credentials reads the status on each request, and a broker without one lists none
+    ✖ failing tests:
+no-result-check: rc=0 ℹ pass 4 ℹ fail 0 
+runtime-no-status: rc=1 ℹ pass 3 ℹ fail 1 
+    ✖ failing tests:
+    ✖ the broker process answers credentials without a token, path or variable in its output, log or store
+views-no-verb: rc=1 ℹ pass 7 ℹ fail 2 
+    ✖ busReader takes --business, else the live bus host, else refuses
+    ✖ credentials route serves the broker credential status from the reader view
+    ✖ failing tests:
+serve-no-verb: rc=1 ℹ pass 7 ℹ fail 2 
+    ✖ bus routes are GET-only, have no write verb, and keep the same-origin checks
+    ✖ credentials route serves the broker credential status from the reader view
+    ✖ failing tests:
+restored: sources match the candidate index
@@ -0,0 +1,2 @@
+packages/bus/tests/credential-status.test.mjs rc=0 ℹ pass 4 ℹ fail 0 
+packages/webui/tests/bus.test.mjs rc=0 ℹ pass 9 ℹ fail 0 
@@ -0,0 +1,89 @@
+# Row 60, bus read: credential status on the broker, round 2 review (Darkwing)
+
+Issue #1549, request comment 27553, packet `agents/filbert/work/queue-60/`
+(commit `5fc7635c`). Base `bbd22f1b`. Candidate manifest sha256
+`13541657c830ed59281f101fc3566924454fd96d1be01df844100544b10cbf93`, 9
+files, matching the request. Scope: Sage's rev 569.
+
+Verdict: **approve**. The rebase changes no code. The `decisions` row is
+right, and Dewey's N1 sentence is right where my round 1 answer to question
+1 was wrong; the correction is below. Nothing blocks.
+
+## Method
+
+- A detached worktree at `bbd22f1b` with `git apply --index build.patch`,
+  then `sha256sum -c` on the manifest: 9 OK. None of the 9 paths changed
+  from `bbd22f1b` to `HEAD` at the time of review.
+- I compared the added and removed lines of round 1's `build.patch`
+  (`5fc7635c^`) with round 2's. They match except for the two
+  `packages/bus/README.md` edits: the new `decisions` table row and the
+  rewritten last sentences of the `credentials` paragraph.
+- I read the merged files where the patch meets rows 58 and 74.
+- The credential and WebUI bus test files, then my 16 round 1 mutants on the new tree
+  (`r2/mutants.sh`, one test file each), with `views-no-verb` pointed at the WebUI bus tests
+  this time.
+
+Node v26.8.1, `TMPDIR=~/darkwing-scratch/rv60r2/tmp`.
+
+## Tests and mutants
+
+On the candidate, 00:35:31Z to 00:35:46Z on 2026-10-12:
+`packages/bus/tests/credential-status.test.mjs` 4 pass, 0 fail;
+`packages/webui/tests/bus.test.mjs` 9 pass, 0 fail (`r2/tests-light.txt`).
+I didn't run the full bus and WebUI suites. Another seat's mutant run held
+the machine, the code is round 1's, and Sage's gate rerun is running the
+full set now.
+
+The same 16 mutants as round 1 (`r2/mutants.txt`): 14 fail, each on the
+test meant for it, and the same two survive for round 1's reasons
+(`account-undefined` is equivalent under the business loader,
+`no-result-check` removes a base line). `views-no-verb` fails 2 WebUI bus
+tests on the first run now that the script points at them.
+
+## The merged hunks
+
+- `serve.mjs`: the patch changes only the `BUS_VERBS` line and its comment.
+  Row 74's checks run in the request handler at lines 107 to 135, and the
+  bus route lookup is at line 160, after them. So in preview mode
+  `/api/bus/credentials` gets the same Host, Origin, sign-in and read-only
+  checks as every other bus read. A GET from the operator's loopback path
+  reaches it as before.
+- `broker.mjs`: the `credentials` case sits before `default:`. Row 58's
+  change is in the `tasks` branch, and the reader allowlist and the
+  constructor are lines row 58 doesn't touch.
+- `webui/tests/bus.test.mjs` and `webui/README.md`: the round 1 additions
+  sit after row 58's and row 74's, sharing no lines.
+
+## The two README edits
+
+**The `decisions` row.** It matches the broker. `keys(a, ['list'],
+['list'])` requires `list`. `roles` returns open decisions not routed to
+`human`, and `recent` returns the last `RECENT` closed ones, which is 50.
+The row says human and reader sessions only, which is what the broker
+checks.
+
+**Dewey's N1.** The new sentences are correct. `Credentials.status()` maps
+over the tokens the host loaded at startup, and the date comes from that
+load; only `state` is computed at the request. The broker takes `account`
+from its businesses, also loaded at startup. So the read and the Business
+view differ only when the business file changes after startup.
+
+## Correction to my round 1 review
+
+In round 1 I answered question 1 by saying this read shows "a token file
+rotated after startup, which the host doesn't reread until restart". That
+was wrong. A token rotation without a date edit changes neither view, which
+is what the README now says. The case a side-by-side view would catch is a
+business file edited after startup: a new date, account or credential that
+the host won't load until it restarts. My answer itself stands, to leave the
+Business view alone in this row, and a later row could still compare the
+two. The reason is that case, not rotation.
+
+I also take back round 1's note 2. `test-release` prints 4 without Docker
+and 14 with it. My round 1 gate had Docker for `test-release`, and
+Filbert's didn't. `build.md` now says so.
+
+## Dewey's N2
+
+Decision 98 rules it: the read ships as built behind the one-user preview.
+I have nothing to add.