Repository navigation
test: exercise real gate authorization refusals - #29332
Conversation
…audit-runtime-construction
…ntime-construction
…test-audit-runtime-title-startup
…test-audit-chat-render-startup
…test-audit-runtime-close-startup
…at-render-startup
…at-render-startup
…st-audit-runtime-close-startup
…udit-bind-host-startup
…-audit-transcript-scroll-startup
…Win/test-audit-ssh-lease-startup
…-audit-gate-authorization-proof
…te-authorization-proof
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: adding tests that exercise real gate authorization refusals. |
| Description check | |
The description is comprehensive and follows the template. It explains the change, rationale, testing, scope, review status, and checklist completion. The Linked Issue section references issue #11745 … |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
Full details: Description check
-
Explanation
The description is comprehensive and follows the template. It explains the change, rationale, testing, scope, review status, and checklist completion. The Linked Issue section references issue
#11745but does not provide a direct issue link; this is a minor template gap.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
ELI5
A gate authorization test claimed to reject another Run's task but never called the operation. It now calls the real handler and proves the request fails without changing the task or creating a gate.
What Changed
Move the no-legacy-adoption case into the existing gate-handler suite, asserting the exact task_not_found response for the caller's Run and unchanged persisted state. Strengthen the existing revoked-caller gate case to require run_required instead of accepting any error except legacy_read_only. Production behavior is unchanged.
Why
The old setup-only case stayed green when authorization was deliberately broken. Reusing the existing handler harness covers ordinary current Runs without expanding the legacy-adoption dispatcher fixture; those dispatcher cases continue covering their separate transport and compatibility behavior. The exact-code assertion catches a misleading refusal that the old check accepted.
Linked Issue
No new issue; internal test coverage repair. References the existing #11745 regression suite.
Visual Proof
N/A — tests only; no visual or interaction change.
Testing
All 49 affected owner/sibling cases pass with pinned CI runtimes Bun1.4.3 and Node24.21.0. Dynamic collection preserves 48 case/file/project identities and replaces only the setup-only case with the real handler case, moving it from the Node project to the existing Bun owner. Node typecheck, targeted formatting, changed-code quality and diff checks pass.
Two deliberate production mutations proved the repaired tests matter: allowing cross-Run gates only when adoption is absent left the original tests green but failed the replacement; returning consumer_fenced instead of run_required left the original H2 gate case green but failed its new assertion. Production files were restored byte-for-byte before the final green run.
Three stable local timing pairs had median 15.063s before / 14.915s after, with one pair slower by 0.633s. No meaningful speed improvement claimed. A preliminary pair was excluded after dependency synchronization between its runs. After refreshing to main
5db05e8ed4, all 49 cases pass again in a fresh baseline/candidate pair and with shuffle104729. Fresh wall 20.661s → 21.177s, module 18.857s → 19.319s; this pair is slower. All local checks pass. The candidate and two production owners match the previously reviewed source exactly. Fresh exact-head hosted qualification has passed, as recorded below.AI Disclosure
Review
Independent preservation and mutation-evidence review approved without findings. All unrelated test bodies remain unchanged.
Agent skill upstream boundary
Notes
Tests only: production 0 lines changed; tests +29/-28. Existing adopted-Run and same-Run success cases remain. No SSH, folder-workspace, cross-platform, wire or provider behavior changes.
Checklist
N/Awith reasonHosted measurement
Selected-module worker time was effectively neutral: 16.493 → 16.595s (+0.6%) between clean parent run 38834270914 and candidate 38835131363. All five artifacts per run passed with zero errors, all three selected modules passed, and actual merge-checkout trees match their heads. Both use Node24.21.0/Bun1.4.3, five ARM shards/four workers; workflow and routing sources are unchanged. The repaired G4 case moves from the Node verification owner to the real Bun handler owner; 48 other identities remain unchanged. Historical unpaired worker cost and local elapsed medians (15.063 → 14.915s, preliminary dependency-sync pair excluded) show no stable speed improvement. This PR is retained for stronger authorization proof. Full exact-head aggregate validation remains required.
Current-head hosted validation
Exact head
d69eb72b3dd945643350a1457781b78aef3ed14epassed full PR workflow 38898304073, includingverify/ PR checks passed, and all 31 separate/current checks passed or were intentionally skipped. Independent review reconciled all five shard inventories against clean lease baseline 38896587663: identical 12,356 discovered/executed files, zero errors or omissions.Selected three-module worker time 8.375 → 8.467s, test execution 1.297 → 1.290s; no speed improvement claimed. Both hosted runs used Node24.19.0/Bun1.4.3. Relevant production owners, harness, dispatcher, workflow, runner and lockfile match; unrelated authorization fixture, SSH state effects and replay spec changes are disclosed. The candidate actual checkout matches the entire published head. This repair earns its place through real authorization regression detection.