Skip to content

fix(acquisition): align managed Doubao runtime - #52

Closed
lunnynight wants to merge 7 commits into
2233admin:mainfrom
lunnynight:codex/doubao-acquisition-20260729
Closed

fix(acquisition): align managed Doubao runtime#52
lunnynight wants to merge 7 commits into
2233admin:mainfrom
lunnynight:codex/doubao-acquisition-20260729

Conversation

@lunnynight

Copy link
Copy Markdown
Contributor

What changed

  • adds the managed Doubao acquisition route and isolates OpenCLI support helpers
  • pins the managed runtime to OhMyOpenCLI b0fdd513f64899b068103ddd7ff0de957d778b5c
  • keeps local CDP execution free from daemon/runtime lineage environment leakage
  • aligns capability readiness with the live composer flow

Why

The authenticated browser acquisition path must use the published Doubao adapter and a clean local CDP environment before geo-xi can retain real answers, citations, traces, and screenshots.

Validation

  • 39 focused unit tests passed
  • tracked diff check passed

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@lunnynight, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fef97a91-6661-4fe7-8a61-b1f6b6f22b0c

📥 Commits

Reviewing files that changed from the base of the PR and between aa774da and fe6a36f.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml
📝 Walkthrough

Walkthrough

The change adds Doubao capability support, authenticated-profile and session readiness checks, target-aware validation, structured OpenCLI errors, trace hashing, adapter-specific runtime lineage, and verification of multiple pinned capability commits.

Changes

OpenCLI capability integration

Layer / File(s) Summary
Runtime pins and verification
Dockerfile, agent/Dockerfile, scripts/install-*, scripts/verify_managed_opencli_runtime.py, tests/unit/test_*runtime*
OpenCLI is pinned to a new revision, both official-site and Doubao commits are ancestry-checked, and runtime verification exercises Doubao commands and reports both capability contracts.
Capability registration and target validation
backend/acquisition/registry.py, backend/schemas/acquisition.py, backend/api/v1/geo_acquisition.py, tests/unit/test_geo_acquisition_api.py
Registrations and descriptors carry optional targets; Doubao requires an authenticated profile, and submissions with an unregistered target return target_not_registered.
Profile-aware probing and execution
backend/acquisition/capabilities.py, backend/acquisition/runner.py, tests/unit/test_acquisition_capabilities.py, tests/unit/test_acquisition_runner.py, tests/unit/test_doubao_acquisition_runner.py
Capability readiness and execution select matching profile kinds, probe sessions, validate conditional inputs and envelopes, map typed failures, and require valid trace hashes.
Channel helpers, routing, and trace lineage
backend/channels/opencli_support.py, backend/channels/opencli_channel.py, backend/agent_server.py, tests/unit/channels/test_opencli_channel.py, tests/unit/test_agent_server.py
Shared parsers, endpoint leasing, structured error extraction, trace hashing, CDP environment cleanup, and adapter-specific runtime lineage are integrated into OpenCLI execution.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant CapabilityAPI
  participant AcquisitionRunner
  participant BrowserPool
  participant OpenCLI
  Client->>CapabilityAPI: submit target and prompt
  CapabilityAPI->>AcquisitionRunner: dispatch registered capability
  AcquisitionRunner->>BrowserPool: select and probe authenticated endpoint
  AcquisitionRunner->>OpenCLI: execute Doubao capture
  OpenCLI-->>AcquisitionRunner: payload, error, and trace hash
  AcquisitionRunner-->>Client: execution result
Loading

Possibly related PRs

Poem

A rabbit hops where OpenCLI runs,
Doubao joins the profile fun.
Hashes sparkle, traces glow,
Safe routes guide the flow below.
Two pinned commits stand in line—
“Hop hooray, the build is fine!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: aligning the managed Doubao runtime.
Description check ✅ Passed The description matches the changeset and accurately describes the Doubao runtime and OpenCLI helper updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@repowise-bot

