Files
jason.woltje 158205c5cc lead: commit darkwing's untracked evidence (348 files: queue-137 queue-141 queue-150 queue-162 queue-165 queue-166 queue-167 queue-169 queue-179 queue-180 queue-181 queue-182 queue-185 queue-186 queue-187 queue-188 queue-190 queue-93)
Unclaimed files left in the canonical checkout. Inventory: canonical-cleanup-inventory.md (commit-as-evidence groups). detect-secrets scan clean on 2026-11-02: hits are sha256 pins, git object ids, test sentinels already in tracked tests and unit-file lines.
2026-11-02 15:05:14 -06:00

9.9 KiB

Row 182: a tester message reaches only a tester-owned thread

Brief: "# Brief: a tester message reaches only a tester-owned thread (N1)" in docs/plans/2026-10-20_tester-thread-follow-ups.md (blob 3b4313cb9f, 841c56d05). Gitea #1680. Owner Darkwing, reviewer Filbert, gate owner Sage. The follow-up is Filbert's N1 on row 177 (#1675 comment 29347).

Files

File What
build.patch the change, git diff 841c56d05 over the paths below
candidate-manifest.sha256 one line per changed path
  • Base: 841c56d05 (refactor head at the start, after 931651f2d).
  • Line numbers are at the base unless marked as the candidate's.
  • Changed: packages/bus/src/broker.mjs, packages/bus/tests/tester-threads.test.mjs, packages/bus/README.md, and packages/bus/tests/messaging.test.mjs. The last one is outside the brief's list (next section).

messaging.test.mjs is outside the brief's list

The first slot run failed one test, "a business with no role definitions at all is unlimited" (messaging.test.mjs:203). Its tester leg was:

  const opened = send('assistant', 'pm');
  for (const from of ['pm', 'tester']) send(from, 'assistant', { in_reply_to: opened.id });
  assert.deepEqual(refusals(), []);

The assistant there runs on the human's thread, so the tester's reply reached a human thread through in_reply_to. That is the hole this row closes, and the send now refuses thread-not-owned. The row 182 rule moves the tester send off the human's thread. Per sage-1212 the leg keeps a passing tester send, because the test's point is that a business with no definitions limits no one. The tester makes its own project and thread, an assistant run claims that thread and sends to the pm, and the tester replies to that message. refusals() is still []. The new refusal is covered by tests 1 and 4 below.

No cli, webui or script test sends a tester reply into a threaded instance. The tester clients send no in_reply_to (packages/cli/src, packages/webui/src).

