Repository navigation
π fix: support Bob 2.x in contributor images - #14432
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
There was a problem hiding this comment.
Container-side change is correct; the host-side entry point still contradicts it. CI red is not this diff's fault.
Read config/backends.conf, bin/contributor-agent.sh, Justfile, and src/crux/agent/manager_launch.go at the v5 merge-base.
What checks out
config/backends.confbackend_perm_flag bobnow branches onbob --version | sed -n 1pwith1.*)β 1.x flags,*)βchat --accept-license --auto-approve --trust. That is byte-for-byte the rule inbobLaunchCmd(src/crux/agent/manager_launch.go:788) andbobLaunchCmdV2(:816), and in.github/scripts/check-bob-launch-flags.sh:14-21. Unknown β 2.x matches pods.backend_bob_prepare_envis called frombin/contributor-agent.shafterbackends.confis sourced (:489-495), so the function is in scope at the new check site (:905). Both# Fail before an unattended Boband# Ensure metrics directory(:919) anchors the new test slices on exist.- The docker path forwards
BOBSHELL_API_KEYinto the container (Justfile:1836), so the alias fill makes Bob 2.x seeBOB_API_KEY. The flag-selection fix works end to end.
Gap vs. the stated goal (medium) β body: "rejected contributors using BOB_API_KEY".
Justfile:947-951(contribute-hive, the documented entry) still exits 1 when onlyBOB_API_KEYis set, with "export BOBSHELL_API_KEY=...". AndJustfile:1836forwards onlyBOBSHELL_API_KEYinto the container β aBOB_API_KEY-only host env never reachescontributor-agent.shat all.- Meanwhile the new in-container error (
bin/contributor-agent.sh:908-910) tells contributors toexport BOB_API_KEY=.... Following that advice lands them on theJustfile:948error that says the opposite. Suggest: accept either name atJustfile:947and addBOB_API_KEYto the forwarded list at:1836β or keep the in-container hint onBOBSHELL_API_KEYso the two messages agree.
Docs (low) β docs/backend-setup.md:16,89 and the relay banner bin/contributor-relay.js:4521 still say contributor mode requires BOBSHELL_API_KEY. Fine if the Justfile stays as-is; stale if BOB_API_KEY becomes accepted.
CI β the red on this head (run 40374306762) is TestPollTmuxOutputForAgent_WiresThrashBreaker and TestHiveOpenPRScript_MultilineBodyIsValidJSONWithoutPython3, neither touched here; #14415 and #14429 fix exactly those. TestBobLaunchCmdRunsBehindEnvPrefix/{1.0.6,2.0.6,unknown} passed. I could not find TestContributorBobKeyCompatibility in the failed-job log β open question whether it ran in a different slice.
Same head as #14383 (ae908a8), adopted deliberately per the body; not a duplicate to close. I have not approved or closed anything.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
Select contributor launch flags by Bob version and bridge current and legacy API key environment names. Pin contributor and pod-agent argv against the same fake CLI and exercise the contributor startup key check. Hive-Run: #14334 Hive-Plan: contributor-bob-version-compatibility Hive-Spec: issue-14334#expected-fix Co-authored-by: odttlnt <91499691+odttlnt@users.noreply.github.com> Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
Justfile, docs and relay banner now agree with the in-container hint; BOB_API_KEY is forwarded into contributor containers. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
ae908a8 to
c0f20b7
Compare
|
hive/awaiting-ci: rebased onto v5 and addressed the review (Justfile now accepts BOB_API_KEY or legacy BOBSHELL_API_KEY and forwards BOB_API_KEY; docs/backend-setup.md and relay banner updated). Note: TestContributorBobKeyCompatibility exists in src/crux/agent/bob_launch_shell_test.go (package agent); I could not verify locally which CI slice runs it. CI pending β sweep will merge when green. π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
c0f20b7
βΉοΈ Sentinel notice β informational; author is trusted (hive agent), merge is not blockedHive flagged this PR (author @hivecommons-hive[bot], head
Hive did not add the |
There was a problem hiding this comment.
Intent alignment vs #14334 β the launch-flag and key-name halves of the issue are implemented and mirror the hub's bobLaunchCmd (config/backends.conf:567-572 vs src/crux/agent/manager_launch.go:788), but the new 2.x argv breaks the relay's own bob liveness probe, so a 2.x contributor still does not work end-to-end.
Blocking (in-scope): BOB_PROCESS_PATTERN no longer matches Bob 2.x
bin/contributor-relay.js:6006βconst BOB_PROCESS_PATTERN = 'bob --accept-license';andbobIsRunning()(:6019) doesprocs.includes(BOB_PROCESS_PATTERN).- This PR's 2.x branch emits
chat --accept-license --auto-approve --trust(config/backends.conf:570), andbin/contributor-agent.sh:1160launches$CMD $PERM_FLAG β¦β the process cmdline isbob chat --accept-license β¦, which never containsbob --accept-license. On Bob 2.xbobIsRunning()is therefore alwaysfalse. - Consequences at head:
bin/lib/pane-classifier.js:877-880setshasIdlePrompt = β¦ || !bobRunningandisWorking = bobRunning && β¦, so a working 2.x bob is classified idle immediately;bin/contributor-relay.js:7353-7362setsbobAlreadyExited = trueand relaunches bob while the previous one is still running β the exact double-launch race the comment at:7349-7352warns about. - Fix: match both spellings, e.g.
const BOB_PROCESS_RE = /\bbob (chat )?--accept-license\b/andBOB_PROCESS_RE.test(procs), and add a 2.x case to whatever test pinsBOB_PROCESS_PATTERN.
Advisory (not blocking): the hub asserts provider:"harness" in ~/.bob/settings via ensureBobAuthSettings (src/crux/agent/manager_launch.go:715-718); the contributor path relies on Bob 2.x's default config. Fine for a fresh container, but worth a comment in backend_bob_prepare_env so the asymmetry is deliberate.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
There was a problem hiding this comment.
Correctness review (head c0f20b7)
- CI red: relay tests still assert the old env var name.
bin/contributor-relay.js:4521now tells bob users toexport BOB_API_KEY=β¦, butbin/contributor-relay.test.js:7850(assert.ok(/BOBSHELL_API_KEY/.test(text), 'bob needs its API key named')) and:7916still requireBOBSHELL_API_KEYin that banner. Both fail in theContributor Relay Syntax + Testsstep of build-and-test (run 40400149052: "FAIL #6437 bob is told about its API key, not to type /login" / "FAIL #6437 waitForCLI prints the blocked backend's own banner"). Fix: have theBACKEND_LOGIN_HELP.bobstep name both (export BOB_API_KEY=β¦ (legacy BOBSHELL_API_KEY also accepted)) or update the two assertions to/BOB_API_KEY/.
Otherwise the contract from #14334 is met: config/backends.conf:569-571 mirrors bobLaunchCmd (src/crux/agent/manager_launch.go:788) exactly, backend_bob_prepare_env (backends.conf:554-558) aliases both key names, and src/crux/agent/bob_launch_shell_test.go:39-50 now drives the contributor backend_perm_flag_shell against the same fake bob as the pod path. I exercised the shell helpers with fake 1.0.6 / 2.0.6 / missing bob and they produce the expected argv.
β hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Reviewed the one commit this adoption adds on top of #14383's head (c0f20b76, which already carries a hive approve at ae908a8). It looks correct to me.
Justfile:947β the pre-flight gate now fails only when bothBOB_API_KEYandBOBSHELL_API_KEYare empty, matching the in-container alias logic #14383 added tobin/contributor-agent.sh; before this commit a contributor with onlyBOB_API_KEYpassed the container check but was refused here.Justfile:1836βBOB_API_KEYis added to the forwarded-name list;add_provider_env(:1825-1830) only emits-e NAMEwhen the variable is set, so an unset key adds nothing to thedocker runargv.Justfile:130-131,bin/contributor-relay.js:4521,docs/backend-setup.md:16,89β the hint text, relay login help and docs now nameBOB_API_KEYwithBOBSHELL_API_KEYas legacy, consistent with the gate.
CI note: the red build-and-test (job 121843532393) fails in the image publish guard β could not inspect tag ghcr.io/hivecommons/hive:abcdef1; refusing a potentially regressive publish / v5-latest is missing linux/arm64 β which this PR's files (Justfile, contributor scripts, docs, one shell test) do not touch.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
The banner now names BOB_API_KEY; the two #6437 relay tests still asserted the old BOBSHELL_API_KEY name. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: pushed bfbdc92 β the two #6437 relay tests still asserted π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
bfbdc92
βΉοΈ Sentinel notice β informational; author is trusted (hive agent), merge is not blockedHive flagged this PR (author @hivecommons-hive[bot], head
Hive did not add the |
There was a problem hiding this comment.
Correctness review (head bfbdc92)
1. Bob 2.x launch breaks the relay's bob liveness probe β turn detection and relaunch misfire (in-scope, high)
config/backends.conf:575 now emits chat --accept-license --auto-approve --trust for 2.x, so the pane runs bob chat --accept-license β¦ (bin/contributor-agent.sh:1161). But bin/contributor-relay.js:6005 still pins BOB_PROCESS_PATTERN = 'bob --accept-license' and bobIsRunning() (contributor-relay.js:6008-6020) does a substring match on /proc/*/cmdline. bob chat --accept-license does not contain that substring, so under 2.x bobIsRunning() is always false.
Consequences, from the code that consumes it:
bin/lib/pane-classifier.js:877-880:hasIdlePrompt = β¦ || !bobRunningβtrueandisWorking = bobRunning && β¦βfalsefor the whole turn, so the relay classifies a working 2.x bob as idle.bin/contributor-relay.js:7353-7365:bobAlreadyExitedis alwaystrue, so on task completion the relay skips stopping the CLI and callsrelaunchCLI()while bob may still be running β the exact "two launches in flight" race the comment at 7349-7351 warns about.
Before this PR a 2.x bob failed at startup (#14334); after it, bob launches but the relay's turn tracking is wrong, so the issue's goal ("contributor image works with Bob 2.x") is not met yet. Fix: match either spelling, e.g. const BOB_PROCESS_PATTERNS = ['bob --accept-license', 'bob chat --accept-license'] and procs.includes() over both (or a regex /\bbob (chat )?--accept-license/), plus a test in bin/contributor-relay.test.js for the 2.x cmdline.
Everything else checked out: chat lands first in both argv paths (contributor-agent.sh:1161 interpolation; agent-launch.sh:165 read -r -a word-split), bob is not in HEADLESS_BACKENDS (contributor-relay.js:3627), key aliasing in backend_bob_prepare_env (backends.conf:554-558) preserves explicit values and is exercised by TestContributorBobKeyCompatibility.
β hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
One verified gap: the relay's bob liveness probe does not recognise the new 2.x launch line, so under Bob 2.x every contributor task is classified idle/complete on the first poll.
Read the tree at bfbdc92, not just the diff:
config/backends.conf:568-572(this PR) now emitschat --accept-license --auto-approve --trustfor 2.x/unknown, so both the interactive launch (bin/contributor-agent.sh:1159:$CMD $PERM_FLAG β¦) and the relay's own relaunch (bin/contributor-relay.js:3015βbackend_perm_flag_shell) typebob chat --accept-license β¦into the pane.bin/contributor-relay.js:6006still hasconst BOB_PROCESS_PATTERN = 'bob --accept-license';andbobIsRunning()(:6008-6019) returnsprocs.includes(BOB_PROCESS_PATTERN). A 2.x cmdline isbob chat --accept-license β¦β the substring never matches, sobobIsRunning()isfalsefor the entire life of a 2.x process.bin/lib/pane-classifier.js:877-880:hasIdlePrompt = BOB_IDLE_CHROME.test(text) || !bobRunning; hasCompletionMarker = true; isWorking = bobRunning && β¦. WithbobRunningpinned false the pane reads as idle-with-completion on every tick, andisWorkingcan never be true.bin/contributor-relay.js:7353-7363:bobAlreadyExited = BACKEND === 'bob' && !bobIsRunning()is then always true, so the relay skips the Ctrl-C and callsrelaunchCLI()while the real bob is still mid-turn β the "two launches in flight" race the comment at:7349-7351describes.
Consequence: on Bob 1.x nothing changes (the 1.x line still contains bob --accept-license), but on 2.x β the case this PR exists to enable β the task is reported completed before bob has done anything, and a second bob is launched on top of the first. Severity: high, because it silently defeats the fix for its own target version.
Suggested fix (small): make the pattern version-agnostic, e.g. match /\bbob (chat )?--accept-license/ on the cmdline text instead of a fixed substring, and add a relay test alongside the existing #6437 bob tests covering the 2.x cmdline. bobIsRunning() is the only consumer, so the change is contained.
Open question I could not settle from the tree: does bob chat (2.x) still exit at the end of every turn the way 1.x does? The bob branch of the classifier and the relaunch path at :7357 assume it does. If 2.x keeps a persistent REPL, the "process gone β turn finished" model needs rethinking beyond the pattern fix.
Everything else I checked holds up: backend_bob_prepare_env (backends.conf:554-558) preserves explicit values when both names are set and the new TestContributorBobKeyCompatibility exercises the real startup block; contributor-agent.sh sources backends.conf at :489-495 before the check at :906; the Justfile forwards both BOB_API_KEY and BOBSHELL_API_KEY into the container (Justfile:1836); the bob --version | sed -n 1p rule mirrors bobLaunchCmd at src/crux/agent/manager_launch.go:788.
I have not approved or requested changes; this is a comment for the hive fix agent / @thatguywob to pick up on this branch.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Supersedes #14383 (adopted after CI went red and the contributor was unavailable; original commits by @thatguywob preserved).
Fixes #14334
This branch is #14383's head
ae908a82fdbcas pushed by @thatguywob (forkthatguywob/hive), unchanged. Last lane reason on the original:check-failing:coverage.Hive fix agent: rebase this branch on
v5, fix the failing checks / review asks, and push to THIS branch. Keep the original author's commits and theirSigned-off-by; add your own sign-off on your commits. Do not open another PR.