repowise-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

✅ Health: 7.3

📋 At a glance
3 hotspots touched · 16 new findings introduced · 1 dead-code finding.

Files & modules (2)
  • backend (2 files)
    • backend/agent_server.py
    • backend/channels/opencli_channel.py
  • tests (1 file)
    • .../channels/test_opencli_channel.py

📌 Before you merge

  • Run tests/integration/test_opencli_channel_api.py: they depend on the changed files

🗺️ Change map

flowchart LR
  subgraph PR ["Changed in this PR (1 with dependents)"]
    f_backend_channels_opencli_channel_py["backend/channels/opencli_channel.py 🔥"]:::changed
  end
  f_backend_api_v1___init___py[".../v1/__init__.py"]
  f_backend_channels_opencli_channel_py --> f_backend_api_v1___init___py
  f_backend_api_v1_browsers_py[".../v1/browsers.py"]
  f_backend_channels_opencli_channel_py --> f_backend_api_v1_browsers_py
  f_backend_api_v1_nodes_py[".../v1/nodes.py"]
  f_backend_channels_opencli_channel_py --> f_backend_api_v1_nodes_py
  f_backend_api_v1_workers_py[".../v1/workers.py"]
  f_backend_channels_opencli_channel_py --> f_backend_api_v1_workers_py
  more(["+1 more dependent"])
  PR --> more
  t_tests_integration_test_opencli_channel_api_py(["✅ tests/integration/test_opencli_channel_api.py"]):::guard
  t_tests_integration_test_opencli_channel_api_py -.-> f_backend_channels_opencli_channel_py
  classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
  classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
  classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Loading

Solid arrows: code that imports the changed files (5 direct dependents, from the last indexed snapshot). Dashed: history/tests.

🚨 Change risk: 9.6/10 (high)
This change's risk is driven by:

  • more lines added than baseline
  • more scattered than baseline
🔎 More signals (2)

🔥 Hotspots touched (3)

  • backend/agent_server.py — 18 commits/90d, 0 dependents · primary owner: xujinghua (100%)
  • backend/channels/opencli_channel.py — 31 commits/90d, 8 dependents · primary owner: xujinghua (100%)
  • .../channels/test_opencli_channel.py — 3 commits/90d, 0 dependents · primary owner: xujinghua (100%)

💀 Dead code (1 finding)

  • 💀 backend/agent_server.py (file-level) (confidence 0.40)

👀 Suggested reviewers @xujinghua


📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-29 16:32 UTC
Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot

@lunnynight
lunnynight marked this pull request as ready for review July 29, 2026 16:19
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai coderabbitai 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.

Actionable comments posted: 4

🧹 Nitpick comments (5)
tests/unit/test_agent_image_runtime_packaging.py (1)

18-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the official-site pin too.

The test now pins OHMYOPENCLI_COMMIT and DOUBAO_CAPABILITY_COMMIT, but ARG OFFICIAL_SITE_CAPABILITY_COMMIT=73cc60c8... in agent/Dockerfile is no longer covered, so it can drift silently.

💚 Proposed addition
     assert (
         "ARG DOUBAO_CAPABILITY_COMMIT="
         "b0fdd513f64899b068103ddd7ff0de957d778b5c"
     ) in dockerfile
+    assert (
+        "ARG OFFICIAL_SITE_CAPABILITY_COMMIT="
+        "73cc60c83586ef2c95469b3b70d6cfc80fa5bc53"
+    ) in dockerfile
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_agent_image_runtime_packaging.py` around lines 18 - 25,
Extend the Dockerfile pin assertions in the test covering agent image runtime
packaging to also verify the exact OFFICIAL_SITE_CAPABILITY_COMMIT value
currently defined in agent/Dockerfile, alongside the existing OHMYOPENCLI_COMMIT
and DOUBAO_CAPABILITY_COMMIT checks.
backend/acquisition/runner.py (2)

439-442: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Importing a private helper across module boundaries.

_session_is_ready is module-private to backend.acquisition.capabilities but is now part of the runner's execution path. Consider exposing a public session_is_ready(...) wrapper there and calling that instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/runner.py` around lines 439 - 442, Replace the
cross-module use of private helper _session_is_ready in the runner’s
session_probe_args path with a public session_is_ready wrapper exposed by
backend.acquisition.capabilities, preserving the existing arguments, await
behavior, and readiness check.

