Skip to content

Answer the unregistered inbox with a foundation code, not a sentence - #289

Merged
pixexid merged 3 commits into
mainfrom
bb/gh-280-typed-unregistered-inbox-refusal-thr_2rbixgj33q
Aug 19, 2026
Merged

pixexid merged 3 commits into
mainfrom
bb/gh-280-typed-unregistered-inbox-refusal-thr_2rbixgj33q

Conversation

@pixexid

@pixexid pixexid commented Aug 19, 2026 •

Copy link
Copy Markdown
Owner

Closes #280
Acceptance: complete
Review tier: A

operatorMessages signalled "this project has no bb-collab inbox" by throwing a bare Error whose sentence the app then string-matched. Since #281 that sentence had two consumers — the panel's error branch and the unread poll's zero-vs-failure decision — so a reword or a localisation would silently have turned a failed read into a proven zero.

The shape, and why

The repo already has a refusal type with codes in src/foundation.ts, and those codes reach the app in the result (FoundationResult.outcome), never on a thrown error. That is not a stylistic choice: the SDK reduces every handler rejection to { code: "handler_error", message: errorMessage(error) }, so structure on the error is destroyed at the boundary. The output value is the only channel a code survives.

So operatorMessages returns { outcome, message?, messages }:

  • outcome is "OK" or "PROJECT_CONFIG_REQUIRED", both already in FoundationCode. as const satisfies readonly FoundationCode[] is the compile-time proof that this is the existing vocabulary and not a parallel scheme.
  • PROJECT_CONFIG_REQUIRED is exactly what foundation already refuses for a missing project_config_heads row — the same query this check runs.
  • message is optional and stays for humans. Nothing branches on it.

Both consumers migrated

  • Panel — branches on result.value.outcome. Aggregate mode skips; scoped mode still shows the refusal, now naming the code.
  • Unread poll — branches on nothing. An answered read is a proven count, and the unregistered outcome carries no rows, so it proves zero without reading a sentence. Removing the branch is a stronger migration than rewriting it.

Proof

tests/sidebar.test.tsx:

  • branches on the outcome code alone, so rewording the human sentence moves nothing — the required test. Same outcome with the old sentence, a French one, and none at all; identical behaviour in aggregate and scoped mode each time. Read in scoped mode deliberately: in aggregate mode "skipped" and "loaded zero rows" are indistinguishable, so an aggregate-only assertion would not have discriminated.
  • treats a rejection carrying the old sentence as the failed read it is — the inverse. The exact sentence now buys nothing.
  • binds the skip to the outcome code the server itself produces — the result is taken from the running server, not authored in the test.

Mutation-checked, each failing only its own target:

mutation fails
branch on result.message instead of outcome branches on the outcome code alone (1 of 48)
re-add a substring message match on the rejection path treats a rejection…, keeps a failure that merely quotes… (2 of 48)

tests/inbox-nav-indicator.test.tsx gains counts the unregistered outcome as a proven zero rather than retaining the last count; the #281 retained-count test is unchanged and still passes.

Unchanged on purpose

What is refused and when: markOperatorMessageRead and replyToOperatorMessage still throw through requireRegisteredInboxProject, and the CLI still errors on an unregistered project and still prints a bare array. The exact registered-project match on operatorMessages is untouched — the reader still returns no rows for a project with no config head.

Other message-string branching

Swept the repo. app.tsx:49 was the only production site; the three remaining message === matches are log-entry assertions in tests/server.test.ts, not control flow. No widening.

