feat(acquisition): dispatch managed ChatGPT capture - #64
Open
lunnynight wants to merge 1 commit into
Open
Conversation
|
✅ Health of changed files: 5.7 → 5.8 (+0.1) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (5 with dependents)"]
f_backend_acquisition_runner_py["backend/acquisition/runner.py 🔥"]:::changed
f_backend_agent_server_py["backend/agent_server.py 🔥"]:::changed
f_backend_browser_pool_py["backend/browser_pool.py 🔥"]:::changed
f_backend_acquisition_capabilities_py["backend/acquisition/capabilities.py"]:::changed
f_backend_channels_opencli_channel_py["backend/channels/opencli_channel.py"]:::changed
end
f_backend_executor_local_py["backend/executor/local.py"]
f_backend_acquisition_runner_py --> f_backend_executor_local_py
f_backend_main_py["backend/main.py"]
f_backend_acquisition_runner_py --> f_backend_main_py
f_backend_worker_tasks_py["backend/worker/tasks.py"]
f_backend_acquisition_runner_py --> f_backend_worker_tasks_py
f_backend_api_v1_browsers_py[".../v1/browsers.py"]
f_backend_browser_pool_py --> f_backend_api_v1_browsers_py
f_backend_api_v1_nodes_py[".../v1/nodes.py"]
f_backend_browser_pool_py --> f_backend_api_v1_nodes_py
f_backend_api_v1_skill_record_py[".../v1/skill_record.py"]
f_backend_browser_pool_py --> f_backend_api_v1_skill_record_py
f_backend_api_v1_workers_py[".../v1/workers.py"]
f_backend_browser_pool_py --> f_backend_api_v1_workers_py
f_backend_api_v1_geo_acquisition_py[".../v1/geo_acquisition.py"]
f_backend_acquisition_capabilities_py --> f_backend_api_v1_geo_acquisition_py
f_backend_channels_opencli_channel_py --> f_backend_api_v1_browsers_py
f_backend_channels_opencli_channel_py --> f_backend_api_v1_nodes_py
f_backend_channels_opencli_channel_py --> f_backend_api_v1_workers_py
f_backend_channels_cli_channel_py["backend/channels/cli_channel.py"]
f_backend_channels_opencli_channel_py --> f_backend_channels_cli_channel_py
more(["+7 more dependents"])
PR --> more
t_tests_unit_test_agent_server_py(["✅ tests/unit/test_agent_server.py"]):::guard
t_tests_unit_test_agent_server_py -.-> f_backend_agent_server_py
t_tests_integration_test_auth_ws_py(["✅ tests/integration/test_auth_ws.py"]):::guard
t_tests_integration_test_auth_ws_py -.-> f_backend_browser_pool_py
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
Solid arrows: code that imports the changed files (16 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (5)
2 more
📊 See the full report for this PR |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change pins and bootstraps OhMyOpenCLI in Docker images and installers, adds authenticated ChangesManaged runtime packaging
Profile-aware acquisition
Runtime execution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AcquisitionRunner
participant BrowserPool
participant OpenCLIChannel
Client->>AcquisitionRunner: Submit chat-ai.capture request
AcquisitionRunner->>BrowserPool: Select authenticated endpoint
AcquisitionRunner->>OpenCLIChannel: Invoke ChatGPT capture with prompt
OpenCLIChannel-->>AcquisitionRunner: Return capture payload
AcquisitionRunner-->>Client: Persist and return result
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
backend/agent_server.py (1)
168-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for request-specific runtime lineage.
The supplied runner tests mock
channel.collect; they do not execute the subprocess-sidegit logpath. Add a focused test forchatgpt/captureandofficial-site/observethat asserts the exactadapters/<site>/<command>.jsargument and the returnedcapability_source_commit.Also applies to: 733-735
🤖 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/agent_server.py` around lines 168 - 183, Add focused regression coverage for _runtime_lineage using chatgpt/capture and official-site/observe, mocking subprocess execution as needed to exercise the git log path rather than channel.collect. Assert each invocation uses the exact adapters/<site>/<command>.js path argument and verify the result exposes the expected capability_source_commit.
🤖 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 `@agent/Dockerfile`:
- Around line 8-12: Align the Node.js version consistently across the Dockerfile
and related install/runtime configuration: either keep Node.js 20 and update all
documented 22+ requirements, warnings, and comments, or change this Dockerfile
to Node.js 22+ while preserving the existing documented requirement. Use the
existing Node.js version symbols and requirement checks in the install scripts
and runtime configuration; do not leave conflicting version expectations.
In `@backend/config.py`:
- Around line 124-128: Update the configuration defaults for agent_http_timeout
and agent_ws_timeout to exceed opencli_timeout=240 with transport overhead,
while preserving their existing roles. Also update configuration validation so
environment-provided overrides for both timeouts must remain greater than
opencli_timeout.
- Around line 124-128: The Settings model around the ohmyopencli_root field must
restore or replace the removed dify_graphon_runtime_url,
dify_graphon_timeout_seconds, invokeai_*, and kats_runtime_* configuration
attributes. Prefer migrating the consumers in dify_imports.py,
worker_runtime.py, image_studio.py, and kats_runtime.py to the new configuration
source; otherwise add the required fields back so direct settings access remains
valid.
- Line 114: Align the noVNC port defaults across the configuration and
deployment paths: update novnc_base_port and the corresponding
NOVNC_PORT/NOVNC_BASE_PORT defaults in Docker Compose and scripts/chrome-pool.sh
to the same value, or change novnc_base_port to 6080 to match the existing
exposed port. Ensure the API’s advertised endpoint matches the first published
noVNC port.
In `@scripts/install-agent.sh`:
- Around line 334-345: Update the managed-runtime installation block around
OHMYOPENCLI_ROOT to require npm before cloning, alongside the existing git
prerequisite check. Fail via die with an appropriate npm-required message before
git clone can create the target directory, while preserving the existing
installation flow afterward.
In `@tests/unit/test_agent_image_runtime_packaging.py`:
- Around line 57-58: Align the Windows installer parameter declaration for
OhMyOpenCliRepo with the test’s expected contract: make it non-mandatory and
assign the default repository URL asserted by the test. Update the parameter
definition in the installer script while preserving the existing clone flow.
---
Nitpick comments:
In `@backend/agent_server.py`:
- Around line 168-183: Add focused regression coverage for _runtime_lineage
using chatgpt/capture and official-site/observe, mocking subprocess execution as
needed to exercise the git log path rather than channel.collect. Assert each
invocation uses the exact adapters/<site>/<command>.js path argument and verify
the result exposes the expected capability_source_commit.
🪄 Autofix
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: dc4af281-95e6-4a9d-bfb3-36164a19c88c
📒 Files selected for processing (17)
Dockerfileagent/Dockerfilebackend/acquisition/capabilities.pybackend/acquisition/registry.pybackend/acquisition/runner.pybackend/agent_server.pybackend/browser_pool.pybackend/channels/opencli_channel.pybackend/config.pyscripts/install-agent.shscripts/install-managed-opencli.ps1scripts/verify_managed_opencli_runtime.pytests/unit/test_acquisition_capabilities.pytests/unit/test_acquisition_runner.pytests/unit/test_agent_image_runtime_packaging.pytests/unit/test_browser_pool.pytests/unit/test_managed_opencli_verifier.py
Comment on lines
8
to
12
| # Base system deps (always installed) | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| curl ca-certificates git procps \ | ||
| && curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \ | ||
| && curl -fsSL https://deb.nodesource.com/setup_20.x | bash - \ | ||
| && apt-get install -y --no-install-recommends nodejs \ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '8,13p' agent/Dockerfile
npm view `@jackwener/opencli`@1.8.5 engines --json
curl -fsSL \
https://raw.githubusercontent.com/2233admin/OhMyOpenCLI/54f729d6447238918c4098e322d80b1c64ab8a7a/package.json \
| jq '{engines, packageManager}'Repository: 2233admin/opencli-Razormind
Length of output: 857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== agent Dockerfile relevant lines =="
cat -n agent/Dockerfile | sed -n '1,90p'
echo
echo "== locate opencli references =="
rg -n "opencli|OpenCLI|setup_20\.x|setup_22\.x|`@jackwener/opencli`|opencli-Razormind|OhMyOpenCLI" -S .
echo
echo "== package manager and node versions if present =="
[ -f package.json ] && jq '.packageManager // {}, .engines // {}' package.json
for f in package-lock.json npm-shrinkwrap.json pnpm-lock.yaml yarn.lock; do
[ -f "$f" ] && echo "--- $f" && rg -n "`@jackwener/opencli`|OhMyOpenCLI|node:|nodejs " "$f" || true
doneRepository: 2233admin/opencli-Razormind
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Dockerfiles =="
fd -i 'Dockerfile$' . -x sh -c 'echo "--- $1"; sed -n "1,110p" "$1"' sh {}
echo
echo "== exact OpenCLI package references in tracked files =="
rg -n "`@jackwener/opencli`|ohmyopencli|OhMyOpenCLI|OpenCLI|opencli/" -S --glob '!node_modules' --glob '!coverage' --glob '!dist' --glob '!build' --glob '!*.lock' . | sed -n '1,200p'
echo
echo "== npm metadata =="
npm view `@jackwener/opencli`@1.8.5 --jsonRepository: 2233admin/opencli-Razormind
Length of output: 50383
Keep the Node.js 20 runtime or align it with documented OpenCLI requirements.
agent/Dockerfile installs Node.js 20 alongside OpenCLI 1.8.5, while other install/runtime configs require Node.js 22+. Since OpenCLI 1.8.5 declares engines.node >=20.0.0, use either Node.js 20 consistently or update every install-agent.sh/runtime requirement, install script warning, and Docker comment to remove the documented 22+ constraint.
🤖 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 `@agent/Dockerfile` around lines 8 - 12, Align the Node.js version consistently
across the Dockerfile and related install/runtime configuration: either keep
Node.js 20 and update all documented 22+ requirements, warnings, and comments,
or change this Dockerfile to Node.js 22+ while preserving the existing
documented requirement. Use the existing Node.js version symbols and requirement
checks in the install scripts and runtime configuration; do not leave
conflicting version expectations.
| # noVNC base port for the first agent instance (agent-1). Additional | ||
| # instances use base+1, base+2, … Matches docker-compose NOVNC_PORT. | ||
| novnc_base_port: int = 6080 | ||
| novnc_base_port: int = 3010 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -i 'novnc_base_port|NOVNC_PORT|6080|3010' . || trueRepository: 2233admin/opencli-Razormind
Length of output: 8977
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- backend/config.py relevant lines ---\n'
sed -n '100,120p' backend/config.py
printf '\n--- backend/api/v1/workers.py relevant lines ---\n'
sed -n '60,115p' backend/api/v1/workers.py
printf '\n--- chrome Docker/entrypoint relevant lines ---\n'
sed -n '30,45p' chrome/Dockerfile
sed -n '1,55p' chrome/entrypoint.sh
printf '\n--- docker-compose noVNC/port mapping relevant lines ---\n'
sed -n '168,235p' docker-compose.yml
sed -n '312,324p' docker-compose.yml
printf '\n--- install scripts noVNC/Frontend references ---\n'
sed -n '75,100p' scripts/install.sh
sed -n '1,105p' scripts/chrome-pool.shRepository: 2233admin/opencli-Razormind
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- backend/config.py relevant lines ---'
sed -n '100,120p' backend/config.py
printf '%s\n' '--- backend/api/v1/workers.py relevant lines ---'
sed -n '60,115p' backend/api/v1/workers.py
printf '%s\n' '--- chrome Docker/entrypoint relevant lines ---'
sed -n '30,45p' chrome/Dockerfile
sed -n '1,55p' chrome/entrypoint.sh
printf '%s\n' '--- docker-compose noVNC/port mapping relevant lines ---'
sed -n '168,235p' docker-compose.yml
sed -n '312,324p' docker-compose.yml
printf '%s\n' '--- install scripts noVNC/Frontend references ---'
sed -n '75,100p' scripts/install.sh
sed -n '1,105p' scripts/chrome-pool.sh
printf '%s\n' '--- docker-compose contract test ---'
sed -n '1,55p' tests/unit/test_public_release_contract.pyRepository: 2233admin/opencli-Razormind
Length of output: 15954
Publish a noVNC 3010 default or accept the 6080 default in the API.
backend/config.py defaults novnc_base_port to 3010, while Docker Compose and scripts/chrome-pool.sh still default the first noVNC port to 6080. Either set NOVNC_PORT/NOVNC_BASE_PORT to 3010 everywhere, or keep the API default at 6080 so the advertised endpoint matches the exposed noVNC port.
🤖 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/config.py` at line 114, Align the noVNC port defaults across the
configuration and deployment paths: update novnc_base_port and the corresponding
NOVNC_PORT/NOVNC_BASE_PORT defaults in Docker Compose and scripts/chrome-pool.sh
to the same value, or change novnc_base_port to 6080 to match the existing
exposed port. Ensure the API’s advertised endpoint matches the first published
noVNC port.
Comment on lines
+124
to
+128
| opencli_timeout: int = 240 | ||
| # Managed acquisition runtime. The commit/version are code-owned pins; | ||
| # this path merely locates the installed checkout on every platform. | ||
| ohmyopencli_root: str = "/opt/ohmyopencli" | ||
| # browser-act CLI subprocess execution timeout, per call (Browser Act integration PR-B). | ||
| # browser-act CLI subprocess execution timeout, per call (GOAL-7 PR-B). |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Raise remote dispatch timeouts with opencli_timeout.
opencli_timeout is now 240 seconds. agent_http_timeout and agent_ws_timeout remain 130 seconds. The supplied ChatGPT capture test in tests/unit/test_acquisition_runner.py Lines 475-531 already passes an OpenCLI timeout of 180 seconds. In agent mode, the center can therefore time out before the agent completes the command, while backend/agent_server.py still allows 240 seconds.
Set both remote dispatch timeouts above opencli_timeout with transport overhead, and validate environment overrides as well.
🤖 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/config.py` around lines 124 - 128, Update the configuration defaults
for agent_http_timeout and agent_ws_timeout to exceed opencli_timeout=240 with
transport overhead, while preserving their existing roles. Also update
configuration validation so environment-provided overrides for both timeouts
must remain greater than opencli_timeout.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -i 'dify|graphon|kats|invokeai|image[-_ ]studio' . || trueRepository: 2233admin/opencli-Razormind
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- backend/config.py around SettingsConfigDict/removed fields ---\n'
sed -n '1,200p' backend/config.py
printf '\n--- tracked py files matching image-studio/invoke/dify config-ish settings ---\n'
rg -n --glob '*.py' 'image[-_ ]?studio|invoke[-_ ]?ai|kats|graphon|dify[-_ ]?runtime|dify[-_ ]?api|opencli[-_ ]?graphon|DIFY|GRAPHON|KATS|INVOKE|INVOK' backend tests integration tests compat -S || true
printf '\n--- tracked files mentioning removed environment/settings keys explicitly ---\n'
rg -n 'IMAGE_STUDIO|INVOKEAI|SIDE|SIDE_|GRAPHON|DIFY_GRAPHON|DIFY_API|DIFY_.*ENDPOINT|KATS|RUNTIME' backend tests integration tests compat .env* deploy scripts docs manifests pyproject.toml -S || trueRepository: 2233admin/opencli-Razormind
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf -- '%s\n' '--- backend/config.py around SettingsConfigDict/removed fields ---'
sed -n '1,230p' backend/config.py
printf -- '%s\n' ''
printf -- '%s\n' '--- tracked py files matching image-studio/invoke/dify config-ish settings ---'
rg -n --glob '*.py' 'image[-_ ]?studio|invoke[-_ ]?ai|kats|graphon|dify[-_ ]?runtime|dify[-_ ]?api|opencli[-_ ]?graphon|DIFY|GRAPHON|KATS|INVOKE|INVOK' backend tests integration tests compat -S || true
printf -- '%s\n' ''
printf -- '%s\n' '--- tracked files mentioning removed environment/settings keys explicitly ---'
rg -n 'IMAGE_STUDIO|INVOKEAI|SIDE|SIDE_|GRAPHON|DIFY_GRAPHON|DIFY_API|DIFY_.*ENDPOINT|KATS|RUNTIME' backend tests integration tests compat .env* deploy scripts docs manifests pyproject.toml -S || trueRepository: 2233admin/opencli-Razormind
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf -- '%s\n' '--- exact removed setting field references in tracked Python ---'
rg -n --glob '*.py' 'DifyRuntime|DifyGraphon|DIFY|GraphonRuntime|KatsRuntime|KATS|ImageStudio|InvokeAI|invokeai|runtime_status|RuntimeStatus|sidecar|Sandbox|sandbox|api_token|base_url|request_timeout_seconds|max_request_bytes|max_output_bytes|concurrent_runs|stored_runs|run_retention|api_key|auth_token|plugin_folder|plugin|side|SIDE' backend tests compat | grep -Ei 'DIFY|GRAPHON|KATS|INVOKE|IMAGE|SIDE|RUNTIME|SAND' | head -n 200 || true
printf -- '%s\n' ''
printf -- '%s\n' '--- config field pattern across tracked repo files ---'
python3 - <<'PY'
import re, pathlib
names = ['dify_runtime', 'dify_graphon', 'dify_graphon_runtime', 'dify_runtime_status', 'runtime_status',
'dify_graphon_url', 'dify_graphon_base_url', 'dify_graphon_endpoint', 'dify_graphon_host',
'kats_runtime', 'kats_runtime_url', 'kats_runtime_endpoint',
'invokeai_enabled', 'invokeai_base_url', 'invokeai_api_token', 'invokeai_request_timeout_seconds',
'invoke_ai_enabled', 'invoke_ai_base_url', 'invoke_ai_api_token', 'invoke_ai_request_timeout_seconds',
'image_studio_enabled', 'studio_enabled', 'image_studio_api_url', 'image_studio_api_key']
terms = [re.compile(n, re.I) for n in names]
for p in pathlib.Path('.').rglob('*'):
if not p.is_file() or '.git' in p.parts:
continue
if p.suffix not in {'.py','.toml','.env','.yml','.yaml','.json','.md'}:
continue
try:
text = p.read_text(errors='ignore')
except Exception:
continue
hits = [n for n in names if any(term.search(text) for term in terms)]
if hits:
print(p, hits)
PY
printf -- '%s\n' ''
printf -- '%s\n' '--- focused image-studio config/service imports/fields ---'
rg -n --glob '*.py' 'invokeai_enabled|invokeai_base_url|invokeai_api_token|invokeai_request_timeout_seconds|image_studio|InvokeAI' backend/backend api tests compat | head -n 250 || trueRepository: 2233admin/opencli-Razormind
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf -- '%s\n' '--- Python files matching image-studio config pattern ---'
rg -n --glob '*.py' 'InvokeAIClient\(|invokeai_enabled|invokeai_base_url|invokeai_api_token|invokeai_request_timeout_seconds|DifyGraphonClient\(|DifyGraphonRuntime|DifyRuntime' backend backend tests compat -S | head -n 250 || true
printf -- '%s\n' ''
printf -- '%s\n' '--- Python files matching Kats config pattern ---'
rg -n --glob '*.py' 'KatsRuntime\(|Kats.*Runtime|kats_runtime_url|kats_runtime_endpoint|KATS_.*URL|KATS_.*HOST|KATS_.*TOKEN' backend backend tests compat -S | head -n 200 || true
printf -- '%s\n' ''
printf -- '%s\n' '--- Python Settings model references that read these sidecar domains ---'
rg -n --glob '*.py' 'get_settings\(\)|get_settings\.dify|settings\.dify|settings\.graphon|settings\.kats|settings\.image_studio|settings\.invoke|settings\.invokeai' backend backend tests compat -S | head -n 200 || trueRepository: 2233admin/opencli-Razormind
Length of output: 23823
Restore missing sidecar settings before removing them from config.
Direct code uses still read settings.dify_graphon_runtime_url, settings.dify_graphon_timeout_seconds, settings.invokeai_*, and settings.kats_runtime_* in backend/api/v1/dify_imports.py, backend/image_studio/worker_runtime.py, backend/api/v1/image_studio.py, and backend/workflow/kats_runtime.py. Add these fields back or migrate the consumers to the new config source; extra="ignore" only hides old env vars, it does not supply missing attributes.
🤖 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/config.py` around lines 124 - 128, The Settings model around the
ohmyopencli_root field must restore or replace the removed
dify_graphon_runtime_url, dify_graphon_timeout_seconds, invokeai_*, and
kats_runtime_* configuration attributes. Prefer migrating the consumers in
dify_imports.py, worker_runtime.py, image_studio.py, and kats_runtime.py to the
new configuration source; otherwise add the required fields back so direct
settings access remains valid.
Comment on lines
+334
to
+345
| # Install the exact project-owned managed-acquisition capability package. | ||
| OHMYOPENCLI_ROOT="$AGENT_DIR/ohmyopencli" | ||
| OHMYOPENCLI_COMMIT="54f729d6447238918c4098e322d80b1c64ab8a7a" | ||
| OFFICIAL_SITE_CAPABILITY_COMMIT="73cc60c83586ef2c95469b3b70d6cfc80fa5bc53" | ||
| 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 | ||
| (cd "$OHMYOPENCLI_ROOT" && npm ci && npm run bootstrap) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail before partial installation when npm is unavailable.
The prior branch only warns when npm is missing, but Line 345 invokes npm ci unconditionally. The installer then fails after it creates $OHMYOPENCLI_ROOT. A later run rejects that directory at Lines 339-340. Require npm before cloning the managed runtime.
Proposed fix
# Install the exact project-owned managed-acquisition capability package.
+ command -v npm >/dev/null 2>&1 || die \
+ "npm is required to install OhMyOpenCLI"
OHMYOPENCLI_ROOT="$AGENT_DIR/ohmyopencli"📝 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
| # Install the exact project-owned managed-acquisition capability package. | |
| OHMYOPENCLI_ROOT="$AGENT_DIR/ohmyopencli" | |
| OHMYOPENCLI_COMMIT="54f729d6447238918c4098e322d80b1c64ab8a7a" | |
| OFFICIAL_SITE_CAPABILITY_COMMIT="73cc60c83586ef2c95469b3b70d6cfc80fa5bc53" | |
| 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 | |
| (cd "$OHMYOPENCLI_ROOT" && npm ci && npm run bootstrap) | |
| # Install the exact project-owned managed-acquisition capability package. | |
| command -v npm >/dev/null 2>&1 || die \ | |
| "npm is required to install OhMyOpenCLI" | |
| OHMYOPENCLI_ROOT="$AGENT_DIR/ohmyopencli" | |
| OHMYOPENCLI_COMMIT="54f729d6447238918c4098e322d80b1c64ab8a7a" | |
| OFFICIAL_SITE_CAPABILITY_COMMIT="73cc60c83586ef2c95469b3b70d6cfc80fa5bc53" | |
| 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 | |
| (cd "$OHMYOPENCLI_ROOT" && npm ci && npm run bootstrap) |
🤖 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 334 - 345, Update the managed-runtime
installation block around OHMYOPENCLI_ROOT to require npm before cloning,
alongside the existing git prerequisite check. Fail via die with an appropriate
npm-required message before git clone can create the target directory, while
preserving the existing installation flow afterward.
Comment on lines
+57
to
+58
| assert '[string]$OhMyOpenCliRepo = "https://github.com/2233admin/OhMyOpenCLI.git"' in windows | ||
| assert "git clone $OhMyOpenCliRepo $OhMyOpenCliRoot" in windows |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the Windows installer parameter with this assertion.
scripts/install-managed-opencli.ps1 declares $OhMyOpenCliRepo as mandatory and provides no default value. Line 57 expects a default repository URL. The test fails against the current installer.
Remove Mandatory = $true and set the asserted default, or change this test and the documented installer contract to require an explicit repository value.
🤖 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 57 - 58, Align
the Windows installer parameter declaration for OhMyOpenCliRepo with the test’s
expected contract: make it non-mandatory and assign the default repository URL
asserted by the test. Update the parameter definition in the installer script
while preserving the existing clone flow.
lunnynight
force-pushed
the
main
branch
3 times, most recently
from
August 7, 2026 10:17
aeff3e0 to
1b5d647
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync local main to upstream after rebase on latest origin/main (94ab53d). Includes ChatGPT managed acquisition feature (37e9e9d).