Windows stability changes - #26
Conversation
|
Thanks for this, David — real effort, and the cross-platform direction is genuinely useful. A few things before it can land, and one honest blocker: The honest blocker: the core reason delegation fails on native Windows is upstream — agy's own headless ( To move it forward:
Really appreciate you pushing on Windows — it's the most-requested gap. I just want to be straight that until #508 is fixed upstream, we can enable the scaffolding but not truly "make delegation work" on native Windows, and I'd rather frame/ship it that way than overpromise. |
…-only Unix users, and remove extra slash commands from PR yuting0624#26
Found independently by both reviewers on this PR. github.event.pull_request does not exist on issue_comment, so 'null != true' coerces to TRUE and a fork passes the guard -- on the one trigger where a maintainer typing '@quorum /review' on someone else's PR is an ordinary thing to do. That would check the fork's merge commit out at the workspace root with live Google Cloud and write-scoped GitHub credentials, which is what the comment above it says cannot happen. The upstream example workflow has the same hole. Resolved against the API in a separate job rather than trusting the payload shape. It runs only on issue_comment and review uses always() so a skip is not a block. Verified against real pull requests: same-repo #41 passes, fork #26 is rejected by name.
Caught by the review of the previous fix, and it matters immediately: the plan for this PR is to dispatch against a PR number by hand. github.event.pull_request exists on pull_request and pull_request_review_comment and nowhere else, so the same null-coercion that let a fork through on issue_comment lets one through on workflow_dispatch. Dispatch needs write access to fire, which makes it a smaller exposure than the comment path -- but that is not a reason to guard one and not the other, and the asymmetry was an oversight rather than a decision. not-a-fork now covers both, resolving the number from either github.event.issue.number or github.event.inputs.pr, and refusing a non-numeric or empty one rather than passing it to the API. Verified: #41 (same repo) rc=0, #26 (fork) rc=1, empty rc=1.
…ecrets exist) (#41) * ci: add quorum-review as a second, independent scan claude-review.yml is one Opus scan carrying this repository's contracts. It is good at drift and at things only this repo cares about, but it cannot exceed one model's recall -- a second opinion only ever sees findings already reported, so a bug the scan walked past is never put in front of it. Two independent scans is the only arrangement that raises the ceiling; quorum-review runs Gemini and Claude on one Google Cloud credential and spends the second opinion on the disagreements. No long-lived secret: OIDC is exchanged for short-lived credentials via Direct Workload Identity Federation. WIF_PROVIDER and GOOGLE_CLOUD_PROJECT name an identity, they are not credentials. Three choices worth stating: - Built-in skill, unmodified. This goes on trial against a known answer key (PR #40 is the pre-fix commit of #36, holding three bugs the existing review found). A skill written to match that key would find it by construction. - Region pinned to global on both models, measured not assumed: claude-sonnet-5 returns FAILED_PRECONDITION 'not servable in region' in us-central1 for this project, and serves in global. - Action pinned to a commit, not a tag -- this job holds a write-scoped token, and quorum-review's own fork example gives exactly this advice. No synchronize trigger and no fail-on gate while on trial; '@quorum /review' runs it on demand. Does nothing until WIF_PROVIDER and GOOGLE_CLOUD_PROJECT exist, so do not merge before the setup lands. * ci: the fork guard didn't cover the issue_comment trigger Found independently by both reviewers on this PR. github.event.pull_request does not exist on issue_comment, so 'null != true' coerces to TRUE and a fork passes the guard -- on the one trigger where a maintainer typing '@quorum /review' on someone else's PR is an ordinary thing to do. That would check the fork's merge commit out at the workspace root with live Google Cloud and write-scoped GitHub credentials, which is what the comment above it says cannot happen. The upstream example workflow has the same hole. Resolved against the API in a separate job rather than trusting the payload shape. It runs only on issue_comment and review uses always() so a skip is not a block. Verified against real pull requests: same-repo #41 passes, fork #26 is rejected by name. * ci: post as the Quorum Review App, and pin every action in the job App token: comments arrive under the App's name instead of github-actions[bot]. Minted per run by actions/create-github-app-token and expires with it. The App holds metadata:read, contents:read, pull_requests:write and has no webhook -- no listener, and nothing that can push a commit. Note that 'permissions:' only governs GITHUB_TOKEN; the reviewer's API calls now run at the App installation's ceiling instead. Pinning, from review: the rationale I wrote for pinning quorum-review to a commit -- a tag can be moved, and this job holds a write-scoped token -- applies identically to actions/checkout, google-github-actions/auth and codeql-action sitting in the same job. All pinned; Dependabot bumps them. Also from review: persist-credentials: false on the primary checkout, which the /head fallback already had. Requires two new repository settings before it can run: vars.QUORUM_APP_ID (4452711 -- not a secret) secrets.QUORUM_APP_PRIVATE_KEY * ci: the fork guard was vacuous on workflow_dispatch too Caught by the review of the previous fix, and it matters immediately: the plan for this PR is to dispatch against a PR number by hand. github.event.pull_request exists on pull_request and pull_request_review_comment and nowhere else, so the same null-coercion that let a fork through on issue_comment lets one through on workflow_dispatch. Dispatch needs write access to fire, which makes it a smaller exposure than the comment path -- but that is not a reason to guard one and not the other, and the asymmetry was an oversight rather than a decision. not-a-fork now covers both, resolving the number from either github.event.issue.number or github.event.inputs.pr, and refusing a non-numeric or empty one rather than passing it to the API. Verified: #41 (same repo) rc=0, #26 (fork) rc=1, empty rc=1. * ci: not-a-fork needs pull-requests:read, or it 403s and blocks everything Caught in review, and it would have broken the very next thing planned: the job declares contents:read but calls 'gh api repos/OWNER/REPO/pulls/N', which needs pull-requests:read. The failure is not a skipped check -- the job 403s, 'review' requires needs.not-a-fork.result != 'failure', and the review becomes permanently unreachable on workflow_dispatch and @quorum comments, the two triggers the guard exists to protect. My 'verified against real pull requests' note tested the logic with a local gh session, which carries far more scope than GITHUB_TOKEN does in this job, so it could not have surfaced this. Recording that in the file next to the permission, because the same mistake is easy to repeat. * ci: quorum-review was pinned to a tag object, not a commit Found while checking the review's note that it could not verify the pinned SHAs from its sandbox. c87fe72 is the annotated *tag object* for v1.6.2; it dereferences to commit a289f48. Actions resolved it -- the first run on this PR used it and succeeded -- and it is immutable in the same way a commit is, since a tag object's SHA covers its target. But it is not what 'pinned to a commit' means to a reader, 'git checkout c87fe72' does not give you the tree, and SHA-pinning tooling expects a commit. Repinned to a289f48. The other four were already commits; all five now verified to dereference to their trailing version comment. * ci: name codeql-action's exact release in the pin comment v3 is a moving tag; the pinned commit is v3.37.4. Say so, so the pin stays auditable after the tag moves on. (An earlier 'mismatch' on this line was a path-expansion bug in my own check, not a bad pin -- all five verified.)
Bumps the pin to v1.7.0 (9d3edce), which carries the three fixes from #14/#15/#16. Also replaces our separate not-a-fork job with upstream's pre-checkout step. Not churn -- ours had a hole theirs cannot have. 'needs.<job>.result != failure' is true for 'cancelled' as well as 'success' and 'skipped', so a cancelled guard job would have let the review job proceed. A step has no such state to get wrong, it runs before anything is cloned and before any credential is minted, and staying close to upstream keeps future updates cheap. Kept two things from ours that upstream's example does not have: the numeric check on the PR number before it reaches the API, and naming claude-review-external.yml as the place forks are reviewed on purpose. Verified against real pull requests: #39 (same repo) rc=0, #26 (fork) rc=1, empty rc=1. Step order confirmed -- Refuse a fork runs first, ahead of both checkouts and the App/GCP credential steps.
PR #26 was the first real exercise of this workflow and it failed at checkout: Refusing to check out fork pull request code from a 'pull_request_target' workflow ... set 'allow-unsafe-pr-checkout: true' Not our logic -- authorise passed, timeline lookup and permission check both worked. actions/checkout added that input (default false) and now refuses the pattern this workflow was built on, which is the pattern behind most pwn-request reports because the next person adds a build step. Opting back in was available and defensible: tools here are restricted to reading and commenting, so there is no execution path today. Declined. 'No execution path' is an invariant someone has to keep re-establishing on every future edit of allowedTools, and not depending on that is what the guardrail is for. Today alone produced three guards that were silent no-ops. Removing the need is cheap here. A fork PR's unchanged files ARE the base repo's, which is already checked out at the root and readable, and gh pr diff carries the changed ones -- for a newly added file the diff is the whole file. On #26, 13 of 20 files are new, including the JS reimplementation of the security gate, so the diff contains essentially everything worth reviewing. Also pinned both actions to commits. This job holds a write-scoped token and an API credential; it was the only workflow left on floating tags.
Overview & Motivation
This PR resolves critical Windows platform issues and introduces key workflow additions to bring
antigravity-for-claude-codeto feature parity with other Claude Code assistant tools (such as Codex for Claude).Core Objectives
.shentrypoints cannot be natively invoked by Claude Code subagents./ask,/adversarial-review,/rescue), proactive subagent guardrails, and prompt-level policy hooks popular in Codex/Claude subagent workflows.Key Changes & Architectural Parity
1. Windows Native Execution & Job Daemonization
cmd.exeby default. Invoking.shscripts directly results in command resolution failures or headless process hangs due to stdin redirection issues..cmdExecutable Wrappers (bin/): Addedagy-delegate.cmd,agy-job.cmd,agy-doctor.cmd,cloud-debug.cmd,agy-trace.cmd, andagy-cost-compare.cmdto provide first-class Windows entrypoints.hooks/): Added JavaScript hook implementations (check-agy.js,inject-policy.js,nudge-delegation.js,validate-delegate-bash.js) that execute cross-platform via Node.js regardless of host shell.scripts/agy-job.sh): Hardened background job detachment, process check loops, and exit code propagation for Windows/PowerShell environments.2. Workflow & Tooling Parity (Codex for Claude Alignment)
To match capabilities expected from modern Claude Code subagent extensions, this update introduces dedicated workflow commands and delegation guardrails:
/antigravity:ask: Fast, single-turn query engine routed toflash-lofor rapid syntax lookups and quick questions without loading heavy workspace context./antigravity:adversarial-review: Adversarial cross-model audit mode that skeptically inspects code changes for edge cases, missing test coverage, and security vulnerabilities./antigravity:rescue: Automated build/test failure analysis tool that generates immediate actionable recovery steps when pipelines fail.nudge-delegation.js) and subagent execution validation (validate-delegate-bash.js) to enforce cost discipline and prevent accidental tool misuse.3. Model Default Alignment (Gemini 3.6 Flash)
flashandflash-lotier model strings fromGemini 3.5 FlashtoGemini 3.6 Flashinscripts/agy-delegate.sh,scripts/doctor.sh, and documentation.CLAUDE_PLUGIN_OPTION_TIER_FLASH.Verification & Test Results
tests/run-tests.sh): 136/136 tests passing cleanly (PASS=136 FAIL=0).