Skip to content

chore: promote staging to staging-promote/edbf0eaa-24719068222 (2026-04-21 13:10 UTC) - #2786

Merged
henrypark133 merged 1 commit into
mainfrom
staging-promote/07972fc0-24724183222
Apr 29, 2026
Merged

henrypark133 merged 1 commit into
mainfrom
staging-promote/07972fc0-24724183222

Conversation

@ironclaw-ci

@ironclaw-ci ironclaw-ci Bot commented Apr 21, 2026 •

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: 7fb41555a9e55677d1aaea29ca567a5b369c2b05..07972fc0992d74fc0ec97911ededf0ee273097ae
Promotion branch: staging-promote/07972fc0-24724183222
Base: staging-promote/edbf0eaa-24719068222
Triggered by: Staging CI batch at 2026-04-21 13:10 UTC

Commits in this batch (62):

Current commits in this promotion (1)

Current base: staging-promote/edbf0eaa-24719068222
Current head: staging-promote/07972fc0-24724183222
Current range: origin/staging-promote/edbf0eaa-24719068222..origin/staging-promote/07972fc0-24724183222

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

* fix(auth): switch OAuth URL construction to url crate to prevent char loss (#2391)

Google OAuth was reportedly receiving `access_type=offlin` instead of
`access_type=offline` when users ran `ironclaw tool auth google-calendar`,
breaking the offline-token flow every Google WASM tool relies on
(Calendar, Gmail, Drive, Docs, Sheets, Slides).

The hand-rolled `format!` + `urlencoding::encode` loops in
`auth::oauth::build_oauth_url` and `tools::mcp::auth::build_authorization_url`
are replaced with `url::Url` + `query_pairs_mut()`, routing every query
parameter through a single well-tested `application/x-www-form-urlencoded`
serializer. The old concat path is kept as a defensive fallback for the
(never-observed-in-practice) case where the authorization URL itself fails
to parse.

Regression coverage added at the call-site level per
`.claude/rules/testing.md`:

* `test_build_oauth_url_preserves_access_type_offline_exactly` — parses
  the returned URL and asserts `access_type == "offline"` exactly (not
  via `.contains()`, which would have passed on `offlin`).
* `test_build_oauth_url_extra_params_preserve_all_chars_across_hash_orderings`
  — loops 16 iterations so random `HashMap` iteration order surfaces any
  bug sensitive to which param lands last.
* `test_google_calendar_capabilities_produce_correct_oauth_url` — loads
  the shipped `google-calendar-tool.capabilities.json` shape, parses it
  via `CapabilitiesFile::from_json`, and drives the same
  `build_oauth_url` call site that `cli::tool::auth_tool_oauth` uses.
* `test_build_authorization_url_extra_params_preserve_all_chars` —
  parallel regression for the MCP authorization-URL builder.

The two pre-existing helper tests were also tightened to round-trip
through `url::Url::parse` + `query_pairs()` rather than relying on
substring assertions, so a 1-char truncation can no longer pass as a
prefix match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(auth): address PR #2746 review feedback

- Reject malformed authorization URLs with a specific error instead of
  concat-normalizing them (gemini-code-assist review).
- Rebuild HashMap per iteration in order-probe tests so different
  iteration orders are actually exercised (Copilot review).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(auth): surface malformed OAuth descriptors at call sites (#2746)

Address review feedback from @serrrfirat on PR #2746: two call sites of
`build_pending_oauth_launch` were using `.ok()?` to silently drop
`OAuthUrlError::MalformedConfig`, which regressed the fail-closed posture
this PR introduced.

Replaces `.ok()?` in both:
- `AuthManager::start_skill_oauth_if_supported`
- `ExtensionManager::start_secret_oauth_flow`

with an explicit `match` that emits `tracing::error!` (carrying
credential/extension/secret/user context) before falling back to the
manual-token path. Operators now get a signal when an OAuth descriptor
is misconfigured, rather than seeing the browser auth flow silently
disappear.

Signatures stay `Option<...>` — the existing
`test_build_oauth_url_rejects_malformed_authorization_url` covers the
helper-level regression; this change is call-site observability.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: channel/cli TUI / CLI channel scope: tool/mcp MCP client scope: extensions Extension management size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 21, 2026
@claude

claude Bot commented Apr 21, 2026

Copy link
Copy Markdown

Code review

Found 3 issues:

  1. [HIGH:95] Inconsistent error conversion patterns across call sites — build_oauth_url() and build_authorization_url() errors are handled differently in each caller:

    • src/cli/tool.rs:938 uses .map_err(|e| anyhow::anyhow!(e.to_string()))?
    • src/bridge/auth_manager.rs:766-777 uses match with explicit tracing::error!() + fallback
    • src/extensions/manager.rs:3860 uses .map_err(|e| ExtensionError::Config(e.to_string()))?
    • src/extensions/manager.rs:4842 uses .map_err(|e| e.to_string())?
    • src/tools/mcp/auth.rs:841 propagates directly with ?

    This inconsistency makes the error surface fragmented. A single pattern should be adopted across all callers.

https://github.com/anthropics/ironclaw/blob/3921ad3285fd9e6b4b5f5f34c76b9a8c05e4c99e/src/auth/oauth.rs#L217-L225
https://github.com/anthropics/ironclaw/blob/3921ad3285fd9e6b4b5f5f34c76b9a8c05e4c99e/src/cli/tool.rs#L936-L939

  1. [HIGH:75] Undocumented implicit dependency on query_pairs_mut() drop order — Both src/auth/oauth.rs:222-238 and src/tools/mcp/auth.rs:928-945 use a bare block to scope the mutable reference, relying on the Drop impl to flush changes before url.into() is called:
    {
        let mut qp = url.query_pairs_mut();
        qp.append_pair(...);
        // Drop fires here, flushing changes to url
    }
    Ok(url.into())  // Safe only because Drop fired
    This pattern is fragile — a future refactor that adds code between the block and Ok(url.into()) could silently lose query pairs without any compiler error. Add a doc comment explaining why the bare block is necessary.

https://github.com/anthropics/ironclaw/blob/3921ad3285fd9e6b4b5f5f34c76b9a8c05e4c99e/src/auth/oauth.rs#L217-L225

  1. [MEDIUM:75] Silent degradation hides misconfigured OAuth descriptors — src/bridge/auth_manager.rs:766-777 and src/extensions/manager.rs:4240-4252 catch OAuthUrlError and fall back to manual token entry with only a log message. Per CLAUDE.md's error-handling principle ("fail loud by default"), a malformed OAuth descriptor should either propagate the error or return an explicit error type instead of silently None, so the caller knows setup failed rather than the user being offered an alternative path.

https://github.com/anthropics/ironclaw/blob/3921ad3285fd9e6b4b5f5f34c76b9a8c05e4c99e/src/bridge/auth_manager.rs#L766-L777
https://github.com/anthropics/ironclaw/blob/3921ad3285fd9e6b4b5f5f34c76b9a8c05e4c99e/src/extensions/manager.rs#L4240-L4252

Base automatically changed from staging-promote/edbf0eaa-24719068222 to main April 29, 2026 04:09
@henrypark133
henrypark133 merged commit 07972fc into main Apr 29, 2026
65 of 68 checks passed
@henrypark133
henrypark133 deleted the staging-promote/07972fc0-24724183222 branch April 29, 2026 04:09

This branch had an error being deployed

1 failed and 5 inactive deployments
Ironclaw-QA / production — 07972fc0 Deployed Apr 21, 2026 by railway-app[bot]
cosmose-ironclaw / production — 07972fc0 Deployed Apr 21, 2026 by railway-app[bot]
Near Foundation Ironclaw / production — 07972fc0 Deployed Apr 21, 2026 by railway-app[bot]
ironclaw-nearai / production — 07972fc0 Deployed Apr 21, 2026 by railway-app[bot]
venice-ironclaw / production — 07972fc0 Deployed Apr 21, 2026 by railway-app[bot]
humble-cat / staging-cameron — 07972fc0 Deployed Apr 21, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/cli TUI / CLI channel scope: extensions Extension management scope: tool/mcp MCP client size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants