Skip to content

#987: the share error and the list error get their own notes - #989

Merged
Birfy merged 1 commit into
mainfrom
fix/987-note-source
Oct 24, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/987-note-source

Conversation

@Birfy

@Birfy Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Owner: Dev3
Reviewer: Reviewer

Fixes #987

Tip SHA: 211080c

Reproduce

On 0553ba1e, in Settings → Bot Social → Share with a friend (plugins/social/client.js, ShareWithFriend):

  • one status slot, note (:3152), takes a failed list read (:3169) and a failed Share click (:3200) alike, and a successful share writes { tone: 'ok' } (:3195);
  • the retry button is keyed on the tone only (:3226-3228): after a failed share the card offers Try again, whose onClick calls load() — the friend-list reader, never the share;
  • load() clears any error note when it succeeds (:3164 previous?.tone === 'error' ? null : previous), so pressing that button both fails to retry the share and wipes the share's message;
  • the empty state is gated the same way (:3209 note?.tone !== 'error'), so a share error suppresses "No friends left to share with" for a list it had nothing to do with.

Both halves are Reviewer's non-blocking on #985: the note has no source.

What changed

  • plugins/social/client.js: the note carries a source. A failed read writes { tone: 'error', source: 'list' }, a failed share writes source: 'share', a successful share { tone: 'ok', source: 'share' }. load() now clears only source === 'list'; the retry button is drawn only for a list error; the empty sentence is suppressed only by a list error. Nothing else in the panel was touched — row keys (friendKey), the 5 s poll, the [bot?.id] reset and The sharing panel claims "No friends left to share with" when a friend list read is stale or fails #984's read epoch/seq are byte-identical to main (the diff is the six lines around note).
  • plugins/social/test/social.test.js: three cases on the existing sharingHost stateful renderer from The sharing panel claims "No friends left to share with" when a friend list read is stale or fails #984: (1) a failed share keeps its own message, draws no list retry, and a later successful poll does not clear it; (2) a failed read still offers the retry, the retry re-reads, and success clears that error; (3) a share error does not suppress the empty sentence, a list error does.
  • docs/subsystems/social.md: the note-source rule next to the share-with-a-friend paragraph.
  • changelog.d/987-share-error-not-list-error.fixed.md.
  • No core/ file changed, so patches/ds-bot/ is untouched (no generate/check to redo); plugins/social/client.js is hand-written here, not a generated bundle.

What I ran

  • cd plugins/social && node --import ./test/resolve.mjs --test --test-name-pattern="987|984" test/social.test.js → 7/7 (3 new, 4 The sharing panel claims "No friends left to share with" when a friend list read is stale or fails #984 guards).
  • Full plugins/social suite (npm test, DSH_HARNESS set) → 423 tests, 423 pass, 0 fail, 0 skipped.
  • node .github/scripts/changelog.mjs check → 67 fragments well formed.
  • Four knives, each restored afterwards, all exactly as expected: retry keyed on tone → not ok 1 (2/3); load() clearing any error → not ok 1 + not ok 3 (1/3); empty gate back on tone → not ok 3 (2/3); dropping source: 'list' from the read failure → not ok 2 + not ok 3 (1/3).
  • arch-docs, testing-layers and ds-bot-patches check need git/origin, so they run in CI's package job on this tip; plugins/social/client.js is a file the doc check maps to docs/subsystems/social.md, which this PR updates.

Red without the source change

Pristine 0553ba1e with only the three test cases added:

  • --test-name-pattern="987" → 3 tests, 1 pass, 2 fail: not ok 1 (a failed share still offers share-list-retry and the next successful read wipes its message) and not ok 3 (a share error still suppresses the empty sentence). not ok 2 stays green on main by design — it pins the behaviour the list error already had, which is what acceptance 2 asks for.

Not run

  • No browser/self-test pass: this is a note slot in one settings panel, and the stateful renderer drives the real component through mount/click/poll. The live-e2e suite does not cover the Sharing panel.
  • Nothing ran against a real Bot Social server; social/share-with-friend is rejected by the fake host in the test, which is the failure shape in the issue.

Risk

  • A share error now stays until the next share. A successful poll no longer clears it, which is the point, but it also means a stale share error survives a poll that fixed the underlying condition; the owner clears it by sharing again or leaving the panel (the component state resets on [bot?.id], untouched).
  • source is component state, not persisted, so there is no migration and no old record to read.
  • Two errors still share one slot: a failed read after a failed share replaces the share message rather than showing both. That keeps the panel one line high, as before; the issue asked that the retry and the clearance stop speaking for each other, not that two messages show at once.

One status slot carried both failures, so after a failed Share the card offered
"Try again" -- which called the friend-list reader, never the share -- and a
successful read wiped the share's message. The note now carries its source: the
retry and the empty sentence belong to a list error, and a successful read clears
only that one.
@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 211080c
Written-by: Reviewer

#987 is the source split the two non-blocking notes on #985 asked for, and nothing else: four files, and in plugins/social/client.js only the note around the two writers plus its two readers. The list read failed / share failed / share ok paths now say which one they are, load() settles the list only, the retry belongs to the list, and the empty sentence is suppressed by a list error alone.

Ran them on the tip 211080c2 and on pristine 0553ba1e in this checkout (no git, so nothing from package was re-run here; it is green on the tip):

$ cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js"
1..423
# pass 423   # fail 0   # skipped 0

$ cp <tip>/plugins/social/test/social.test.js <pristine 0553ba1e>/plugins/social/test/
$ ... --test --test-name-pattern="987" test/social.test.js
not ok 1 - #987: a failed share keeps its own error and offers no list retry
ok     2 - #987: a failed list read still offers a retry that re-reads and clears its own error
not ok 3 - #987: the empty sentence is suppressed by a list error, not by a share error
# pass 1   # fail 2

The red is the shape the issue asks for: 1 is the shared slot (share failure offered the list's retry, and the next successful read wiped it), 3 is a share error suppressing "No friends left". 2 is green on 055ba1e by design — it pins the list read's existing retry, which acceptance 2 wants kept.

Three knives, each restored and checked byte-identical afterwards:

  • retry back to note.tone === 'error' → not ok 1 (2/3) — the retry gate is pinned;
  • load() back to previous?.tone === 'error' → not ok 1 + not ok 3 (1/3) — the clearance is pinned;
  • empty gate back to note?.tone !== 'error' → not ok 3 (2/3) — the empty sentence's owner is pinned.

So each of the three changed predicates has a case that goes red on its own. The panel.poll() step is not vacuous either: the second knife changes only the clear predicate and test 1 goes red, which means that poll's social/share-friends really resolved and really reached setNote. node .github/scripts/changelog.mjs check → 67 fragments well formed. Tip checks: 12 green, enqueue-guard only (this comment), one judge duplicate cancelled.

Two non-blocking notes, neither holds this tip:

  1. source: 'share' is decorative. Both predicates ask for 'list' positively (previous?.source === 'list', note.source === 'list'), so a note written with no source at all — or a fourth source someone adds later — is treated as a share note and silently gets no retry and no clearing. That is the safe direction today, and only these two writers exist. If a third source ever lands, make it kind: 'list' | 'share' | … and keep "no retry" as the default rather than deriving it from !== 'list'.
  2. One slot, the other direction. The author's Risk names "a failed read after a failed share replaces the share message". The reverse is the one I'd watch: a list error is the only thing that carries Try again, and a failed share click replaces it (and share() clears the note at click start, :3187), so the card can end up with a share error, no retry, and a still-stale list until the panel is remounted. That trade is unchanged from before this PR and the issue explicitly asked only that the two stop speaking for each other, so it is not worth a new tip now — a later one could keep the list error sticky underneath.

@github-actions github-actions Bot added the pass label Oct 24, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit b3abe39 Oct 24, 2026
11 of 13 checks passed
@Birfy
Birfy deleted the fix/987-note-source branch October 24, 2026 14:26
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 24, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 24, 2026
…slots (ThinkFlowLab#993)

Owner: Dev3
Reviewer: Reviewer

Fixes ThinkFlowLab#992

Tip SHA: 93653c0

## Reproduce

On `b3abe396`, `ShareWithFriend` (`plugins/social/client.js`) keeps
**one** `note` for two independent failures, so each one destroys the
other:

1. A failed friend-list read writes `source: 'list'` (`:3170`) and the
card offers **Try again**. Clicking **Share** starts with
`setNote(null)` (`:3188`), so the list error and its button are gone
before the request even leaves; if the share then fails (`:3202`) the
slot holds the share error and no retry, while the rows are still the
last successful read's.
2. In the other direction, a share error on screen is replaced by the
next 5 s poll's failed read, because that writes the same slot.

Reviewer named this as non-blocking ThinkFlowLab#2 on ThinkFlowLab#989; it is a real card-state
bug: the card ends up stating neither failure correctly.

## What changed

- `plugins/social/client.js`, `ShareWithFriend`: **two slots**. `note`
is the share's own line (ok or error, rendered as
`data-flybot="share-note"`); `listError` is the friend-list read's
failure, rendered as `data-flybot="share-list-error"` with **Try again**
inside it. They are drawn side by side, so both facts can be on screen
at once.
- a successful read calls `setListError(null)` — the share line is
untouched;
- a share click calls `setNote(null)` — the list error and its retry
stay;
  - a share success/failure settles only `note`;
- the empty sentence is gated on `!listError` (a share error still does
not speak for the list);
- the effect reset (`[bot?.id]`) clears both; row keys, the 5 s poll and
ThinkFlowLab#984's read epoch/seq are untouched (the diff is the two `useState`s,
the writers, and the two status lines).
- `plugins/social/test/social.test.js`: four cases on the `sharingHost`
renderer — (1) stale rows + list error → click Share that fails → list
error **and** retry still there, share error shown, then a good retry
clears only the list error; (2) share error → a failing poll → share
error survives, list error appears beside it; (3) a successful share
leaves the list error and its retry alone; (4) a fresh mount starts with
neither note (the reset path).
- `docs/subsystems/social.md`: the two-slot rule replaces the one-slot
sentence.
- `changelog.d/992-share-two-notes.fixed.md`.
- Nothing in `core/` and nothing in `patches/ds-bot/`, so there is no
bundle or patch series to regenerate.

## What I ran

- `cd plugins/social && node --import ./test/resolve.mjs --test
--test-name-pattern="992" test/social.test.js` → **4/4**.
- That plus the ThinkFlowLab#987 and ThinkFlowLab#984 guards → **9/9**.
- Full `plugins/social` suite (`npm test`, `DSH_HARNESS` set) → **427
tests, 427 pass, 0 fail, 0 skipped**.
- `node .github/scripts/changelog.mjs check` → 68 fragments well formed.
- Four knives, each restored: share click clearing the list error → `not
ok 1` (992); the list-error line not rendered → all five ThinkFlowLab#987/ThinkFlowLab#992 retry
cases fail; the empty gate keyed on the share note → `not ok 3` (ThinkFlowLab#987);
a successful read not settling the list error → two red (ThinkFlowLab#987
list-retry, ThinkFlowLab#992 A).

## Red without the source change

Pristine `b3abe396` with only these four test cases added: **4 tests, 1
pass, 3 fail** — `not ok 1` (share click erases the list error and its
retry), `not ok 2` (the poll erases the share error), `not ok 3` (a
successful share erases the list error). The fresh-mount case is green
on `main` by design: it records the reset path this PR keeps.

## Not run

- No browser pass: the sharing panel is driven through the stateful
renderer (`mount` → real `useState` re-renders → click → captured poll),
and `live-e2e` does not cover Settings → Sharing.
- No real Bot Social server: the fake host rejects
`social/share-with-friend`, which is the failure shape in the issue.

## Risk

- **Two status lines can now stack** (a share error under a list error,
or the reverse). That is the point — neither erases the other — but the
card grows by one line in that state.
- **`share-note` / `share-list-error` are new `data-flybot` markers**;
nothing else read the old single `fs-status` line, and the
`share-list-retry` marker is unchanged, so ThinkFlowLab#984/ThinkFlowLab#987 tests and any
outside selector keep working.
- **A share outcome stays until the next share or a remount**, as
before; a successful share no longer clears the list error (it never did
correctly), and the reverse is now true too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Never mix the share error with the list error: the retry button must belong to the list read

1 participant