490-498: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated profile-failure-code derivation.

The same no_clean_profile / no_{kind}_profile expression appears at Lines 415-419 and again here. Extract a small helper (e.g. _no_profile_code(kind)) so the two paths cannot drift.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/runner.py` around lines 490 - 498, Extract the duplicated
profile-failure-code expression into a shared helper such as
_no_profile_code(kind), then update both the earlier profile-failure path and
the NoCleanProfileError handler in the execution flow to call it. Preserve the
existing anonymous-to-no_clean_profile mapping and the no_{kind}_profile
behavior for other profile kinds.
backend/acquisition/registry.py (1)

81-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Doubao dead-route probe prompt is duplicated with two different values. The registry and the runtime verifier each hardcode their own probe token, so the verifier does not exercise the exact route the registry probes.

  • backend/acquisition/registry.py#L81-L88: keep runtime-route-probe as the single source of truth (or export a shared constant) for the doubao route_probe_args.
  • scripts/verify_managed_opencli_runtime.py#L106-L113: replace the divergent managed-runtime-route-probe literal with the registry's token/constant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/registry.py` around lines 81 - 88, The Doubao dead-route
probe token is duplicated inconsistently between the registry and runtime
verifier. Keep “runtime-route-probe” as the single source of truth in
backend/acquisition/registry.py lines 81-88, optionally exposing a shared
constant, and update scripts/verify_managed_opencli_runtime.py lines 106-113 to
reuse that token instead of “managed-runtime-route-probe”.
backend/acquisition/capabilities.py (1)

146-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the shared daemon-port value for bridge-mode env setup.

_browser_environment still sets OPENCLI_DAEMON_PORT = "19825" directly, while the bridge startup path is driven by _DAEMON_PORT in agent_server.py. If the default is changed there or via OPENCLI_DAEMON_PORT, this session probe can use a different bridge port than the daemon the process launches. Reuse the same source-of-truth rather than duplicating "19825".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/capabilities.py` around lines 146 - 156, Update
_browser_environment’s bridge-mode setup to derive OPENCLI_DAEMON_PORT from the
shared _DAEMON_PORT source used by agent_server.py, rather than hardcoding
"19825". Preserve the existing environment cleanup and non-bridge behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/acquisition/capabilities.py`:
- Around line 241-252: Update the endpoint readiness logic in probe_capabilities
to avoid eagerly evaluating every _session_is_ready call: iterate through
endpoints, await each probe sequentially, and stop immediately when one returns
true. Preserve ready=false when no endpoint succeeds and keep the existing
unavailable_reason handling unchanged.

In `@backend/acquisition/runner.py`:
- Around line 655-663: Update the artifact reference condition in the runner
flow to require valid_trace in addition to trace_artifact and trace_sha256.
Reuse the existing valid_trace result from the trace validation logic,
preserving the current artifact_refs append behavior only when the digest passes
the established 64-character hexadecimal validation.
- Around line 61-69: In the error classification logic, update the marker tuple
used by the refusal check in the runner’s classification function to remove the
broad "refused" token. Preserve only markers that specifically indicate model
refusal, so connection errors such as ECONNREFUSED or "connection refused"
continue to flow to the transport/timeout/page-drift classification branches.

In `@scripts/install-agent.sh`:
- Around line 339-347: The installation guard in the OhMyOpenCLI setup must not
trigger `set -e` when `$OHMYOPENCLI_ROOT` is absent: replace the `[[ -e ... ]]
&& die` form with an explicit conditional that only calls `die` when the target
exists. In both `git -C "$OHMYOPENCLI_ROOT" merge-base --is-ancestor` checks,
append failure handling that calls `die` so capability ancestry validation
failures stop installation.

---

Nitpick comments:
In `@backend/acquisition/capabilities.py`:
- Around line 146-156: Update _browser_environment’s bridge-mode setup to derive
OPENCLI_DAEMON_PORT from the shared _DAEMON_PORT source used by agent_server.py,
rather than hardcoding "19825". Preserve the existing environment cleanup and
non-bridge behavior.

In `@backend/acquisition/registry.py`:
- Around line 81-88: The Doubao dead-route probe token is duplicated
inconsistently between the registry and runtime verifier. Keep
“runtime-route-probe” as the single source of truth in
backend/acquisition/registry.py lines 81-88, optionally exposing a shared
constant, and update scripts/verify_managed_opencli_runtime.py lines 106-113 to
reuse that token instead of “managed-runtime-route-probe”.

In `@backend/acquisition/runner.py`:
- Around line 439-442: Replace the cross-module use of private helper
_session_is_ready in the runner’s session_probe_args path with a public
session_is_ready wrapper exposed by backend.acquisition.capabilities, preserving
the existing arguments, await behavior, and readiness check.
- Around line 490-498: Extract the duplicated profile-failure-code expression
into a shared helper such as _no_profile_code(kind), then update both the
earlier profile-failure path and the NoCleanProfileError handler in the
execution flow to call it. Preserve the existing anonymous-to-no_clean_profile
mapping and the no_{kind}_profile behavior for other profile kinds.

In `@tests/unit/test_agent_image_runtime_packaging.py`:
- Around line 18-25: Extend the Dockerfile pin assertions in the test covering
agent image runtime packaging to also verify the exact
OFFICIAL_SITE_CAPABILITY_COMMIT value currently defined in agent/Dockerfile,
alongside the existing OHMYOPENCLI_COMMIT and DOUBAO_CAPABILITY_COMMIT checks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e7c0d375-7d2c-4014-a3a9-be0c9860f739

📥 Commits

Reviewing files that changed from the base of the PR and between a85ead8 and aa774da.

📒 Files selected for processing (21)
  • Dockerfile
  • agent/Dockerfile
  • backend/acquisition/capabilities.py
  • backend/acquisition/registry.py
  • backend/acquisition/runner.py
  • backend/agent_server.py
  • backend/api/v1/geo_acquisition.py
  • backend/channels/opencli_channel.py
  • backend/channels/opencli_support.py
  • backend/schemas/acquisition.py
  • scripts/install-agent.sh
  • scripts/install-managed-opencli.ps1
  • scripts/verify_managed_opencli_runtime.py
  • tests/unit/channels/test_opencli_channel.py
  • tests/unit/test_acquisition_capabilities.py
  • tests/unit/test_acquisition_runner.py
  • tests/unit/test_agent_image_runtime_packaging.py
  • tests/unit/test_agent_server.py
  • tests/unit/test_doubao_acquisition_runner.py
  • tests/unit/test_geo_acquisition_api.py
  • tests/unit/test_managed_opencli_verifier.py

Comment on lines +241 to +252
if ready and registration.session_probe_args:
ready = any(
[
await _session_is_ready(registration, pool, endpoint)
for endpoint in endpoints
]
)
if not ready:
unavailable_reason = (
registration.session_unavailable_reason
or "browser_session_not_ready"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

any([... for endpoint in endpoints]) probes every endpoint even after one succeeds.

ready = any(
    [
        await _session_is_ready(registration, pool, endpoint)
        for endpoint in endpoints
    ]
)

The list comprehension eagerly awaits _session_is_ready for every endpoint before any() is applied — there's no short-circuiting once a ready endpoint is found. probe_capabilities() runs on the hot submission path (_validate_capability in backend/api/v1/geo_acquisition.py calls it per request), so with multiple authenticated endpoints this needlessly serializes extra subprocess/HTTP round-trips (each probe can take up to COMMAND_TIMEOUT_SECONDS) on every capability check.

⚡ Proposed fix: short-circuit with an explicit loop
         if ready and registration.session_probe_args:
-            ready = any(
-                [
-                    await _session_is_ready(registration, pool, endpoint)
-                    for endpoint in endpoints
-                ]
-            )
+            ready = False
+            for endpoint in endpoints:
+                if await _session_is_ready(registration, pool, endpoint):
+                    ready = True
+                    break
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/capabilities.py` around lines 241 - 252, Update the
endpoint readiness logic in probe_capabilities to avoid eagerly evaluating every
_session_is_ready call: iterate through endpoints, await each probe
sequentially, and stop immediately when one returns true. Preserve ready=false
when no endpoint succeeds and keep the existing unavailable_reason handling
unchanged.

Comment on lines +61 to +69
if any(
marker in normalized
for marker in (
"refusal",
"refused",
"doubao_capture_refusal",
)
):
return "model_refusal"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

"refused" will misclassify connection errors as model_refusal.

Messages like connect ECONNREFUSED 127.0.0.1:9222 or connection refused contain refused and are mapped to model_refusal, which mislabels a transport failure as a model-side refusal (and is checked before the timeout/page-drift branches). Tighten the marker set.

🐛 Proposed fix
     if any(
         marker in normalized
         for marker in (
-            "refusal",
-            "refused",
+            "refusal",
+            "model refused",
+            "refused to answer",
             "doubao_capture_refusal",
         )
     ):
         return "model_refusal"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if any(
marker in normalized
for marker in (
"refusal",
"refused",
"doubao_capture_refusal",
)
):
return "model_refusal"
if any(
marker in normalized
for marker in (
"refusal",
"model refused",
"refused to answer",
"doubao_capture_refusal",
)
):
return "model_refusal"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/runner.py` around lines 61 - 69, In the error
classification logic, update the marker tuple used by the refusal check in the
runner’s classification function to remove the broad "refused" token. Preserve
only markers that specifically indicate model refusal, so connection errors such
as ECONNREFUSED or "connection refused" continue to flow to the
transport/timeout/page-drift classification branches.

Comment on lines +655 to 663
if trace_artifact and trace_sha256:
artifact_refs = [
*artifact_refs,
{"kind": "trace", "ref": result.metadata["trace_artifact"]},
{
"kind": "trace",
"ref": trace_artifact,
"sha256": trace_sha256,
},
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Gate the trace artifact ref on valid_trace, not just truthiness.

valid_trace (Lines 610-615) requires a 64-char hex digest, but the artifact ref here only requires both values to be non-empty. When required_artifacts is empty, a malformed trace_sha256 is still persisted as an artifact hash, so stored artifact refs can carry a digest the runner itself rejected.

🐛 Proposed fix
-            if trace_artifact and trace_sha256:
+            if valid_trace:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if trace_artifact and trace_sha256:
artifact_refs = [
*artifact_refs,
{"kind": "trace", "ref": result.metadata["trace_artifact"]},
{
"kind": "trace",
"ref": trace_artifact,
"sha256": trace_sha256,
},
]
if valid_trace:
artifact_refs = [
*artifact_refs,
{
"kind": "trace",
"ref": trace_artifact,
"sha256": trace_sha256,
},
]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/acquisition/runner.py` around lines 655 - 663, Update the artifact
reference condition in the runner flow to require valid_trace in addition to
trace_artifact and trace_sha256. Reuse the existing valid_trace result from the
trace validation logic, preserving the current artifact_refs append behavior
only when the digest passes the established 64-character hexadecimal validation.

Comment thread scripts/install-agent.sh
Comment on lines 339 to +347
command -v git >/dev/null 2>&1 || die "git is required to install OhMyOpenCLI"
[[ -e "$OHMYOPENCLI_ROOT" ]] && die \
"Managed OhMyOpenCLI target already exists; archive it explicitly before reinstalling: $OHMYOPENCLI_ROOT"
git clone "$OHMYOPENCLI_REPO" "$OHMYOPENCLI_ROOT"
git -C "$OHMYOPENCLI_ROOT" checkout --detach "$OHMYOPENCLI_COMMIT"
git -C "$OHMYOPENCLI_ROOT" merge-base --is-ancestor \
"$OFFICIAL_SITE_CAPABILITY_COMMIT" HEAD
git -C "$OHMYOPENCLI_ROOT" merge-base --is-ancestor \
"$DOUBAO_CAPABILITY_COMMIT" HEAD

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm whether errexit is enabled in the installer and how `die` is used elsewhere.
rg -n 'set -[a-z]*e|set -o errexit|^die\(\)|die\(\) *\{' scripts/install-agent.sh
rg -n -C2 '\]\] && die' scripts/install-agent.sh

Repository: 2233admin/opencli-admin

Length of output: 784


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant git block and surrounding function, then probe the bash errexit AND-list behavior
# for the exact pattern used in line 340 under a subshell.
sed -n '1,45p' scripts/install-agent.sh
sed -n '330,360p' scripts/install-agent.sh

bash -c 'set -e; [[ -e /nonexistent ]] && { echo unreachable; }; echo after' || echo "subshell exited due errexit on absent [[ -e ]]"
bash -c 'set -e; [[ -e /tmp ]] && { echo exists; }; echo after'

Repository: 2233admin/opencli-admin

Length of output: 4512


🌐 Web query:

Bash man errexit AND lists && lists set -e non-zero exit status pipeline exit commands

💡 Result:

In Bash, the errexit option (enabled via set -e or set -o errexit) causes the shell to exit immediately if a command exits with a non-zero status [1]. However, there are specific exceptions to this rule where non-zero exit statuses do not trigger an immediate exit [2][1]. Exceptions for errexit: The errexit option is ignored for commands that are part of the following constructs [2][1]: 1. The condition list following if, while, or until keywords [2][1]. 2. Any command in an AND (&&) or OR (||) list, except for the command following the final operator [2]. 3. Any command in a pipeline, except for the last command [2]. 4. Any command whose return status is being inverted via the! reserved word [3][2]. For pipelines specifically, the return status is normally the exit status of the last command [3]. If the pipefail option is enabled (set -o pipefail), the pipeline's return status is the value of the last (rightmost) command to exit with a non-zero status, or zero if all commands exit successfully [3][4]. When using errexit, if a command that would normally trigger an exit is placed in one of these exempted positions, the shell continues execution [2][1]. Note that if a command is the final command in an AND/OR list or the last command in a pipeline, its non-zero exit status will still cause the shell to exit (unless otherwise protected by an if, while, or until condition) [2].

Citations:


Handle the missing OhMyOpenCLI target and pin ancestry checks explicitly.

With set -euo pipefail, [[ -e "$OHMYOPENCLI_ROOT" ]] && die will exit when the target path is absent before git clone; use an if guard unless missing is an error. Add || die to both merge-base --is-ancestor checks so unsupported or absent capability commits stop installation instead of continuing with an unverified checkout.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/install-agent.sh` around lines 339 - 347, The installation guard in
the OhMyOpenCLI setup must not trigger `set -e` when `$OHMYOPENCLI_ROOT` is
absent: replace the `[[ -e ... ]] && die` form with an explicit conditional that
only calls `die` when the target exists. In both `git -C "$OHMYOPENCLI_ROOT"
merge-base --is-ancestor` checks, append failure handling that calls `die` so
capability ancestry validation failures stop installation.

@lunnynight lunnynight closed this by deleting the head repository Aug 1, 2026
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.

1 participant