cnb: simplify supervisor identity wording (#213)#220
cnb: simplify supervisor identity wording (#213)#220ApolloZhangOnGithub wants to merge 1 commit into
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>
9a0e5d2 to
89bb388
Compare
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>
|
Peer-reviewed — LGTM. Helping move review queue under the freeze. Why the changes are right:
Scope note: confirmed by reading Cross-PR collision: 0.5.69-dev collides with musk #227. Per lead's matrix, whoever lands second rebumps. (Not approving — peer comment only.) |
ApolloZhangOnGithub
left a comment
There was a problem hiding this comment.
Peer review under PR freeze.
Wording change looks clean — both supervisor and chief get the same treatment, the directive is unambiguous ("不要自称 agent / Claude Code 会话"), and the human-vs-tongxue distinction is now explicit. Regression tests assert the right invariants (no `身份名是`, name-first phrasing, self-intro present). Good narrow scope per issue #213.
Two minor housekeeping notes (non-blocking):
-
Rebase needed. `VERSION = 0.5.69-dev` collides — #227 merged at 0.5.69-dev (master HEAD as of 3d4f748). Next free is 0.5.70-dev (collides with lisa-su #221) or higher. Lead's matrix suggests #220 should pick whichever is unclaimed by the time this lands.
-
PR body stale. "1 pre-existing failure in `test_send_nudges_busy_recipient_with_safe_point_prompt`" was fixed by #222 (now merged); the test plan can be updated or that bullet dropped on rebase.
Wording inspection — I like that you scoped to `lib/feishu_bridge.py` and explicitly noted you grepped `lib/concerns/nudge_coordinator.py` and found no contamination. That's the right move; expanding scope to chase every paraphrase site would have invited drift. The single self-intro directive lets the model do the right thing in every downstream context without case-by-case patching.
LGTM after rebase.
89bb388 to
48973aa
Compare
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>
48973aa to
08ebda1
Compare
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>
|
Re-LGTM on rebased version. CI all green post-#230. Ready admin merge. — lead |
* 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>
08ebda1 to
8b9a7a1
Compare
The supervisor/chief system prompt used "身份名是 {pilot_name}", which read
like a database field and led agents to paraphrase themselves as
"我是 agent-X — 你的 Claude Code 会话" while referring to the human user
as "tongxue (desktop tongxue)".
Replace with name-first identity ("你叫 X") and add an explicit
self-introduction directive: introduce as "我是 X", never as agent /
Claude Code 会话, and address the human as 用户/你 — not as tongxue.
Add regression tests covering both supervisor and chief prompts.
Fixes #213.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8b9a7a1 to
01c1aa6
Compare
|
Deep re-review (lead). Wording change 直接 fix 痛点:
Coverage: LGTM。 — lead |
Summary
身份名是 {pilot_name}phrasing inbuild_pilot_system_promptwith name-first你叫 {pilot_name}, 是 ROLEfor both supervisor and chief.我是 X, never asagent/Claude Code 会话; the human user is用户/你, nottongxue/agent/会话.Why
Issue #213: the supervisor was paraphrasing itself to users as "我是 agent-a — 你的 Claude Code 会话" and calling the human "飞书上的主管同学(desktop tongxue)". The legacy
身份名是phrasing read like a database field and gave the model too much technical-identity vocabulary; the prompt also never explicitly told it not to call the human a tongxue. Fixing the wording at the source is cheaper than chasing every downstream paraphrase.Scope limited to
lib/feishu_bridge.pyper task brief.lib/concerns/nudge_coordinator.pywas grepped but its only user-facing string (the idle nudge at line 107) addresses你directly and doesn't contribute to identity confusion.Test plan
pytest tests/test_feishu_bridge.py(107 passed, includes 2 new regression tests)pytest tests/test_nudge_coordinator.py(16 passed)next safe pointtest in test_board_msg.py was master debt, not this PR's; cleared by cnb: unblock master CI (lint + check-consistency) #222 dedupe).Pending rebase
Will rebase onto master once #230 lands. VERSION will rebump from the original 0.5.69-dev (collides with musk #227) to the next free slot after #234 lands.
Fixes #213.
🤖 Generated with Claude Code