Repository navigation
fix(integration-tests): ack the daemon tool guard in the mock ACP child (#9160) #9162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code part adopted; PR-management part needs a maintainer.
This round adopted PR 9161's extra line: the mock child now also does
delete process.env[PRIVATE_EXTERNAL_TOOL_GUARD_PROVIDER_ENV](commitac2fa92abc). Verified against the real children first —acpAgent.tsandgemini.tsxboth consume and delete both guard markers, andqwen servepasses exactly these two envs viachildEnvOverrides(the token/capability envs are scrubbed daemon-side and never reach the child, so the fixture does not delete them). This PR's fixture change is now a strict superset of PR 9161's.Not actionable from this flow (no GitHub credentials): adding
Fixes #9159to the PR description and closing PR 9161, or the inverse. Recommendation: keep this PR, addFixes #9159to its description, and close PR 9161, since it now carries the complete fixture change. Leaving this thread open for the maintainer's call.Verification this round:
npm run build,npm run bundle,npm run typecheck,npm run lintall pass; both journal-recovery integration suites (the fixture's consumers) pass 3/3 against the bundled CLI withQWEN_SANDBOX=false.中文说明
代码部分已采纳;PR 管理部分需要维护者处理。
本轮已采纳 PR 9161 多出的那一行:mock 子进程现在同样执行
delete process.env[PRIVATE_EXTERNAL_TOOL_GUARD_PROVIDER_ENV](提交ac2fa92abc)。实现前已对照真实子进程核实——acpAgent.ts与gemini.tsx都会消费并删除这两个守卫标记,而qwen serve通过childEnvOverrides传递给子进程的恰好就是这两个环境变量(token/capability 环境变量在守护进程一侧即被清除、不会到达子进程,因此夹具不删除它们)。本 PR 的夹具改动现在是 PR 9161 的严格超集。本流程无法执行(没有 GitHub 凭据):在 PR 描述中添加
Fixes #9159并关闭 PR 9161,或反向操作。建议:保留本 PR、在其描述中添加Fixes #9159、关闭 PR 9161,因为本 PR 已包含完整的夹具改动。此线程保持打开,等待维护者决定。本轮验证:
npm run build、npm run bundle、npm run typecheck、npm run lint全部通过;两个 journal-recovery 集成测试套件(该夹具的消费方)在QWEN_SANDBOX=false下针对打包后的 CLI 运行,3/3 全部通过。