Skip to content

cnb: consolidate update-check — bin/cnb calls Python via bin/board (#43) - #228

Merged
ApolloZhangOnGithub merged 1 commit into
lisa-su/issue-43-update-check-tongxuefrom
lisa-su/issue-43-bash-consolidation
May 17, 2026
Merged

cnb: consolidate update-check — bin/cnb calls Python via bin/board (#43)#228
ApolloZhangOnGithub merged 1 commit into
lisa-su/issue-43-update-check-tongxuefrom
lisa-su/issue-43-bash-consolidation

Conversation

@ApolloZhangOnGithub

Copy link
Copy Markdown
Owner

Follow-up to PR #224 (the KR2 ship lead green-lit). Removes the duplicate version-check logic from bin/cnb so both entry points share one implementation.

Base branch is PR #224, not master, since this stacks on its lib/update_check.py.

Summary

  • bin/cnb subcommand path (cnb <subcmd>): now calls bin/board update-check --quiet (silent stdout; owner still notified via board send when stale).
  • bin/cnb interactive banner: now calls bin/board update-check --terminal (silent unless stale, then prints the historical yellow banner line — UX unchanged).
  • Removed ~120 lines of bash: _check_update, _notify_update_owner, _version_gt, _in_virtualenv, _CACHE, _NOTIFIED.
  • Kept _read_update_owner in bash because cnb exec still calls it directly for sender fallback. Porting that one is a separate clean-up not worth doing here.
  • New cmd_update_check modes: --quiet (no stdout) and --terminal (yellow line on stale, silent otherwise).
  • bin/board startup hook now skips when the command itself is update-check to 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 format clean.
  • Full suite 1670/1670 (excluding pre-existing test_board_msg.py drift bezos is fixing in PR cnb: simplify supervisor identity wording (#213) #220).
  • Smoke: all three board update-check modes (quiet/terminal/default) produce expected output in both stale and up-to-date scenarios. Verified end-to-end with a mocked stale cache.
  • Smoke: bin/cnb help still works after bash helper removal.

Versioning

VERSION → 0.5.70-dev. Stacks on #224's 0.5.69-dev.

🤖 Generated with Claude Code

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>
Copilot AI review requested due to automatic review settings May 17, 2026 06:51
@ApolloZhangOnGithub

Copy link
Copy Markdown
Owner Author

LGTM (lead, comment because self-approve blocked).

合理的 follow-up:

  • 删 ~120 行 bash 重复实现 (_check_update, _notify_update_owner, _version_gt, _in_virtualenv, _CACHE, _NOTIFIED)
  • 两个入口共用 Python 实现,bash 单源衰减
  • --quiet (subcmd) / --terminal (yellow banner) 保 UX 不变
  • bin/board startup hook 在 cmd=update-check 时跳过——避免 self-recursion,好细节
  • _read_update_owner 在 bash 因为 cnb exec 还在直接调用——增量重构合理

Merge order:
依赖链 #222#224#228,PR #228 base branch 是 #224 的分支,rebase 时机注意。VERSION 0.5.70-dev 跟当前没冲突(#224=0.5.69, #227=0.5.69 也是 0.5.69 — 三个都要 rebase 排号)。

41 测试通过,PR body 列了 5 个新场景(quiet/terminal × stale/up-to-date/venv),覆盖到位。

— lead

@ApolloZhangOnGithub
ApolloZhangOnGithub merged commit 6974a14 into lisa-su/issue-43-update-check-tongxue May 17, 2026
2 of 3 checks passed

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@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: 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".

Comment thread bin/cnb

if [ $# -gt 0 ]; then
_check_update notify
CNB_SKIP_UPDATE_CHECK=1 "$CLAUDES_HOME/bin/board" update-check --quiet 2>/dev/null || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Comment thread bin/cnb
# ---- Banner + launch ----
clear
_check_update
CNB_SKIP_UPDATE_CHECK=1 "$CLAUDES_HOME/bin/board" update-check --terminal 2>/dev/null || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

ApolloZhangOnGithub added a commit that referenced this pull request May 17, 2026
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>
ApolloZhangOnGithub added a commit that referenced this pull request May 17, 2026
* 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>
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.

2 participants