No test pins a hash of the four paths (git grep over packages/*/tests and scripts/test-*.sh). The one test that reads a changed file as text is limit-checks.test.mjs:323, a pattern scan of broker.mjs for limit-check/ and token-limit. It is in the bus suite and passed in run 1.

Correction (Filbert's N2, #1680 comment 29388): the line above is wrong about "the one test". project-runtime.test.mjs:13 also reads broker.mjs as text. It pins the file's import lines. My grep matched it, but I cut the output with head before that line. The candidate adds no import, and the test is in the bus suite, which passed in both runs.

Design (written before the change, per sage-1201)

Where a tester's message gets its thread

A tester message.send (broker.mjs:1823, tester branch :1834-1838) checks in_reply_to only for existence in the business (:1879-1884). It then calls #threadFor(s, to, in_reply_to) (:323). For a threaded instance, #threadFor walks the in_reply_to chain and takes the first thread it reaches, by one of two branches:

  1. A message on the chain is in a thread (:333-336). The check there is thread-instance-mismatch only.
  2. Row 143: a message on the chain is a spec notice from _bus (:339-343). #specThread (:2630) returns the newest opened thread in the notice's project whose owner is the business's human (t.owner=? bound to this.#business(s.business).human, :2637).

Neither branch checks the owner against the sender. With the shipped assistant lists, #checkMessaging (:1859) refuses the tester first (message-sender-refused), so only an assistant definition with no lists reaches either branch.

Can the spec-notice branch return a thread the tester doesn't own?

Yes. #specThread returns only threads owned by the human, never tester, so for a tester sender every thread it returns is one the tester doesn't own. The chain reaches it when the tester replies to a spec notice, or to any message whose chain reaches one. in_reply_to takes any message id in the business, including a notice's. So the spec branch gets the same check.

The check

One check after the walk picks a thread, on both branches: if the sender is the tester (s.tester) and the thread's owner isn't tester, refuse. Comparing with this.#owner(s) (:358, 'tester' for a tester) keeps the rule in the place row 177 put it.

It applies only to s.tester. The brief keeps the reverse direction:

  • A role's reply up a chain into a tester-owned thread is how the chief's answer comes back (test 3).
  • A human sender refuses earlier, at :326 (thread-required-use-thread-send).
  • A thread run returns its own thread at :324 before the walk.

The refusal code: thread-not-owned

I reuse the existing code rather than add one.

  • It names the same rule. REQ-USER-2 at #thread (:353) refuses thread-not-owned when a session reaches a thread another owner made. Row 177 extended it to the tester (#owner, :358), so the tester's thread.open, thread.send and thread.messages on a human thread already answer thread-not-owned (tester-threads.test.mjs:84). A tester message whose chain lands in a human thread breaks the same rule.
  • Every client already maps it. The webui tester listener answers 403 with "This thread is not available to this user." (packages/webui/src/chat-http.mjs:15,23). The cli prints the code (packages/cli/src/thread.mjs:60). A new code would need a new mapping in each, or fall through as an unmapped error.
  • The cost is that the tester didn't name the thread here; the chain did. The README paragraph says so, so a tester who sees the code on a message.send knows to look at in_reply_to.

Rejected alternatives:

  • A new code such as tester-thread-not-owned. That is more specific, but it needs a mapping in each client and adds a second name for one rule.
  • Skip the branch and walk on past a thread the tester doesn't own. The tester's message would then fall through to thread-required-reply-to-thread-message or land in a thread further up the chain, and the test would get a misleading answer.

Tests

The tests go in packages/bus/tests/tester-threads.test.mjs (row 177's file). All use a business whose assistant definition has no lists (messaging null), so #checkMessaging lets the tester through.

  1. The tester sends to the assistant in reply to a message in the human's thread H. The send refuses thread-not-owned. It writes no messages row and no message_threads row, and the human's thread.messages on H reads the same before and after.
  2. The same tester send in reply to a message in the tester's thread T lands in T.
  3. The round trip: the tester's thread.send in T, the T run's message to the chief, the chief's reply to the assistant. The chief's reply lands in T, and the T run receives it.
  4. The spec branch: the tester replies to a spec notice in the project of a human thread. It refuses thread-not-owned and writes nothing. The test writes the notice row the way #specNotice (:2588) does, since the save path needs the git project fixture of spec-review.test.mjs, and that fixture can't turn on bus.tester (the broker takes a structuredClone of the businesses, :208). On the same chain, the chief's relay to the assistant lands in the human's thread, which shows that the fixture notice reaches the branch.

Test 1 runs a holder on H. Without the check, the tester's message would be stored in H and delivered to H's run, not refused tester-target-not-running. Test 3 is row 177's round trip with the chief added as a plain run. The assistant run in T sends to the chief, and the chief's reply to that message lands in T and reaches the T run.

Mutant: drop the owner check. Tests 1 and 4 should kill it.

Runs

run182.sh runs the whole bus suite (packages/bus, node --test tests/*.test.mjs) and then three mutants against tester-threads.test.mjs with --test-name-pattern='row 182'. Each mutant edits broker.mjs after a checked backup and puts it back by the backup's sha. TMPDIR and output are in my scratch.

Run 1, slot from sage-1209, 2026-10-20T05:09:08.203Z to 05:09:26.237Z:

Run Result
bus suite 451 tests, 450 pass, 1 fail (messaging.test.mjs:203, above)
m1: the #testerOwns check line removed killed
m2: the thread branch returns m.thread unchecked killed
m3: the spec branch returns spec unchecked killed

Run 2, after the messaging.test.mjs change, slot from sage-1215, 2026-10-20T05:24:47.741Z to 05:25:04.691Z:

Run Result Failing tests
bus suite 451 tests, 451 pass, 0 fail none
m1: the #testerOwns check line removed killed T9 tests 1 and 4
m2: the thread branch returns m.thread unchecked killed test 1
m3: the spec branch returns spec unchecked killed test 4

broker.mjs was back at its candidate sha after the mutants. Tests 2 and 3 pass under every mutant, since they check that the tester's own thread and the chief's reply still land.

Candidate

  • Base 841c56d05. Nothing in 841c56d05..origin/refactor (d1272e714 at 05:25Z) touches the four paths, and build.patch applies cleanly to both 841c56d05 and d1272e714 (git apply --cached --check on a scratch index).
  • build.patch: sha256 c050948ebdf6e8d6e075b897bf21c7a7cfbffef699ca15e54a45a02a3e1a16f5, patch-id (stable) fa9a8988ebfcbe3cb84beeff9d74dead8ea002e1, 4 files, 88 insertions, 6 deletions.
  • candidate-manifest.sha256: sha256 9b5cc63c37f57a7a9954d6d3ecd1c5ee23a014b003c653d5ff7c0e055d57e866.
Path sha256
packages/bus/src/broker.mjs 01b5ab3a175f9488b31554a44abeba9b455dedbc3d68cb2dc8147a3fdcc58976
packages/bus/tests/tester-threads.test.mjs c09a0798cbc297aa5c0bf52bffafa278582c9be6815737d2d4cace56dd7d618f
packages/bus/README.md 4377cc32aed37fa1bb645c0bbc3340eed4ec01740e67327449949474e35d3fdb
packages/bus/tests/messaging.test.mjs c0767449e3f50d74b0ce193fdd2ffeaf123e486d49e03cdf7fddbaec0fb87de7