fix(google-wasm): auth required errors - #4969
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughFour Google WASM extension crates ( ChangesStructured 401 auth-required errors across Google WASM API modules
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
github-actions
Bot
added
size: M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces structured error handling for 401 Unauthorized responses across Google Docs, Drive, Sheets, and Slides WASM extensions, mapping them to a structured auth_required JSON error. It also adds a corresponding integration test in the host runtime to verify this behavior. Feedback suggests updating the upload_file function in the Google Drive extension to also use the new api_status_error helper, ensuring consistent authentication gate triggering during file uploads.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
serrrfirat
marked this pull request as ready for review
June 16, 2026 10:19
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/ironclaw_first_party_extensions/assets/google-drive/wasm-src/src/api.rs`:
- Line 40: In
crates/ironclaw_first_party_extensions/assets/google-drive/wasm-src/src/api.rs,
the upload_file function at lines 60 and 66-76 currently builds plain-text
errors instead of routing through api_status_error like the code at line 40
does. Replace the error construction at those locations (siblings) with calls to
api_status_error to ensure 401 responses in multipart uploads are properly
classified as AuthRequired rather than generic failures. Additionally, add a
regression test (using #[test] or #[tokio::test]) that verifies
google-drive.upload_file correctly handles a 401 response through the
host-runtime caller, confirming the auth_required path is taken for
authentication failures on multipart uploads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 82cb32b8-29f4-4222-9716-7d613b17e16b
⛔ Files ignored due to path filters (4)
crates/ironclaw_first_party_extensions/assets/google-docs/wasm/google_docs_tool.wasmis excluded by!**/*.wasm,!**/*.wasmcrates/ironclaw_first_party_extensions/assets/google-drive/wasm/google_drive_tool.wasmis excluded by!**/*.wasm,!**/*.wasmcrates/ironclaw_first_party_extensions/assets/google-sheets/wasm/google_sheets_tool.wasmis excluded by!**/*.wasm,!**/*.wasmcrates/ironclaw_first_party_extensions/assets/google-slides/wasm/google_slides_tool.wasmis excluded by!**/*.wasm,!**/*.wasm
📒 Files selected for processing (5)
crates/ironclaw_first_party_extensions/assets/google-docs/wasm-src/src/api.rscrates/ironclaw_first_party_extensions/assets/google-drive/wasm-src/src/api.rscrates/ironclaw_first_party_extensions/assets/google-sheets/wasm-src/src/api.rscrates/ironclaw_first_party_extensions/assets/google-slides/wasm-src/src/api.rscrates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs
serrrfirat
changed the title
Collaborator
Author
|
Addressed the review comments in Fixed Review Feedback
Validation
GitHub checks: pending on the new head. |
serrrfirat
enabled auto-merge
June 17, 2026 14:53
zetyquickly
previously approved these changes
Jun 23, 2026
zetyquickly
left a comment
zetyquickly
left a comment
Contributor
There was a problem hiding this comment.
Approving. Verified end-to-end on this branch — forced a 401 on a real google-drive.list_files call and confirmed the guest now emits auth_required → RuntimeCapabilityOutcome::AuthRequired → reauth gate, where main produced a retryable operation_failed. Audited all four tools: every non-2xx path routes through api_status_error (the upload_file gap is fixed).
This resolves the core of #4991. For the close note: token refresh is handled upstream by proactive refresh at staging (#5053 / #5087 / #4174), so the issue's "reactive refresh-retry parity" is a superseded non-goal, not missing — every credential branch (refreshable / revoked / no-refresh-token / missing) now lands correctly.
Non-blocking: the 401→auth_required regression covers Drive only; since the helper is copy-pasted per tool, one test against a non-Drive tool (e.g. google-docs.get_document) would lock the contract for all four.
LGTM 🚢
…uth-required' into codex/google-wasm-auth-required
zetyquickly
self-requested a review
June 23, 2026 20:05
zetyquickly
approved these changes
Jun 23, 2026
henrypark133
added a commit
that referenced
this pull request
Jun 25, 2026
…on/surface Addresses the CI failure and the remaining review feedback on the credential_requirements enrichment PR. - github_wasm_runtime_contract.rs: the two google-drive WASM 401 tests asserted credential_requirements.is_empty() — the pre-fix, un-wired contract from #4969 (provider-null, unsubmittable gate, #5174). The enrichment now populates the gate from the single credential obligation, so assert one requirement with provider=google + OAuth setup. This is the runtime-401 re-auth fallback; proactive refresh (inline + background keepalive) already runs before injection. - host.rs: document the reactive-refresh-on-runtime-401 follow-up on the enrichment helper, and correct the downstream-consumer note (OAuth setup launches the OAuth flow, ManualToken renders the token card). - credential.rs: add direct unit tests for binding_scope_owns_account covering the session_id and surface exact-match branches. The durable filesystem caller tests partition account records by surface+session path, so those axes only ever returned CredentialMissing and never executed the guard's equality branches (coderabbit review point). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
henrypark133
added a commit
that referenced
this pull request
Jun 25, 2026
* fix(reborn): populate provider on runtime auth-required gates A WASM/runtime capability whose injected credential returns 401 raises an `auth_required` gate with empty `credential_requirements` (`runtime_adapters.rs` `wasm_guest_dispatch_error`). That left `AuthPromptView.provider` null, so the WebUI manual-token card threw client-side (`useChat.submitAuthToken` requires `provider`) and never sent the submit — surfacing as "Could not save the token" with no network request. Enrich an empty `DispatchError::AuthRequired.credential_requirements` from the capability's already-declared credential obligations (`InjectCredentialAccountOnce` -> `RuntimeCredentialAuthRequirement`) in the capability host, where both the dispatch result and the obligations are in scope. Runtime-agnostic; never overrides a populated list. This reuses the same declared-requirement data the credential-missing path already surfaces, so re-auth gates become submittable. Adds a caller-level regression test driving `CapabilityHost::invoke_json` that asserts the gate carries the provider (fails before the fix), plus a non-override test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(host-api): move auth-requirement enrichment onto host_api types; fix take-one Address code-review findings on the runtime auth-required enrichment: - Correctness: emit at most one credential requirement. The downstream consumer (auth_prompt_from_credential_requirement) matches exactly one (`let [requirement] = ...`); emitting >1 for capabilities with multiple credential obligations made it fall through and leave the gate unsubmittable. Enrich with `.take(1)`. - Altitude/duplication: move the logic onto the types that own it in ironclaw_host_api — `Obligation::credential_auth_requirement()` and `DispatchError::enrich_auth_requirements(&[Obligation])`. Delete the free helper from the capability host (host.rs shrinks; the two call sites become one-liners and can't drift). - Tests: add resume-path coverage (auth_resume_json -> dispatch_resumed_ capability), a multi-obligation test locking the take-one contract, and host_api unit tests for both new methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn): owner-granularity scope check for manual-token/selection completion Folds the second link of runtime credential re-auth into this PR. Manual-token (and credential-selection) submit mints a fresh per-request `invocation_id`, so completing a flow that reconnects to a credential account created in an earlier flow failed `scope_matches` full-equality with CrossScopeDenied (HTTP 403) — the "Could not save the token" follow-on once the gate became submittable. This is #4935 defect A on the unbound/reusable path. - `complete_manual_token` and `complete_credential_selection` (product_auth_durable/flows.rs) now use `binding_scope_owns_account`: owner-granularity (tenant/user/agent/project hard-required, session + surface exact-matched) while ignoring the ephemeral invocation_id (and thread/mission, intentional for owner-reusable accounts). - Mirror the same fix in the in-memory fake (fakes.rs) so it cannot mask the divergence in unit tests. - Tests: cross-invocation reconnect succeeds (both paths); genuinely foreign owner still rejected; cross-session and cross-surface still rejected (path-partitioned on disk). Follow-ups (not in this PR): rename `binding_scope_owns_account` -> `scope_owns_account` (now used on unbound paths too); extract a shared completion-account validation helper to unify the three call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: move auth-requirement enrichment policy to capabilities; tighten condition Address PR #5180 review (thermo-nuclear + multi-agent): - Altitude: keep the neutral `Obligation::credential_auth_requirement` mapper in ironclaw_host_api, but move the enrichment POLICY out of `DispatchError::enrich_auth_requirements` (product-workflow cardinality has no place in the neutral vocab crate per its guardrail) into a private helper in ironclaw_capabilities. - Correctness: synthesize the auth-gate credential requirement ONLY when the runtime gave no auth signal of its own (both `required_secrets` and `credential_requirements` empty) AND the capability declares EXACTLY ONE credential obligation. Raw-secret gates (required_secrets populated) are no longer mis-prompted as product-auth; multi-credential capabilities no longer get a wrong-provider gate (was `.take(1)` guessing the first). - Tests: unit tests for all helper branches; updated the multi-obligation contract test to assert the gate is left unmodified (empty) rather than pointed at an arbitrary provider. Also adds durable rejection coverage for complete_credential_selection (foreign owner reaches binding_scope_owns_account -> CrossScopeDenied; session/ surface are path-partitioned -> CredentialMissing, guard exact-match is defense-in-depth). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): fix runtime-401 reauth-gate contract; cover guard session/surface Addresses the CI failure and the remaining review feedback on the credential_requirements enrichment PR. - github_wasm_runtime_contract.rs: the two google-drive WASM 401 tests asserted credential_requirements.is_empty() — the pre-fix, un-wired contract from #4969 (provider-null, unsubmittable gate, #5174). The enrichment now populates the gate from the single credential obligation, so assert one requirement with provider=google + OAuth setup. This is the runtime-401 re-auth fallback; proactive refresh (inline + background keepalive) already runs before injection. - host.rs: document the reactive-refresh-on-runtime-401 follow-up on the enrichment helper, and correct the downstream-consumer note (OAuth setup launches the OAuth flow, ManualToken renders the token card). - credential.rs: add direct unit tests for binding_scope_owns_account covering the session_id and surface exact-match branches. The durable filesystem caller tests partition account records by surface+session path, so those axes only ever returned CredentialMissing and never executed the guard's equality branches (coderabbit review point). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn): fix stale symbol/line refs in auth scope comments Address two low-severity review nits: - capability_host_auth_required_enrichment_contract.rs: header referenced the old helper name enrich_auth_required_from_obligations; rename to the real enrich_dispatch_error_credential_requirements. - fakes.rs / flows.rs: scope comments hard-coded credential.rs:580, which is already stale (binding_scope_owns_account is now at line 607). Drop the line number and point at the symbol only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(capabilities): cover raw-secret gate preservation; soften refresh doc Address two review nits on the enrichment helper: - host.rs: the OAuth runtime-401 follow-up doc stated cross-layer refresh (inline injection + background keepalive) as a guarantee, but those live in other crates and are not enforced here. Soften to "may already have been attempted". - capability_host_auth_required_enrichment_contract.rs: add invoke_json_preserves_required_secrets_from_dispatcher — a caller-level test driving CapabilityHost::invoke_json with a raw-secret AuthRequired (required_secrets populated, credential_requirements empty) while an InjectCredentialAccountOnce obligation is declared, asserting the gate is left unmodified (secrets preserved, not rewritten into a provider prompt). Previously covered only at the private-helper level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
auth_requiredguest errors for Google API 401 responses in bundled Drive, Docs, Sheets, and Slides WASM toolsRuntimeCapabilityOutcome::AuthRequiredRoot Cause
The bundled Google WASM tools returned raw provider error strings for non-2xx Google API responses. For 401 responses, that raw text was mapped by the host as
operation_failed, which made the model-visible recovery say same-call retry was allowed instead of triggering the auth gate.The host runtime already supports structured WASM guest errors with
kind = auth_required; the Google tools were just not emitting that structured shape.Validation
cargo fmt --check./scripts/build-wasm-extensions.sh --first-partycargo test -p ironclaw_host_runtime --test github_wasm_runtime_contract host_runtime_services_maps_google_drive_wasm_401_to_auth_requiredcargo test -p ironclaw_host_runtime --test github_wasm_runtime_contract host_runtime_services_routes_googlegit diff --checkNotes
I cleaned local generated build directories after the WASM rebuild to recover disk space, then reran the focused host-runtime tests successfully.