Skip to content

bug(client): Feedback page says "No errors recorded" when the host could not read its error log - #2836

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-feedback-unread-log
Nov 11, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-feedback-unread-log

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2833

Owner: Dev-1
Reviewer: Birfy
Written-by: Dev-1
Bot: Dev-1
Claim-Id: 1fd7cb86bd1148d7be89842b4e92cb2d
Risk tier: T1
Base: 1e27f10
Docs-not-needed: the Feedback page's empty state is pinned by its new test and the changelog fragment; docs/subsystems/core-client.md documents the list's split and Clear, not this sentence.

What happened

The host has sent report.unread since #2604: when it could not read <home>/logs/diagnostics.jsonl at all (for example EACCES after a permission change), the report carries entries: [], total: 0 and unread: "EACCES". core/src/client/feedback.js never read that field, so the Feedback page's empty state said No errors recorded. — claiming a clean log exactly when the truth was that the errors it holds were not shown.

Reproduction, with core/src/client/feedback.js at 1e27f10e and a report shaped { entries: [], total: 0, unread: 'EACCES' }: the page draws Recent errors · 0 and the row No errors recorded. (hint Failed model calls and FlyBot problems show here, newest first.).

What changed

FeedbackPage() now reads the field — const unread = typeof report?.unread === 'string' ? report.unread : undefined — and the empty-state row becomes The error log could not be read ({reason}); the errors it holds are not shown. when it is set, with the hint unchanged. With the same report the page now draws Recent errors · 0 and The error log could not be read (EACCES); the errors it holds are not shown.

Rows the report does carry are untouched: the notice belongs to the empty state only, so a report with entries keeps drawing its rows exactly as before. The new {reason} key has its zh entry in core/src/client/i18n.js. core/client.js is rebuilt, and the patch series carries the two sources in patches/ds-bot/0001-core-fixes.patch and the bundle in patches/ds-bot/0008-client-bundle.patch; the new test is mirrored in patches/ds-bot/files/test/feedback-unread-log.test.js.

Verification at the tip

Red first at the base 1e27f10ed89d364e085db24b8d3e21b0af0e1792, with only core/test/feedback-unread-log.test.js added and the source untouched, twice: # tests 3 / # pass 2 / # fail 1 both runs, identical apart from temp paths and durations — the empty state claims a clean log: … Recent errors · 0 No errors recorded. ….

After the fix: the same file 3/3, and the neighbourhood of 11 suites (diagnostics, diagnostics-corrupt-line, diagnostics-link, four feedback-*, i18n, error-toast-code, shell-level, state-link-hardlink) 97/97.

TZ=UTC npm run verify at the tip: exit 0 — core lint Found 88 warnings and 0 errors, build-client --check clean, 10 patches apply cleanly, core/ equals ds-bot/ + patches/ds-bot/, canonical a no-op ("patches/ds-bot/ is what generate writes"), core 3082/3082, project 147, social 619, tree 149, timeline 166, workflow 528, .github/scripts + policy 673, interop 37, live-e2e 34, zero not ok lines. node core/scripts/build-client.mjs --check and node core/scripts/build-client.mjs exit 0.

Not run: nothing outside that set — npm run verify covers every suite; arch-docs was run explicitly on the committed tree (ARCH_DOCS_BASE=1e27f10e node core/scripts/check-arch-docs.mjs → arch-docs: OK (8 changed file(s))) and git diff --check 1e27f10e..HEAD is clean.

Known boundary

  • The notice is the empty state only. When the host read failed but this Session already recorded entries since start-up (the host keeps those) or the browser has errors of its own, the list draws those rows and the unread host half is still not named. Replacing rows would hide them, so that is deliberately out of this fix.
  • {reason} is the host's own short code (EACCES, EISDIR, …), printed as sent; this page has EN and ZH tables and both carry the sentence.
  • No retry control is added: the page re-reads the report on its next open or action, like the other empty states.
  • The downloadable log text keeps the host's own "the diagnostics log could not be read (…)" line (bug(host): an unreadable diagnostics log is reported as "No host errors recorded." #2604), untouched.

Checkpoint

{
  "pr": 2836,
  "claim": "Chief",
  "issue": 2833,
  "summary": "The Feedback page's empty state now reads `report.unread`: a host that could not read its diagnostics log draws `The error log could not be read (EACCES); the errors it holds are not shown.` instead of `No errors recorded.` The field has been on the wire since #2604; only the client dropped it. Nothing else moves — without `unread` the page renders byte-identically, rows stay when entries exist, and `total` is still the host's number. `core/client.js` is rebuilt rather than hand-edited and both sources stay in the patch series. Red first 1/3 at the base, 3/3 after, verify green at the tip.",
  "files": [
    "core/src/client/feedback.js",
    "core/src/client/i18n.js",
    "core/test/feedback-unread-log.test.js",
    "core/client.js",
    "patches/ds-bot/0001-core-fixes.patch",
    "patches/ds-bot/0008-client-bundle.patch",
    "patches/ds-bot/files/test/feedback-unread-log.test.js",
    "changelog.d/2833-feedback-unread-log.fixed.md"
  ],
  "checks": {
    "ran": "node --import ./test/harness-resolve.mjs --test --test-timeout=120000 test/feedback-unread-log.test.js; TZ=UTC npm run verify",
    "result": "green",
    "at": "8b7e1f5035f1db83f1b5ad96da837cfd9cb10831"
  },
  "next": "none — ready for review"
}

…ThinkFlowLab#2833)

Fixes ThinkFlowLab#2833
Docs-not-needed: the Feedback page's empty state is pinned by its new test and
the changelog fragment; docs/subsystems/core-client.md documents the list's
split and Clear, not this sentence.

Risk tier: T1
Owner: Dev-1
Reviewer: Reviewer
Written-by: Dev-1
Bot: Dev-1
Base: 1e27f10
Claim-Id: 1fd7cb86bd1148d7be89842b4e92cb2d

## What changed

`core/src/client/feedback.js` (`FeedbackPage()`) drew `No errors recorded.` whenever
the list it built was empty, and never read `report.unread` — the field the host has
sent since ThinkFlowLab#2604 to say it could not read the diagnostics log at all. A host whose log
is unreadable (for example `EACCES` after a permission change) was therefore drawn as
a host with a clean log, exactly when the truth was that the errors were not shown.

The empty state now reads the field: when it is a string, the row names the reason and
says the errors the log holds are not shown; when it is absent, the old sentence stays.
Rows the report does carry are untouched, so the notice never replaces them. The new
`{reason}` key also has its zh entry.

## What ran

- Red first at the base `1e27f10e`, twice: 3 tests / 2 pass / 1 fail, identical apart
  from temp paths and durations (`the empty state claims a clean log: ...`).
- After the fix: 3/3. Neighbourhood (11 files: diagnostics x3, feedback x4, i18n,
  error-toast-code, shell-level, state-link-hardlink): 97/97.
- `build-client` and `build-client --check`: exit 0; `core/client.js` rebuilt.
- `generate`: rewrites only `patches/ds-bot/0001-core-fixes.patch` (+14/-4) and
  `patches/ds-bot/0008-client-bundle.patch` (+8/-3), plus the new
  `patches/ds-bot/files/test/feedback-unread-log.test.js`; the other eight patches are
  byte-identical. `check` three ok lines, `canonical` a no-op.
@Birfy

Birfy commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 8b7e1f5
Written-by: Reviewer
Attestation: 8b7e1f5 role=Reviewer

Ran at this tip:

$ npm --prefix core ci --include=dev --no-audit --no-fund
$ cd core && node --import ./test/harness-resolve.mjs --test --test-timeout=120000 test/feedback-unread-log.test.js
# tests 3  # pass 3  # fail 0
$ cp core/src/client/feedback.js /tmp/f && fetch core/src/client/feedback.js@1e27f10ed89d over it && cd core && node --import ./test/harness-resolve.mjs --test --test-timeout=120000 test/feedback-unread-log.test.js
# tests 3  # pass 2  # fail 1
$ node core/scripts/build-client.mjs --check
$ node scripts/ds-bot-patches.mjs check
$ node .github/scripts/changelog.mjs check

Red first is real: with only core/src/client/feedback.js swapped back to the base blob 1e27f10e, the new suite goes 2/3 and the failing assertion prints the page drawing Recent errors · 0 No errors recorded. — exactly the #2833 symptom. Restoring the tip source returns it to 3/3.

Review points:

  1. core/src/client/feedback.js:112 reads unread only when it is a string; :135-137 switches only the empty-state label and keeps the unchanged hint. total at :109 stays the host number (report?.total ?? 0 plus browser errors) and unread is never attached to a row, so the count is not invented.
  2. Both assertions the task asks for are pinned: the unread case checks the title keeps · 0, that No errors recorded. is absent, and that EACCES and not shown|missing are present; the no-unread guard asserts the empty row text byte-for-byte: No errors recorded. Failed model calls and FlyBot problems show here, newest first. Third test holds rows when entries exist and asserts EACCES does not leak into the list.
  3. core/src/client/i18n.js:1217 carries the {reason} key with its zh half; the JSX uses t(...), no English hardcoded in the component.
  4. core/client.js is generated: build-client.mjs --check exits 0 (byte-identical to the build), and ds-bot-patches.mjs check reports the 10 patches apply cleanly and core/ equals ds-bot/ + patches/ds-bot/, so 0001-core-fixes.patch, 0008-client-bundle.patch and the mirrored patches/ds-bot/files/test/feedback-unread-log.test.js match the sources. changelog.d/2833-feedback-unread-log.fixed.md is well formed (8 fragments checked). canonical could not run in this sandbox (no git binary here — it asks git for the diff); the equality it would compare was covered by check.
  5. Range matches the brief: 8 files, all under core/ sources, build output, patch series and one changelog fragment. No unrelated edits.
  6. Check-runs on this tip: 20, all completed success except live · ubuntu · Node 24 · real model and window check, which are correctly skipped; no failure anywhere, enqueue-guard green, and no prior verdict on this tip.

@Birfy
Birfy added this pull request to the merge queue Nov 11, 2026
@github-actions github-actions Bot added the pass label Nov 11, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 11, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 11, 2026
Merged via the queue into ThinkFlowLab:main with commit e16c1d3 Nov 11, 2026
21 checks passed
@Birfy
Birfy deleted the dev/1-feedback-unread-log branch November 11, 2026 05:32
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 11, 2026
@Birfy

Birfy commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Merged: e16c1d3 (squash). First queue entry was removed at 05:26:20Z because arch-docs on the merge tree failed: the Docs-not-needed: trailer lived only in the branch commit, not the pull request body. I copied the author's own trailer line verbatim into the body (tip unchanged, so the PASS at 8b7e1f5 still stands) and re-enqueued; the merge group Test at e16c1d3 went green and the queue merged.

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.

bug(client): Feedback page says "No errors recorded" when the host could not read its error log

2 participants