@qwen-code /triage

@qwen-code /triage

Thanks @tt-a1i for the fix!

Template looks good ✓ — all required sections are filled in, including the bilingual Chinese translation. The prior Stage 1 gate flagged the old template format; the updated body addresses every point.

On direction: this is a straightforward bug fix for #5324. The sandbox image-name parser breaks on registry ports (localhost:5000/… → localhost-5000 instead of the image basename). Clearly within scope — sandbox configuration is a core CLI feature, and this is a P3 bug. No CHANGELOG reference needed for a targeted fix like this.

On approach: the scope is tight and appropriate. Three files, all related to the fix:

• Extract the private parseImageName into a testable parseSandboxImageName helper — justified because the old function was untestable inside sandbox.ts.
• Fix the parsing logic (last : after last / instead of naive split(':')).
• Add digest stripping as a bonus correctness improvement.
• Five unit tests covering the key cases.

No scope creep, no drive-by refactors. This is what a focused bugfix PR should look like.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

Code Review

Reviewed the diff independently. The fix is correct and clean:

• Parsing logic — finding the last : after the last / is the right way to distinguish registry ports from tags. Verified against all five test cases including edge cases (untagged ports, digest-only, tag+digest).
• Digest stripping — image.split('@')[0] is simple and correct. The ?? image fallback is technically unreachable (split always returns at least one element), but harmless.
• Extraction — moving the function out of sandbox.ts into sandboxImageName.ts is justified for testability. The old parseImageName was a private function with no test coverage.
• No regressions — the call site in sandbox.ts line 606 is the only consumer. One-line swap, no other changes to sandbox.ts.
• Tests — five cases cover the key scenarios: standard tagged image, registry port with tag, registry port without tag, digest stripping, and tag+digest combo. All pass.

No blockers. No AGENTS.md violations. The code is minimal, focused, and does exactly what the PR says.

Test Results

Unit Tests

 ✓ src/utils/sandbox.test.ts (5 tests) 5ms

 Test Files  1 passed (1)
      Tests  5 passed (5)

Typecheck & Lint

• npm run typecheck --workspace=packages/cli ✅
• npm run lint ✅

Before/After Behavior

Ran the old parseImageName vs the new parseSandboxImageName side by side:

=== Sandbox Image Name Parser: Before vs After (PR #5325) ===

Input:  ghcr.io/qwenlm/qwen-code:0.18.3
Before: qwen-code-0.18.3
After:  qwen-code-0.18.3  [SAME]

Input:  localhost:5000/team/qwen-code:dev
Before: localhost-5000/team/qwen-code
After:  qwen-code-dev  [FIXED]

Input:  localhost:5000/team/qwen-code-sandbox
Before: localhost-5000/team/qwen-code-sandbox
After:  qwen-code-sandbox  [FIXED]

Input:  registry.example.com/team/qwen-code-sandbox@sha256:abcdef
Before: qwen-code-sandbox@sha256-abcdef
After:  qwen-code-sandbox  [FIXED]

Input:  registry.example.com/team/qwen-code-sandbox:dev@sha256:abcdef
Before: qwen-code-sandbox-dev@sha256
After:  qwen-code-sandbox-dev  [FIXED]

The old parser was even worse than the issue described — it didn't just lose the tag on localhost:5000/…:dev, it produced localhost-5000/team/qwen-code (with slashes, which Docker rejects in container names). The fix resolves all of these correctly.

Note: full E2E sandbox testing (actual Docker container creation) requires a Docker daemon and sandbox configuration, which isn't available in this CI environment. The function is only called during container creation, so the unit tests + behavioral demo above cover the logic completely.

— Qwen Code · qwen3.7-max

Reflection

Stepping back to look at the whole picture:

This PR fixes a genuine bug (#5324) that breaks sandbox container creation for anyone using a private registry with a port — a common setup in enterprise environments. The old parser was even more broken than the issue described: it produced container names with slashes (Docker-invalid) in addition to losing the tag.

My independent proposal before reading the diff was the same approach: find the last : after the last /, extract the basename, handle digests. The PR matches this exactly — no extra abstraction, no scope creep, no unnecessary files.

The code is 21 lines of logic plus 44 lines of tests. Three files changed, all necessary. If I had to maintain this in six months, I'd thank the author for making it trivially readable and well-tested.

Everything checks out:

• Template ✅
• Direction ✅
• Scope ✅ (minimal and focused)
• Code review ✅ (correct, no blockers)
• Unit tests ✅ (5/5 pass)
• Typecheck ✅
• Lint ✅
• Before/after behavior ✅ (verified via demo)

Approving. ✅

— Qwen Code · qwen3.7-max

✅ Maintainer verification — real sandbox-launch test under tmux

Verified this PR locally by driving the actual qwen sandbox launch with a private-registry image and observing the real derived container name (tmux + a fake docker shim, since this host has no Docker), plus the unit suite. Verdict: a correct fix that turns a hard failure into working behavior — good to merge. I add a 🐧 Linux runtime check on top of the PR's CI coverage.

Environment

• Isolated git worktree at the PR head (a8382207, on current upstream main), fresh npm ci (no symlinked node_modules), real esbuild bundle.
• Linux · Node v22.22.2 · bundled qwen 0.18.3.
• Scope is clean: exactly 3 files, +67 / −9, single commit (the env-var/comment changes visible in a 3-dot diff are unrelated upstream commits, not this PR).

1. Static checks (reproducing the PR test plan)

2. Why this matters (the bug)

Sandbox container names are derived from the image reference. The old image.split(':') treats the first : as the tag separator — but a private registry host like localhost:5000 contains a : for its port. So the derived name keeps the port and the registry path including /. Docker container names may only contain [a-zA-Z0-9][a-zA-Z0-9_.-]+, so the old name is rejected by Docker — i.e. the sandbox fails to start for any port-qualified registry image (not just a cosmetic naming issue).

3. Real sandbox-launch A/B in tmux

Ran the real qwen -p hi with QWEN_SANDBOX=docker and QWEN_SANDBOX_IMAGE=localhost:5000/team/qwen-code:dev. A fake docker on PATH satisfies the image/network preflight so the real start_sandbox reaches its container-naming step and emits the real docker run --name … — no container is actually run.

OLD (pre-PR):  ContainerName (regular): localhost-5000/team/qwen-code-0   <- port kept + contains '/'  => Docker would REJECT
               docker run … --name localhost-5000/team/qwen-code-0

NEW (this PR): ContainerName (regular): qwen-code-dev-0                   <- clean, valid container name
               docker run … --name qwen-code-dev-0

The name appears both on stderr and in the actual docker run --name argument. (Note: the real localhost:5000/team/qwen-code:dev resolves to localhost-5000/team/qwen-code under the old parser — even worse than the localhost-5000 cited in the description, because it also keeps the /team/qwen-code path.)

4. Function-level A/B (real parseSandboxImageName vs the pre-PR split(':'))

Every previously-correct case is unchanged (no regression); only the broken port/digest cases change — and they change from Docker-invalid names to valid ones.

Observations (non-blocking)

1. Real-world severity: because the old names contained / (and @ for digests), Docker would reject them — so this is a hard "sandbox won't start" bug for port-qualified private-registry images, not just an ugly name. The fix restores functionality.
2. Good refactor: the logic is now a small, pure, exported function with direct unit coverage; only the parser changed, and start_sandbox is otherwise untouched.
3. Cross-platform: pure string logic, platform-agnostic. PR is CI-green on macOS/Windows/Linux; I additionally exercised the real Linux sandbox flow end-to-end.

旧版（PR 前）: ContainerName (regular): localhost-5000/team/qwen-code-0   <- 保留端口且含 '/' => Docker 会拒绝
              docker run … --name localhost-5000/team/qwen-code-0

新版（本 PR）: ContainerName (regular): qwen-code-dev-0                   <- 干净、合法的容器名
              docker run … --name qwen-code-dev-0