Skip to content

feat(planning): badge the interview page title while the tab is hidden - #5522

Open
kyle-sexton wants to merge 12 commits into
mainfrom
feat/5473-hidden-tab-title-badge
Open

kyle-sexton wants to merge 12 commits into
mainfrom
feat/5473-hidden-tab-title-badge

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #5473

Summary

The interview page kept a fixed title, so a person working in another tab could not see that Claude had posted activity or a note. While the tab is hidden the title now shows a count of new activity and notes; the count clears when the tab is shown.

Fix

  • plugins/planning/surface/index.html: the title badge, with a check in the ui journey test.
  • plugins/planning/surface/README.md: one sentence stating the behavior.
  • planning plugin 0.47.3 to 0.48.0, with a CHANGELOG entry.

Verification

  • scripts/check-changelog-parity.sh --check --check-order passes.
  • scripts/validate-plugins.sh passes.
  • plugins/planning/surface/surface.test.sh: PASS=294 FAIL=0 SKIP=1, including the ui_journey phase that restores the plain title.

Related

#4653 (the Option B decision that filed this issue), #5480 (records that decision).

🤖 Generated with Claude Code

@kyle-sexton
kyle-sexton marked this pull request as ready for review September 30, 2026 03:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T03:04:02.542921Z 1226de4 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1226de4fa4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/planning/surface/index.html Outdated
Comment thread plugins/planning/surface/index.html Outdated
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 5s —— View job


Security review complete

  • Gather context (PR diff, changed files)
  • Run /review:security-review skip gate
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

This PR adds a hidden-tab title badge to the planning surface interview page (plugins/planning/surface/index.html) plus matching test/doc/changelog/version updates. Reviewed against the criteria in /review:security-review:

  • The new count (away) is derived entirely from local client state (unseenActs().length, noteThread() timestamps vs. S.noteSeen) — no new server endpoint, no new user input, no new data flow across a trust boundary.
  • document.title = (away > 0 ? "(" + away + ") " : "") + (m.title || "Interview") assigns a plain string to document.title, which the browser treats as text, not markup — no injection vector even though m.title originates from stored interview data (and that pre-existing assignment path is unchanged by this diff, just relocated).
  • No changes to auth, secrets, file paths, or command execution.
  • Non-code changes (plugin.json version bump, CHANGELOG.md, README.md) are documentation-only.
  • No GitHub Actions files touched.
  • No instruction-surface files (CLAUDE.md/AGENTS.md/rules/skill or agent bodies) touched, so the instruction-exception-register lens does not apply here.

Note: this checkout is shallow (git diff origin/main...HEAD fails with "no merge base"), so the review was done by reading the actual feature commits (65d666c, e5078ff, 1226de4) directly rather than a single merge-base diff; the file set matches the PR's changed-files list.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 0s —— View job


Code review complete

  • Gather context (PR diff, files changed)
  • Understand request: invoke /review:code-review for this PR
  • Run the code-review skill/procedure
  • Post inline comments for anchored findings
  • Summarize cross-file findings and results

Summary

Reviewed the diff (gh pr diff 5522, scoped to changed files) against this repo's
/review:code-review criteria: correctness, error handling, test coverage, maintainability.
No security-lane items found (and none would apply here regardless).

Two findings posted as inline comments on plugins/planning/surface/index.html:

  1. Line 1638 — The badge math (away = un + unread - S.hiddenBase) assumes unseen counts only grow while hidden, but if the Notes or Activity fly-out panel is left open/pinned when the tab is hidden, apply()'s periodic poll/SSE updates still call renderFly() → renderActivity()/renderNotes() regardless of visibility, which silently mark new entries "seen" and undercount or suppress the badge. This path isn't covered by the new ui_journey.js phases 8/9, which never open either panel before hiding the tab.
  2. Line 459 — S.hiddenBase is only ever seeded by the visibilitychange listener, so a page opened while already hidden (background tab) never gets a badge until it cycles visible→hidden once.

Both are genuine gaps in the feature as described ("while the tab is hidden the title now shows a count"), not style nits — I did not attempt fixes per this lane's scope (code review, not implementation).

Note: I did not independently run surface.test.sh or the other verification commands the PR body lists; those are author-claimed and unverified by me. The rest of the diff (CHANGELOG.md, plugin.json, README.md, surface.test.sh loop-count edits) looked correct and consistent with the code change.

Comment thread plugins/planning/surface/index.html Outdated
Comment thread plugins/planning/surface/index.html
kyle-sexton and others added 3 commits September 30, 2026 01:11
… state

Track the keys present when the tab hides (or loads hidden) and count what is new against them, so an open Activity or Notes flyout marking entries seen no longer erases the count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 30s —— View job


Security review complete

  • Gather context (PR diff, changed files)
  • Run /review:security-review skip gate and criteria
  • Review changed files for security-relevant surface
  • Post findings

No security issues found.

Reviewed the current net diff against origin/main at HEAD 33e1f38 (the mailbox-style gh pr diff --patch output omits merge commit a356b568f's changes since merges don't generate their own format-patch hunks, so I additionally diffed origin/main HEAD directly per file to confirm I had the true current state, including that commit's fixes).

Changed surface, all in plugins/planning/surface/ plus doc/version bumps:

  • index.html — the new title-badge logic (arrivalKeys(), S.hiddenBase, the away count) is derived entirely from local, in-memory client state (activity/note entries already loaded into S.doc/S.resp). No new server endpoint, no new user-controlled input, no new data flow across a trust boundary.
  • The document.title assignment (L779-L780) builds a plain string ("(" + away + ") " + (m.title || "Interview")) and assigns it to document.title, a property the browser always treats as text — not markup — so this isn't an injection vector even though m.title originates from stored interview data; that assignment path itself is unchanged from before this PR, just relocated and prefixed.
  • The visibilitychange listener and the S.hiddenBase seed-on-load path (L1644-L1648) only read local document/state and set local state — no auth, secrets, file paths, or command execution touched.
  • surface.test.sh / tests/ui_journey.js changes are test-only (new journey phases 8-11 covering the visible/hidden and open-flyout cases).
  • plugin.json (version bump), CHANGELOG.md, README.md are documentation-only.
  • No GitHub Actions files touched.
  • No instruction-surface files (CLAUDE.md/AGENTS.md/rules/skill or agent bodies) touched, so the instruction-exception-register lens does not apply.

This confirms the earlier security pass on commit 1226de4 (no issues found) still holds for the two commits added since (a356b568f's flyout/background-tab tracking fix, and 33e1f38's duplicate-version-key cleanup): both are logic/data fixes over already-local, already-reviewed state, with no new trust-boundary or injection surface.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 13s —— View job


Code review complete

  • Gather context (PR diff, changed files, prior review threads)
  • Invoke /review:code-review procedure for this PR
  • Review changed files against that procedure's criteria
  • Post inline comments for anchored findings
  • Summarize cross-file findings and results

Summary

Reviewed the current head (33e1f38) against origin/main (this checkout is shallow, so I read the accumulated diff across the feature commits plus the live file content rather than a single merge-base diff, and cross-checked the isolated per-commit patch to confirm which hunks are actually this PR's vs. concurrent main churn merged into the branch).

The two edge cases flagged in the earlier review round are fixed, verified against the current code, not just the author's replies:

  • Flyout-open undercounting (Codex + Claude): renderHeader() now derives away from arrivalKeys() — a content-keyed Set diffed against a hiddenBase snapshot (index.html:779) — rather than the old seen-count subtraction, so an open Activity/Notes flyout marking entries seen no longer erases the badge. ui_journey.js phases 10/11 exercise exactly this (open panel, hide, new activity arrives, badge still shows).
  • Initially-hidden tab (Claude): the first apply() now seeds S.hiddenBase from arrivalKeys() when document.hidden is already true (index.html:1601).

One finding posted inline on plugins/planning/surface/index.html:1601: that second fix (background-tab load) has no test anywhere in the suite. The author's reply pointed to journey phases 10/11 for this finding too, but those phases cover the flyout-open case, not a page whose document.hidden is already true at first load — a distinct code path with zero coverage. Not a correctness bug in the current diff, but a concrete, addressable gap on behavior the PR's own fix commit calls out by name.

Rest of the diff (plugin.json version bump, CHANGELOG.md, README.md sentence, surface.test.sh loop-count/skip-count edits) is internally consistent — no duplicate version keys, changelog ordering intact, the 1..11 phase loop and SKIP: 294 ... 110 counts in surface.test.sh match the phases actually defined in ui_journey.js.

Per this lane's scope, I did not independently re-run surface.test.sh / check-changelog-parity.sh / validate-plugins.sh; the PASS=294/FAIL=0/SKIP=1 in the PR body is author-claimed and unverified by me. No security-relevant surface in this diff (client-only state, no new endpoints, no document.title injection vector — plain string assignment) and a security lane already ran and posted "no issues found" on this PR, so I did not duplicate that pass.
· branch feat/5473-hidden-tab-title-badge

Comment thread plugins/planning/surface/index.html
kyle-sexton and others added 3 commits September 30, 2026 09:09
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

feat(planning): hidden-tab title badge on the interview page

1 participant