npm run verify exit 0 — 370 passed, 2 skipped, 22 files passed / 2 skipped. dist/ regenerated by the builder; dist/*.meta.json not committed.

Review round 2 — P2 remedy

server.ts now returns invalidCli(...) for a non-OK outcome instead of throwing one, and tests/server.test.ts covers the CLI list path, which had no test at all.

The mutant the review found — deleting that refusal — made bb collab inbox --project <unregistered> return exit 0 and []. That is #280's own defect class on the agent-facing surface. The behaviour was correct; nothing proved it stayed correct.

mutation filtered unfiltered
committed 1 passed | 176 skipped (177) 177 passed (177)
refusal deleted 1 failed | 176 skipped (177) — expected +0 to be 2 1 failed | 176 passed (177)
CLI refuses unconditionally 1 failed | 176 skipped (177) — expected 2 to be +0 1 failed | 176 passed (177)

Re-run at the rebased head, not carried across from 6a28e1f. 1 + 176 = 177 in every row, so the filter hit exactly one test; the unfiltered runs show no other test moved either way.

Both halves are asserted: a registered project lists exit 0 as a bare array with the seeded row in it, an unregistered one exits 2 with a refusal. The positive control is not decoration — a green on the refusal alone is compatible with the verb not existing, the flag being ignored, or the seeding never having run.

P3 (message): taken. message is optional on the result, so new Error(listed.message) could construct an empty error; invalidCli(listed.message ?? listed.outcome) drops the throw/catch round trip in the same block and gives a future messageless outcome something to print.

P3 (mutation table): taken — the earlier table now names the substring form.

Rebase onto 42fadcf

origin/main moved twice under review — 91d5d65 → 9dccb7a (#293) → 42fadcf (#288). #288 touched the same inbox surfaces, so server.ts, tests/server.test.ts and dist/server.js conflicted.

server.ts and tests/server.test.ts were merged by hand. The merged reader takes #288's withSenderTitles parameter and returns #280's envelope, with the unregistered early return before title resolution — an unregistered project has no rows to title. dist/server.js was regenerated by the builder, never hand-edited.

Two #288 tests auto-merged without conflict but still assumed the bare-array shape, and had to be reshaped onto { outcome, messages }: keeps inbox messages readable when a sender title cannot be resolved (tests/server.test.ts) and falls back to the secondary sender id when its live title is unavailable (tests/sidebar.test.tsx, routed through the existing okMessages helper). Both are why the evidence was re-run rather than carried forward.

#288's own property holds: the plain list still performs no threads.get, only withSenderTitles: true does, and the unread poll never asks for titles.

npm run verify exit 0 — 373 passed, 2 skipped, 22 files passed / 2 skipped.

pixexid added a commit that referenced this pull request Aug 19, 2026
Review P2 on #289. Moving the refusal into the reader's result meant the CLI
had to refuse on its own account, and that new line had no test — deleting it
left the suite identically green. The mutant returned exit 0 and `[]` for an
unregistered project: #280's own defect class, a refused read presenting as an
empty inbox, on the surface where a seat reads its durable inbox.

The behaviour was already correct; nothing proved it stayed correct. The test
asserts both halves — a registered project lists exit 0 as a bare array with
the seeded row in it, an unregistered one exits 2 with a refusal — because a
green on the refusal alone is also compatible with the verb not existing, the
flag being ignored, or the seeding never having run.

Deleting the refusal fails it (expected +0 to be 2); making the CLI refuse
unconditionally fails it too. Filtered: 1 failed | 175 skipped (176).

Also review P3, same line: `message` is optional on the result, so
`new Error(listed.message)` could construct an empty error. Returning
invalidCli directly with the outcome code as fallback drops the throw and
catch round trip in the same block and gives a future messageless outcome
something to print.
Closes #280

`operatorMessages` now answers `{ outcome, message?, messages }`, where
`outcome` is drawn from the repo's existing `FoundationCode` vocabulary
and `satisfies readonly FoundationCode[]` is the standing proof that no
parallel error scheme was invented at the app boundary. The condition is
the one foundation already refuses as PROJECT_CONFIG_REQUIRED: no row in
`project_config_heads`.

It travels in the result rather than on a thrown error because the SDK
reduces every handler rejection to `{ code: "handler_error", message }`,
so a code carried on the error would not survive the boundary.

Both consumers migrate. The panel branches on the code; the unread poll
stops branching at all, because an answered read is a proven count and
the unregistered outcome carries no rows. The exact registered-project
check, its timing, and the CLI's output and refusal are unchanged.
Review P2 on #289. Moving the refusal into the reader's result meant the CLI
had to refuse on its own account, and that new line had no test — deleting it
left the suite identically green. The mutant returned exit 0 and `[]` for an
unregistered project: #280's own defect class, a refused read presenting as an
empty inbox, on the surface where a seat reads its durable inbox.

The behaviour was already correct; nothing proved it stayed correct. The test
asserts both halves — a registered project lists exit 0 as a bare array with
the seeded row in it, an unregistered one exits 2 with a refusal — because a
green on the refusal alone is also compatible with the verb not existing, the
flag being ignored, or the seeding never having run.

Deleting the refusal fails it (expected +0 to be 2); making the CLI refuse
unconditionally fails it too. Filtered: 1 failed | 175 skipped (176).

Also review P3, same line: `message` is optional on the result, so
`new Error(listed.message)` could construct an empty error. Returning
invalidCli directly with the outcome code as fallback drops the throw and
catch round trip in the same block and gives a future messageless outcome
something to print.
Rebase resolution onto 42fadcf. #288 landed withSenderTitles and sender-title
resolution on the same function this PR reshaped, so three files conflicted.

server.ts and tests/server.test.ts were resolved by hand. listOperatorMessages
keeps #288's async signature, `bb` parameter and default-off withSenderTitles,
and gains the outcome envelope around it; the unregistered early return sits
before the title resolution, which is correct because an unregistered project
has no rows to title. dist/server.js was regenerated by the builder, never
hand-resolved.

Two #288 tests merged without conflict but still assumed the bare array:
the RPC assertion in "keeps inbox messages readable when a sender title cannot
be resolved", and a sidebar mock, which now goes through the existing
okMessages wrapper like its siblings.

#288's own property is preserved and still asserted: the plain list performs no
threads.get, only withSenderTitles does, and the unread poll still does not
request titles.
@pixexid
pixexid force-pushed the bb/gh-280-typed-unregistered-inbox-refusal-thr_2rbixgj33q branch from 6a28e1f to 2d43308 Compare August 19, 2026 03:30
@pixexid
pixexid marked this pull request as ready for review August 19, 2026 03:41
@pixexid
pixexid merged commit b3046c6 into main Aug 19, 2026
2 of 5 checks passed
@pixexid
pixexid deleted the bb/gh-280-typed-unregistered-inbox-refusal-thr_2rbixgj33q branch August 19, 2026 03:48
@github-actions

Copy link
Copy Markdown

Merged PR #289 completed the declared acceptance for #280; the issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inbox: unregistered-project condition needs a typed error, not a prose sentence

1 participant