fix(planning): flag a group summary written before questions were added

[kyle-sexton]

Closes #5451

Summary

A group's summary in the interview surface kept describing the old question set after questions were added to the group, and nothing prompted the session or the page to refresh it.

Fix

• round.py: writing a group summary records the group's current question ids as summaryOf (schema extended). add and add-round print a warning when a question lands in a group whose summary was written for a different set, naming the round.py group --summary command that refreshes it.
• index.html: the group hint shows a Stale chip while the group's members differ from summaryOf, like the per-question stale flag.
• interview/context/surface.md documents the behavior; tests/ui_a.js adds a browser check.
• planning 0.47.0 to 0.47.1 with a CHANGELOG entry.

Verification

• python3 -m unittest test_round test_schema test_server test_exporters in plugins/planning/surface: 357 tests OK (1 skipped).
• scripts/check-changelog-parity.sh --check --check-order: passes.
• scripts/validate-plugins.sh: all manifests and the catalog validated.
• Browser check in tests/ui_a.js (run by surface.test.sh): not confirmed locally, the run had not finished when this PR was opened. CI skips the browser checks by design.

Related

Refs #5191 (parent). Merged origin/main, resolving one conflict in op_add (kept both the stale-summary warning and check_primaries).

🤖 Generated with Claude Code

[chatgpt-codex-connector]

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-29T23:14:07.441128Z	6a4c8ec	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.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a4c8eca2d

ℹ️ 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".

[claude]

Claude finished @kyle-sexton's task in 3m 11s —— View job

---

Code Review (/review:code-review)

• Read context and custom instructions
• Invoke /review:code-review for PR fix(planning): flag a group summary written before questions were added #5490
• Post findings as inline PR review comments
• Post summary comment

Scope: plugins/planning/surface/round.py, index.html, schema/questions.schema.json, test_round.py, tests/fixtures/questions.json, tests/ui_a.js, skills/interview/context/surface.md, CHANGELOG.md, .claude-plugin/plugin.json — diffed against origin/main (gh pr diff 5490).

No blocking findings. Traced the full stale-summary flow end to end:

• group_members / record_summary_of / warn_stale_summaries in round.py — the ids compared are consistent (archived/superseded included on both the Python and JS sides, matching what the page actually lists).
• op_add_round (round.py#L399-L432) calls record_summary_of twice for a group that both gets a new summary and new questions in the same call — once via put_group (pre-add membership), once again after the questions loop (post-add membership). The second call overwrites the first, so the persisted summaryOf and the no-warning behavior both come out correct; it's a redundant computation, not a wrong result, and the TestGroupSummaryOf cases in test_round.py exercise exactly this path. Not flagging it as its own finding since nothing observable is wrong.
• Codex's earlier finding on this PR (round.py group --id {gid} vs. the positional form) is already fixed on this head — the warning now prints round.py group {gid} --summary ..., matching the subparser's positional id.
• index.html's summaryStale() mirrors the Python side's membership/sort semantics, and the hintStale chip is only wired into the "groups" rail view, where group summary hints are actually shown — no stale chip is dropped or falsely raised in the other views.
• Schema addition (summaryOf on a group) and the fixture/test updates are consistent with the new field.

Not independently verified (author-claimed only, per this lane's rules): the unittest/changelog-parity/manifest-validation runs and the tests/ui_a.js browser check described in the PR body — this sandbox's Bash tool required interactive approval for process execution that isn't available in this unattended run, so I reviewed the code and tests statically rather than executing them.
· branch fix/5451-group-summary-stale

[claude]

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

---

Security review complete ✅

• Gather context (PR diff, changed files, custom instructions)
• Run /review:security-review skill for PR fix(planning): flag a group summary written before questions were added #5490 at HEAD 03c108e
• Post findings

Scope reviewed: all 9 changed files (round.py, index.html, questions.schema.json, surface.md, tests, CHANGELOG, plugin manifest), diffed against origin/main.

Findings: none. This change adds a computed summaryOf field to the group schema, a stderr warning in round.py's add/add-round ops, and a client-side "Stale" chip in the static index.html page:

• round.py group_members/record_summary_of/warn_stale_summaries — pure local JSON manipulation and a print(..., file=sys.stderr) warning; no injection surface, no new file/network I/O, no secrets or credentials handled.
• index.html summaryStale/rail rendering — the new Stale chip is a static string; the existing esc() escaping still wraps the only untrusted-ish value (s.hint), so no new XSS surface.
• Schema/test/fixture/CHANGELOG changes carry no logic.

The one real defect on this diff — the earlier round.py group --id {gid} usage in the warning text, which didn't match the CLI's positional id argument (Codex's P2 comment) — is a correctness bug, not a security issue, and is already fixed on this branch in c7923a6.

No auth/authorization boundaries, injection surfaces, or credential handling are touched by this PR; server.py (the only network-facing component in this plugin) is unmodified and out of scope. Also checked the instruction-surface-deletion lens (CLAUDE.md/AGENTS.md/rules/skill bodies): the one prose doc changed here (surface.md) only appends a clause to an existing row, it doesn't remove or soften a guardrail.
· branch fix/5451-group-summary-stale

[claude]

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

---

Security Review (/review:security-review)

• Read context and custom instructions
• Invoke /review:security-review for PR fix(planning): flag a group summary written before questions were added #5490
• Post findings
• Post summary comment

Scope: plugins/planning/surface/round.py, index.html, schema/questions.schema.json, test_round.py, tests/fixtures/questions.json, tests/ui_a.js, skills/interview/context/surface.md, CHANGELOG.md, .claude-plugin/plugin.json — diffed against origin/main (gh pr diff 5490), reviewed at HEAD cce42ad.

No security findings.

This PR adds a summaryOf tracking field to groups and a staleness check:

• group_members, record_summary_of, warn_stale_summaries operate purely on the local questions.json document already loaded by the CLI — no shell execution, no file-path construction from user input, no network calls. The next(...) lookups in record_summary_of and op_add_round (L399-L432) are only reachable after the group has just been created/updated by put_group in the same call, so there's no untrusted-id crash path distinct from pre-existing behavior.
• index.html's summaryStale() only compares sorted id arrays from S.doc; the new Stale chip markup at L700 is a static string, and the existing summary text continues to go through esc() before insertion — no new XSS sink is introduced.
• Schema, test, fixture, and doc changes (questions.schema.json, test_round.py, tests/fixtures/questions.json, tests/ui_a.js, surface.md, CHANGELOG.md, plugin.json) are data/test/doc-only with no security-relevant surface.

No instruction-surface deletions in this diff (surface.md only adds a clause; no guardrail file is narrowed or removed). No GitHub Actions changes in this PR (out of scope for zizmor either way).
· branch fix/5451-group-summary-stale

[claude]

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

---

Code Review (/review:code-review)

• Read context and custom instructions
• Invoke /review:code-review for PR fix(planning): flag a group summary written before questions were added #5490
• Determine review scope vs. the last completed review on this lane
• Post summary comment

Scope check. Compared the true PR diff (gh pr diff 5490, i.e. against origin/main) at the current head cce42ad with the diff this lane already reviewed and cleared at 03c108e (see the "Claude finished @kyle-sexton's task" comment above, completed 2026-09-30 02:36 UTC). The only commit added since then is a merge of origin/main into this branch; diffing the two heads directly on the feature files (round.py, schema/questions.schema.json, test_round.py, skills/interview/context/surface.md) shows zero changes between them. The apparent differences elsewhere (index.html's two-column card CSS, the extra dependsOn fixture entry, the new ui_a.js viewport assertions, the CHANGELOG.md/plugin.json version churn) are all content from PR #5463 that landed on main and arrived here purely via the merge — none of it is part of this PR's actual diff against origin/main, and it doesn't touch the summaryOf/stale-chip code path this PR adds.

Conclusion: skip gate applies (this lane's own gate: "this head already has a successful review from this lane that still applies"). No new findings to report — the prior review's assessment stands:

• group_members / record_summary_of / warn_stale_summaries in round.py are consistent between the Python and JS membership semantics.
• The Codex P2 finding (--id vs. positional group id in the warning text) is fixed on this head at round.py#L353.
• index.html's summaryStale() mirrors the Python membership/sort semantics and is only wired into the groups-rail hint where group summaries are shown.

No inline comments posted since no new findings exist on the current diff.

Not independently verified (author-claimed only): the unittest/changelog-parity/manifest-validation runs and the tests/ui_a.js browser check described in the PR body.

[claude]

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

---

Code Review (/review:code-review)

• Read context and custom instructions
• Invoke /review:code-review for PR fix(planning): flag a group summary written before questions were added #5490 at HEAD f875931
• Post findings as inline PR review comments
• Post summary comment

Scope check. Diffed the PR against origin/main (gh pr diff 5490) at the current head f875931181df9cf5e4a5a97fadcc9457ca9aec669 and compared it with the diff this lane already reviewed and cleared at cce42ad (see the "Claude finished @kyle-sexton's task" comment completed 2026-09-30 03:04 UTC). The PR's actual diff against origin/main — the same five commits (round.py/questions.schema.json/test_round.py, the index.html/surface.md/fixtures/ui_a.js Stale-chip commit, the two version-bump commits, and the --id → positional fix) — is byte-for-byte unchanged.

The only new commit since cce42ad is another merge of origin/main. Diffing the nine PR files directly between cce42ad and this head confirms the only content that landed is unrelated: PR #5482's version-label lint fix (VERSION_LABEL regex in round.py, the matching surface.md clause, and its test) plus changelog/version churn from that merge. None of it touches the summaryOf/stale-chip code path this PR adds.

Conclusion: skip gate applies (criterion 4 — "this head already has a successful review from this lane that still applies"). No new findings; the prior review's assessment stands:

• group_members / record_summary_of / warn_stale_summaries in round.py are consistent between the Python and JS membership semantics.
• The earlier Codex P2 finding (--id vs. positional group id in the warning text) is fixed on this head at round.py#L354.
• index.html's summaryStale() mirrors the Python membership/sort semantics and is only wired into the groups-rail hint where group summaries are shown.

No inline comments posted since there is no new diff content to anchor findings to.

Not independently verified (author-claimed only): the unittest/changelog-parity/manifest-validation runs and the tests/ui_a.js browser check described in the PR body.
· branch fix/5451-group-summary-stale

[claude]

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

---

Security Review (/review:security-review)

• Read context and custom instructions
• Invoke /review:security-review for PR fix(planning): flag a group summary written before questions were added #5490 at HEAD f875931
• Post findings
• Post summary comment

Scope check (skip gate). gh pr diff 5490 (against origin/main) at head f875931181df9cf5e4a5a97fadcc9457ca9aec669 touches the same 9 files with byte-identical content to the diff this lane already reviewed and cleared at 03c108e/cce42ad (see the two prior "Security Review" comments above, both no findings). The only commits added since then are two merges of origin/main into this branch (bringing in unrelated PRs like #5482's version-label lint fix and #5486's detect.sh identity subcommand); diffing cce42ad against this head confirms those unrelated commits are what changed, not this PR's own diff. This lane's skip gate applies: "This head already has a successful security review that still applies."

No security findings, per the prior reviews' analysis, which still holds on this diff:

• group_members, record_summary_of, warn_stale_summaries — pure local JSON manipulation over the already-loaded questions.json document; no shell execution, no untrusted file paths, no network I/O, no secrets/credentials handled.
• index.html's summaryStale() only compares sorted id arrays; the new Stale chip at L700 is static markup, and the existing summary text still goes through esc() — no new XSS sink.
• Schema, test, fixture, doc, and version/changelog changes are data/test/doc-only with no security-relevant surface.

Instruction-surface deletions: none in this diff — surface.md only appends a clause to an existing row; no guardrail file is narrowed or removed.

GitHub Actions: no workflow files touched by this PR; out of scope for this lane either way.

No inline comments posted (no new findings on this diff).
· branch fix/5451-group-summary-stale

[claude]

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

---

Security Review (/review:security-review)

• Read context and custom instructions
• Invoke /review:security-review for PR fix(planning): flag a group summary written before questions were added #5490 at HEAD 70b1e5a
• Post findings
• Post summary comment

Scope check (skip gate). gh pr diff 5490 at head 70b1e5a3336cce058ed41481b1e13297bae6f9d0 touches the same 9 files with byte-identical content to the diff this lane already reviewed and cleared at 03c108e/cce42ad/f875931 (see the prior "Security Review" comments above, all no findings). The only commit added since then is another merge of origin/main into this branch, bringing in five unrelated PRs (#5484 carry-chip wording, #5412 promotion-evidence docs, #5425 OTEL store cap, #5494 machine-profile docs, #5424 subagent turns-to-complete). Diffing f875931 against this head confirms that merge is what changed — including one line inside plugins/planning/surface/index.html from #5484's carry-chip tooltip — none of it touches the summaryOf/stale-chip code path this PR adds. This lane's skip gate applies: "This head already has a successful security review that still applies."

No security findings, per the prior reviews' analysis, which still holds on this diff:

• group_members, record_summary_of, warn_stale_summaries operate purely on the already-loaded local questions.json document — no shell execution, no untrusted file paths, no network I/O, no secrets/credentials handled.
• index.html's summaryStale() only compares sorted id arrays from S.doc; the Stale chip markup at L700 is static, and the existing summary text still goes through esc() — no new XSS sink.
• Schema, test, fixture, doc, and version/changelog changes are data/test/doc-only with no security-relevant surface.

Instruction-surface deletions: none in this diff — surface.md only appends a clause to an existing row; no guardrail file is narrowed or removed.

GitHub Actions: no workflow files touched by this PR; out of scope for this lane either way.

No inline comments posted (no new findings on this diff).
· branch fix/5451-group-summary-stale

[claude]

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

---

Code Review (/review:code-review)

• Read context and custom instructions
• Invoke /review:code-review for PR fix(planning): flag a group summary written before questions were added #5490 at HEAD 70b1e5a
• Determine review scope vs. the last completed review on this lane
• Post summary comment

Scope check. git diff f875931..70b1e5a --stat -- plugins/planning/ (the last head this lane reviewed and cleared, per the "Claude finished @kyle-sexton's task" comment completed 2026-09-30 03:04 UTC, and the current head) shows only 4 files touched:

• plugins/planning/.claude-plugin/plugin.json — version bump 0.47.6 → 0.47.7
• plugins/planning/CHANGELOG.md — adds the 0.47.7 entry (this PR's own, already reviewed) and a 0.47.6 entry for fix(planning): explain the "open N rounds" chip on interview question cards #5449 that arrived via the merge
• plugins/planning/surface/index.html — one line, the carry-chip tooltip text from PR fix(planning): reword the carry chip and explain it on hover #5484 (open N rounds → carried N rounds, with a title attribute), at L657 — unrelated to this PR's summaryOf/stale-group code
• plugins/planning/surface/tests/ui_journey.js — a new assertion for that same carry-chip wording, not part of this PR's changed-files list

The one new commit since f875931 is another merge of origin/main (bringing in #5484 and other already-merged PRs). round.py, schema/questions.schema.json, test_round.py, tests/fixtures/questions.json, tests/ui_a.js, and skills/interview/context/surface.md — the files carrying this PR's actual summaryOf/stale-group logic — are byte-for-byte unchanged since f875931.

Conclusion: skip gate applies (this lane's own gate, criterion 4 — "this head already has a successful review from this lane that still applies"). No new findings; the prior reviews' assessment stands:

• group_members / record_summary_of / warn_stale_summaries in round.py are consistent between the Python and JS membership semantics.
• The earlier Codex P2 finding (--id vs. positional group id in the warning text) is fixed on this head at round.py#L353.
• index.html's summaryStale() mirrors the Python membership/sort semantics and is only wired into the groups-rail hint where group summaries are shown.

No inline comments posted — no new findings exist on the current diff, and none of the changes since the last review land on this PR's own lines.

Not independently verified (author-claimed only, per this lane's rules): the unittest/changelog-parity/manifest-validation runs and the tests/ui_a.js browser check described in the PR body.
· branch fix/5451-group-summary-stale