Skip to content

fix(reborn): clean Web Access setup copy - #4948

Merged
think-in-universe merged 2 commits into
mainfrom
codex/remove-web-access-install-copy
Jun 16, 2026
Merged

think-in-universe merged 2 commits into
mainfrom
codex/remove-web-access-install-copy

Conversation

@hanakannzashi

Copy link
Copy Markdown
Contributor

Summary

  • Remove the install-first wording from the Web Access onboarding next-step copy.
  • Add contract coverage so Web Access configure text stays activation-focused after install.

Validation

  • cargo fmt --all -- --check
  • cargo test -p ironclaw_reborn_composition bundled_extension_summaries_include_onboarding_messages --locked
  • git diff --check

Security Impact

None.

Database Impact

None.

Blast Radius

Limited to bundled Web Access onboarding copy in the Reborn extension catalog.

Rollback Plan

Revert this PR to restore the previous Web Access onboarding copy.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f31e46d5-fba8-40da-8c82-16a837d9e100

📥 Commits

Reviewing files that changed from the base of the PR and between 96c0af9 and 14fdc7d.

📒 Files selected for processing (1)
  • crates/ironclaw_reborn_composition/src/available_extensions.rs

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Updated onboarding instructions for the web-access extension to clarify the activation and publishing workflow.

Walkthrough

The credential_next_step string for the web-access extension in available_extensions.rs is changed from install-then-activate phrasing to "Activate Web Access to publish its tools." The unit test asserting that exact string is updated to match.

Changes

web-access Onboarding Copy

Layer / File(s) Summary
credential_next_step string and test assertion
crates/ironclaw_reborn_composition/src/available_extensions.rs
credential_next_step literal at line 154 changed to "Activate Web Access to publish its tools."; test case at lines 1806–1811 updated to assert the new string.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • italic-jinxin
  • think-in-universe

Poem

A single string, reworded clean,
"Activate" where "install" had been.
The test keeps pace, no lie remains,
One line of truth, no wasted pains.
🦀 Cargo compiles, all is serene.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits style with type(fix) and scope(reborn), accurately summarizing the main change to Web Access onboarding copy.
Description check ✅ Passed Description covers Summary, Validation, Security Impact, Database Impact, Blast Radius, and Rollback Plan; missing Change Type checklist and Reborn Trust-Boundary Checklist, but core substance is complete.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added size: XS < 10 changed lines (excluding docs) risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Jun 16, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the onboarding next-step text for the 'web-access' extension in available_extensions.rs to remove the redundant installation instruction, simplifying it to only mention activation. A corresponding unit test was also added to verify this updated text. There are no review comments, and the changes look correct, so I have no feedback to provide.

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.

@think-in-universe
think-in-universe added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit 1e50ddf Jun 16, 2026
69 checks passed
@think-in-universe
think-in-universe deleted the codex/remove-web-access-install-copy branch June 16, 2026 06:55
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Co-authored-by: Robert Yan <mstr.raphael@gmail.com>
henrypark133 added a commit that referenced this pull request Jun 25, 2026
…ivery (#5222)

Triggered-run Slack delivery recorded `Failed` for runs that park in
`BlockedApproval` / `BlockedAuth` awaiting the user's approval or re-auth and
never resolve. After delivering the actionable gate/auth prompt, the delivery
loop re-enters `wait_for_actionable_triggered` to handle the eventual
transition to `Completed` (deliver the final reply, delete the stale OAuth
prompt). When the user never acts — the common case — that re-wait polled to
the 30-minute backstop and the wait-error arm recorded a generic `Failed`,
clobbering the `Delivered` it had already earned (the outcome store overwrites
blindly via `CasExpectation::Any`).

Production evidence (logs.1782348290172): 23× "did not finish before Slack
delivery timeout" in ~1.5h, all `outcome=Failed`; 21× `BlockedApproval`, 3×
`BlockedAuth` (from `secret expired` → `AuthRequired` on
google-calendar.list_events). Two runs traced exactly: `BlockedApproval` at
23:15:39 → timeout at 23:45:45 (30m06s); `BlockedAuth` at 23:30:24 → timeout
at 00:00:29 (30m05s).

Fix: in `deliver_triggered_run`, when the wait times out AND a blocked prompt
was already delivered (`delivered_blocked_marker.is_some()`), the run is parked
awaiting the user — a successful, terminal-for-delivery outcome. Record
`Delivered` and return instead of polling on and recording `Failed`. This
mirrors the live-run path's existing `RunWaitTimedOutAfterNotification` quiet-
success semantics. The 30-minute backstop is preserved as the failure signal
only for runs that never reach an actionable state (still running / stuck).

Not a regression: the blocked-delivery machinery (wait_for_actionable_triggered,
the 30-min backstop) shipped together in #4948; this is a latent design defect
that never handled the never-resolved case. Reuses the existing `Delivered`
outcome (documented as "final reply or gate prompt was delivered") — no new
outcome variant, no new config, no new state machine.

Regression tests (gated --features slack-v2-host-beta), all proven red on the
unpatched arm and green after the fix:
- triggered_persistent_blocked_approval_records_delivered_not_failed
- triggered_running_then_blocked_approval_records_delivered_not_failed
- triggered_persistent_blocked_oauth_auth_records_delivered_not_failed
- triggered_never_actionable_run_times_out_failed (backstop preserved: zero
  posts, Failed)

Adds a `ScriptedTurnCoordinator::with_states_clamped` helper (sticky final
state) and folds the one-off test coordinator into it. Invariant captured as a
doc comment on `deliver_triggered_run`.

Follow-up (separate track): converge the local parked-state check onto the
canonical `TurnStatus::wait_class()` classifier once it lands.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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: low Changes to docs, tests, or low-risk modules size: XS < 10 changed lines (excluding docs)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants