review(windows): pre-upstream native ACP lifecycle - #1
Draft
smallgun01 wants to merge 23 commits into
Draft
Conversation
Bound Unix shell post-kill pipe joins so timeout cannot hang on setsid descendants. Re-apply the restricted DACL after ReplaceFileW, create the auth temp with an explicit security descriptor, and take the trustee from the process token SID. Keep the Job Object controller alive for later terminate callers. Trigger the Windows gate on pull_request/main path filters and derive canary provenance from github context.
The Windows ACL test had two #[test] attributes, which fails to compile on Windows CI. Bound completed-path pipe joins as well so a setsid descendant cannot hang a normal exit.
- F7: document working_dir sandbox rejection on all platforms in PR body - F8: extract shared WINDOWS_RUNTIME_ENV_KEYS constant (openab-core -> agent) - F9: assert no-provider/no-credentials failure shape in canary - F10: preserve pipe bytes + truncation marker on bounded pipe join expiry
…bdev#1506) These are ARN-parsing tests, so the ARN shape is what they exercise — the account and secret names were incidental, but real: - account 903779448426 -> 123456789012 (AWS's documentation account) - oab/telegram/pahudxbot[-AC80TP] -> example/telegram/bot[-AbCdEf], matching the appauthexample-AbCdEf convention already used a few tests further down No secret *value* was ever present here — only the ARN of the secret holding the Telegram bot token, so nothing needs rotating. But the secret's name told a reader which secrets this account holds, and test fixtures have no reason to carry it. Every input/expected pair was updated together, so each test asserts exactly the same parsing behaviour as before. Existing history is deliberately left alone: the repo has 195 forks and GitHub retains orphaned commits, so a rewrite would break 25 open PRs and every clone without actually un-publishing anything. Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
CI clippy 1.98 -D warnings fails on print_box's redundant
format!("{}", line) where line is already &&str.
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
2 participants
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.
Review Contract
Goal
Close the internal review blockers for the Windows native ACP lifecycle spike before any upstream PR is opened. The code remains a non-release spike and must continue to be reviewed against the clean upstream pin
280db4d.Non-goals
Accepted Residual Risks
Acceptance Criteria
acp/process_tree.rsand exposes one platform-neutral handle.ERROR_FILE_NOT_FOUND.APPDATAandLOCALAPPDATA.Follow-ups
40a6486.