cnb: consolidate update-check — bin/cnb calls Python via bin/board (#43) - #228
Conversation
Follow-up to PR #224. Removes the duplicate version-check logic from bin/cnb (~120 lines) so both entry points (`cnb <subcmd>` and the interactive banner) share one implementation in lib/update_check.py. - `bin/cnb` subcommand path (line 165): replaced `_check_update notify` with `bin/board update-check --quiet` (silent stdout; owner still gets notified when stale). - `bin/cnb` banner path: replaced `_check_update` with `bin/board update-check --terminal` (silent unless stale, then prints the historical yellow banner line so UX is unchanged). - Removed `_check_update`, `_notify_update_owner`, `_version_gt`, `_in_virtualenv`, `_CACHE`, `_NOTIFIED` from bash. Kept `_read_update_owner` because `cnb exec` still uses it for sender fallback. - Added `--quiet` and `--terminal` modes to `cmd_update_check`, with tests for each (silent up-to-date, silent stale-quiet, yellow banner on stale-terminal, no banner in venv). - `bin/board` startup hook now skips when the command itself is `update-check` to avoid double-firing the check. 41 update-check tests pass; full suite 1670/1670 (excluding the pre-existing test_board_msg drift bezos is fixing in PR #220). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
LGTM (lead, comment because self-approve blocked). 合理的 follow-up:
Merge order: 41 测试通过,PR body 列了 5 个新场景(quiet/terminal × stale/up-to-date/venv),覆盖到位。 — lead |
6974a14
into
lisa-su/issue-43-update-check-tongxue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98c8e4fd59
ℹ️ 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".
|
|
||
| if [ $# -gt 0 ]; then | ||
| _check_update notify | ||
| CNB_SKIP_UPDATE_CHECK=1 "$CLAUDES_HOME/bin/board" update-check --quiet 2>/dev/null || true |
There was a problem hiding this comment.
Keep the quiet update check from leaking stdout
When any cnb <subcmd> runs in a legacy .claudes/ project, this preflight invokes bin/board, whose environment discovery prints the .claudes migration hint to stdout before cmd_update_check sees --quiet; projects with a .cnb/ directory but no config similarly print the config error. Because only stderr is redirected here, commands like cnb help, cnb version, or scripts expecting clean output now get extra text that the old bash update check never emitted. Redirect stdout as well or make the board bootstrap silent for this preflight.
Useful? React with 👍 / 👎.
| # ---- Banner + launch ---- | ||
| clear | ||
| _check_update | ||
| CNB_SKIP_UPDATE_CHECK=1 "$CLAUDES_HOME/bin/board" update-check --terminal 2>/dev/null || true |
There was a problem hiding this comment.
Avoid notifying from the interactive banner check
When an interactive cnb launch sees a stale cached version and an update owner/board DB exists, this --terminal invocation still runs cmd_update_check, which calls check_update() before branching on --terminal; check_update() sends the [cnb update] board message. The old interactive _check_update path only printed the yellow banner and did not mutate the board, so simply opening cnb can now enqueue an owner notification/spam an inbox unexpectedly.
Useful? React with 👍 / 👎.
Reverts the bin/cnb subcommand-path consolidation from #228. Replacing the inline bash `_check_update notify` with `bin/board update-check --quiet` caused two failures on Linux CI that don't repro locally: 1. `test_bin_cnb_projects_scan_dispatches_json_contract` — empty stdout from `cnb projects scan --json` (sync hook somehow interferes with downstream subcommand output capture on Linux). 2. `test_version_subcommand_notifies_lead_when_outdated` — when the prior commit detached the hook with `& disown` to avoid #1, the notification stopped being delivered before the test's inbox check. Either path (sync / async) breaks a different test. The bash version was working before; restore it and let the bin/board startup hook remain as the deliverable for #43. Both paths are now in place — a small amount of duplication, but it's safer to ship than to keep fighting CI. The bin/board hook (the actual KR2 value — tongxue running `board` directly get the check) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* cnb: tongxue update-check via board startup hook (#43) Port the bash version-check from `bin/cnb` into a reusable Python module so tongxue who run `board` directly (skipping the `cnb` wrapper) also detect a stale install and route an update task to the device-supervisor tongxue. Each tongxue does not self-update. - `lib/update_check.py`: pure-Python check_update + helpers, shares the existing on-disk cache (`~/.cnb/latest-version`) and notification suppression (`~/.cnb/update-notified`) with the bash version. - `bin/board`: silent startup hook in main(); never blocks dispatch. `CNB_SKIP_UPDATE_CHECK=1` disables for tests / quick runs. - `board update-check [--force]`: manual trigger for debugging. - Skipped in venv (user-managed install). - Cross-provider versions are normalized: PEP 440 .dev0, npm -dev, v-prefix, prerelease suffixes all compare correctly. - Steady-state overhead is sub-millisecond: cache hit + tuple compare; npm is only re-fetched in the background when the 60-min TTL expires. - 36 unit tests cover normalization, venv detection, owner resolution precedence, notification suppression, async refresh, and the CLI command. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cnb: consolidate update-check — bin/cnb calls Python via bin/board (#43) Follow-up to PR #224. Removes the duplicate version-check logic from bin/cnb (~120 lines) so both entry points (`cnb <subcmd>` and the interactive banner) share one implementation in lib/update_check.py. - `bin/cnb` subcommand path (line 165): replaced `_check_update notify` with `bin/board update-check --quiet` (silent stdout; owner still gets notified when stale). - `bin/cnb` banner path: replaced `_check_update` with `bin/board update-check --terminal` (silent unless stale, then prints the historical yellow banner line so UX is unchanged). - Removed `_check_update`, `_notify_update_owner`, `_version_gt`, `_in_virtualenv`, `_CACHE`, `_NOTIFIED` from bash. Kept `_read_update_owner` because `cnb exec` still uses it for sender fallback. - Added `--quiet` and `--terminal` modes to `cmd_update_check`, with tests for each (silent up-to-date, silent stale-quiet, yellow banner on stale-terminal, no banner in venv). - `bin/board` startup hook now skips when the command itself is `update-check` to avoid double-firing the check. 41 update-check tests pass; full suite 1670/1670 (excluding the pre-existing test_board_msg drift bezos is fixing in PR #220). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cnb: address review nits on update-check (#43) Both from the PR #224 review: 1. Hoist the `is_venv()` check to the top of `cmd_update_check`. The old order ran `--force`'s suppression-clear and 2s sleep before `check_update`'s internal venv guard kicked in, so debugging from a venv shell wasted an npm spawn + wait. 2. Replace `time.sleep(2)` after the async refresh with a new `refresh_latest_version_sync(timeout=10)` helper. The old sleep was a race: a slow npm finished after the sleep returned, defeating `--force`; a fast one made the user wait for nothing. The sync variant blocks until npm returns or times out. Net: 6 new tests (sync-refresh success / timeout / missing npm / non-zero exit / empty stdout; venv-short-circuits-before-refresh). 42/42 update_check tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cnb: detach update-check hook in bin/cnb subcommand path (#43) The subcommand-path hook was synchronously waiting for `bin/board update-check --quiet` to complete before dispatching, which surfaced as an empty-stdout failure on Linux CI for `cnb projects scan --json` (local pass, CI fail across 3.11/3.12/3.13). Detach the hook with `( ... ) & disown` plus full fd redirect so it cannot block or pollute the downstream subcommand whose stdout the test captures. Local repro of the test still passes; this is a defensive bet for the Linux CI failure. The banner path (line ~486) keeps its synchronous `--terminal` call since the user is sitting at the terminal anyway. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cnb: revert bin/cnb consolidation, keep bash _check_update (#43) Reverts the bin/cnb subcommand-path consolidation from #228. Replacing the inline bash `_check_update notify` with `bin/board update-check --quiet` caused two failures on Linux CI that don't repro locally: 1. `test_bin_cnb_projects_scan_dispatches_json_contract` — empty stdout from `cnb projects scan --json` (sync hook somehow interferes with downstream subcommand output capture on Linux). 2. `test_version_subcommand_notifies_lead_when_outdated` — when the prior commit detached the hook with `& disown` to avoid #1, the notification stopped being delivered before the test's inbox check. Either path (sync / async) breaks a different test. The bash version was working before; restore it and let the bin/board startup hook remain as the deliverable for #43. Both paths are now in place — a small amount of duplication, but it's safer to ship than to keep fighting CI. The bin/board hook (the actual KR2 value — tongxue running `board` directly get the check) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * cnb: dispatcher must keep lead working, not nudge dev idle (#223) Inverts the previous nudge model: - Workers (dev) idle is normal — they wait for lead to assign work. Remove the "OKR continue" nudge that interrupted them with self-driven work prompts. - Lead idle is anomalous — the org has no engine. Add explicit lead keep-alive: when lead is idle, nudge it to scan team status, review PR queue, and proactively dispatch the next issue to free workers. NudgeCoordinator.tick now processes lead separately with a different message and only inbox/lead_idle nudge types (no queued_flush for lead since it's the org root). Bump VERSION to 0.5.79-dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR #224 (the KR2 ship lead green-lit). Removes the duplicate version-check logic from
bin/cnbso both entry points share one implementation.Base branch is PR #224, not master, since this stacks on its
lib/update_check.py.Summary
bin/cnbsubcommand path (cnb <subcmd>): now callsbin/board update-check --quiet(silent stdout; owner still notified via board send when stale).bin/cnbinteractive banner: now callsbin/board update-check --terminal(silent unless stale, then prints the historical yellow banner line — UX unchanged)._check_update,_notify_update_owner,_version_gt,_in_virtualenv,_CACHE,_NOTIFIED._read_update_ownerin bash becausecnb execstill calls it directly for sender fallback. Porting that one is a separate clean-up not worth doing here.cmd_update_checkmodes:--quiet(no stdout) and--terminal(yellow line on stale, silent otherwise).bin/boardstartup hook now skips when the command itself isupdate-checkto avoid double-firing.Why now
Lead's message: "你也可以接手 KR2 部分继续后续工作". Listed in my OKR holding queue as the deferred KR2 follow-up. With PR #224 LGTM'd, this consolidation is the natural next slice.
Test plan
pytest tests/test_update_check.py— 41/41 pass (5 new: --quiet silent stale, --quiet silent up-to-date, --terminal silent up-to-date, --terminal yellow on stale, --terminal silent in venv).ruff check+ruff formatclean.test_board_msg.pydrift bezos is fixing in PR cnb: simplify supervisor identity wording (#213) #220).board update-checkmodes (quiet/terminal/default) produce expected output in both stale and up-to-date scenarios. Verified end-to-end with a mocked stale cache.bin/cnb helpstill works after bash helper removal.Versioning
VERSION → 0.5.70-dev. Stacks on #224's 0.5.69-dev.
🤖 Generated with Claude Code