feat(web-push): browser push notifications + PWA — the web app as a first-party notification channel - #7398
Merged
Conversation
…irst-party notification channel The web app becomes a real, selectable notification route for automations, at parity with Slack/Telegram: a bundled first-party channel extension (web-push) delivers W3C Web Push (RFC 8030/8291/8292) to the user's enrolled browsers through the existing catalog → notification-channel set → notifier → delivery coordinator → channel adapter → policy-enforced egress chain, with zero new routing machinery and the two-lane delivery contract untouched. The WebUI ships as an installable PWA (root-scope service worker with push + notification-click deep links; manifest and icons already existed), and the automations page's notification-channels panel replaces the always-on "web app" placeholder with a real toggleable row plus a per-browser enroll/disable flow. New crates: ironclaw_web_push (domain: subscription records + CAS store, RFC 8291 aes128gcm encryption pinned to the RFC's Appendix A vector, VAPID key-material generation, transport-free request planning, channel identity grammar, late-bound runtime slot) and ironclaw_web_push_extension (channel package: manifest with vapid_authorization egress injection, adapter with 404/410 pruning and honest Sent-without-ref evidence, personal-DM codec, owner-scoped catalog provider). One generic host addition: the RuntimeCredentialTarget::VapidAuthorization egress injection kind — the host signs the RFC 8292 ES256 JWT at the existing credential chokepoint with the audience derived from the request's own push-service origin; adapters never see key bytes. VAPID material is auto-generated and seeded at composition boot. Enrollment is an authenticated product surface (three new /api/webchat/v2/web-push routes) with descriptors declared in ironclaw_product_contracts::web_push per the transport/product boundary, and endpoints validate against the manifest-declared push-service hosts. Also fixes a boot bug the new integration tests exposed: DeploymentChannelBinding rejected outbound-only channels, which would have failed the runtime build at serve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🚅 Deployed to the ironclaw-pr-7398 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 8, 2026 22:27
Destroyed
github-actions
Bot
added
scope: docs
Contributor
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Automatic trigger · attempt 1 of 3 · completed in 14m 13s IronLoop completed the review and posted it to GitHub. 🔗 Result |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds end-to-end Web Push support. It adds scoped subscription storage, RFC-compliant encryption and VAPID authorization, outbound delivery, authenticated WebUI APIs, browser enrollment, service-worker handling, localized controls, and integration coverage. ChangesWeb Push delivery
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Browser
participant WebUI
participant ProductService
participant SubscriptionStore
participant WebPushChannelAdapter
participant PushService
Browser->>WebUI: Request status or enroll subscription
WebUI->>ProductService: Dispatch typed Web Push operation
ProductService->>SubscriptionStore: Validate and store caller-scoped subscription
ProductService-->>Browser: Return enrollment status and VAPID public key
WebPushChannelAdapter->>SubscriptionStore: Load enrolled subscriptions
WebPushChannelAdapter->>PushService: Send encrypted VAPID-authorized notification
PushService-->>WebPushChannelAdapter: Return delivery response
WebPushChannelAdapter->>SubscriptionStore: Remove expired subscription after 410
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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 |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 IronLoop review
Found four correctness issues in the new web-push implementation.
Findings: 🔴 High 1 · 🟠 Medium 3
🔴 High · Create VAPID material atomically across replicas
Inline on crates/app/ironclaw_composition/src/factory/production_backend_assembly.rs:1495. See the inline comment for details.
🟠 Medium · Keep VAPID rotation out of generic admin configuration
Inline on crates/extensions/packages/web-push/manifest.toml:24. See the inline comment for details.
🟠 Medium · Enforce the push payload budget by bytes
Inline on crates/domains/ironclaw_web_push/src/message.rs:72. See the inline comment for details.
🟠 Medium · Bind browser enrollment state to the authenticated account
Inline on crates/product/ironclaw_webui/frontend/src/pages/automations/hooks/useWebPushDevice.ts:32. See the inline comment for details.
Validation
- ✅ Finding location verification — Verified every reported location with zero-context diffs for refs/ironloop/merge-base..refs/ironloop/head.
- ⚪ Focused runtime tests — Not run. No focused runtime test was needed; static tracing established the key lifecycle, payload sizing, and browser/account state defects.
Review details
- Run:
6aca6846-10ff-4264-8a2b-ba4fbe30036c - Workflow: Review
- Attempts: 1
| } | ||
| })?; | ||
| secret_store | ||
| .put( |
Contributor
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🔴 High · Create VAPID material atomically across replicas
The metadata check and unconditional `put` are not atomic, and the secret store permits replacement. Two replicas cold-starting against a shared empty store can each advertise a distinct generated public key, while the last write becomes the signing key. Clients enrolled through the other replica are then bound to the wrong applicationServerKey and their pushes are rejected. Use create-if-absent/CAS (or a deployment lock) and reread the canonical material on contention.
| fields = [ | ||
| # Seeded programmatically at boot when absent; `required = false` so the | ||
| # operator config surface never demands manual input. | ||
| { handle = "web_push_vapid", label = "VAPID key material (auto-generated)", secret = true, required = false }, |
Contributor
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🟠 Medium · Keep VAPID rotation out of generic admin configuration
This exposes the boot-generated VAPID blob as a mutable admin secret. Channel egress prefers an admin-config value, but the product service permanently reports the public key captured at boot. A valid admin replacement therefore changes the key used to sign deliveries while `/web-push/status` continues to give browsers the old applicationServerKey, making both existing and new enrollments fail. Keep this internal seed outside generic admin configuration, or implement a coherent rotation and re-enrollment flow.
| ) -> Self { | ||
| Self { | ||
| title: truncate_chars(&sanitize_text(title.into()), MAX_TITLE_CHARS), | ||
| body: truncate_chars(&sanitize_text(body.into()), MAX_BODY_CHARS), |
Contributor
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🟠 Medium · Enforce the push payload budget by bytes
The body is truncated by Unicode character count, then rejected if its serialized JSON exceeds 3,800 bytes. For example, 1,500 four-byte emoji pass the character cap but serialize to over 6 KB; request planning then fails permanently and no notification is sent. Truncate against the serialized UTF-8/JSON byte budget and add a multibyte regression case.
| const [browser, setBrowser] = React.useState({ state: "checking" }); | ||
| React.useEffect(() => { | ||
| let cancelled = false; | ||
| getWebPushBrowserState().then( |
Contributor
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🟠 Medium · Bind browser enrollment state to the authenticated account
The hook treats any origin-level PushManager subscription as enrolled without reconciling it with the caller-scoped backend subscription set. If user A enrolls and user B later signs in in the same browser profile, B sees only Disable despite having no record; disabling first removes the browser-global subscription, breaking A's enrollment, while B's scoped backend removal does nothing. Correlate state with the current caller before offering Disable and never locally unsubscribe an unowned endpoint.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
🤖 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/app/ironclaw_cli/src/runtime/mod.rs`:
- Around line 683-690: Update web_push_vapid_subject_from_env to parse and
validate the environment value as an absolute HTTPS URL using the shared URL
type or reqwest::Url; compare the scheme with eq_ignore_ascii_case(), reject
non-HTTPS URLs and invalid host/path values, and preserve the normalized URL
only after successful validation. Handle std::env::VarError::NotUnicode
explicitly instead of treating all environment lookup failures as absent.
In `@crates/app/ironclaw_composition/src/factory/production_backend_assembly.rs`:
- Around line 1476-1481: Update the VAPID credential parsing in the production
backend assembly to retain the serde_json parse error instead of discarding it
via map_err(|_| ...). Attach the source error to the cause-bearing
RebornBuildError while preserving the existing sanitized caller-facing reason.
- Around line 1454-1504: Update the VAPID initialization flow around the
existing metadata check and `SecretStorePort::put` call to use an atomic
create-if-absent or conditional update operation. When initialization races,
treat the already-created credential as the winner, re-read and parse that
stored material, and derive `vapid_public_key` from it before advertising
`applicationServerKey`; avoid unconditional replacement of the secret.
In `@crates/contracts/ironclaw_host_api/src/http.rs`:
- Around line 180-189: Remove the derived Debug implementation from
VapidCredentialMaterialV1 and provide a custom formatter or secret-wrapper
representation that never prints es256_private_key_pkcs8_b64url. Preserve
diagnostic output for the non-sensitive public_key_b64url and subject fields
while redacting the private key.
- Around line 182-189: Improve VapidCredentialMaterialV1 with a fallible
validator or constructor that decodes both base64url fields, verifies the
private key is a valid P-256 PKCS#8 key, confirms the public key is a valid
65-byte uncompressed P-256 point, and validates subject as an RFC 8292 mailto:
or https: URI. Invoke this validation during assemble_web_push and again before
egress signing so invalid persisted material fails closed; add a regression test
proving malformed stored material prevents Web Push runtime assembly while
preserving existing configuration precedence and provider resolution.
In `@crates/domains/ironclaw_web_push/Cargo.toml`:
- Line 18: Update the Cargo.lock base64 dependency entries to resolve to one
coherent version across the dependency graph, while leaving the base64
declaration in the ironclaw_web_push manifest unchanged. Reconcile the existing
0.21.7, 0.22.1, and 0.23.0 lock entries using Cargo’s dependency resolution
rather than manually changing the manifest.
In `@crates/domains/ironclaw_web_push/README.md`:
- Around line 8-10: Update the README description of PushSubscriptionRecord,
PushEndpoint, and PushSubscriptionKeys to remove the nonexistent
SUPPORTED_PUSH_SERVICE_HOSTS reference and accurately state that push-service
hosts are deployment-supplied via
PushEndpoint::validate_against_push_services(&allowed_hosts), resolved from
web-push manifest channel.egress entries during assemble_web_push.
In `@crates/domains/ironclaw_web_push/src/error.rs`:
- Around line 41-45: Update WebPushError::store so it retains the source as an
internal typed cause while assigning a fixed sanitized reason/category; do not
derive the user-visible reason from source.to_string(). Ensure formatting
WebPushError::Store cannot expose backend storage diagnostics.
In `@crates/domains/ironclaw_web_push/src/message.rs`:
- Around line 62-76: Update WebPushNotificationPayload::new to sanitize tag and
normalize url through a new app-relative URL helper beside sanitize_text. The
URL helper must strip controls, accept only single-slash paths up to 512
characters, and replace invalid, scheme-bearing, host-relative, or overlong
values with "/"; apply the existing text sanitization to tag before storing it.
In `@crates/domains/ironclaw_web_push/src/store.rs`:
- Around line 311-396: Add a Tokio test named
concurrent_enrollments_do_not_lose_a_browser alongside the existing store tests.
Wrap the store in Arc, spawn concurrent tasks that each call the public
upsert_subscription operation with distinct endpoints, await all joins
successfully, then list the scope and assert all eight subscriptions remain,
validating CAS retry behavior without calling cas_update directly.
In `@crates/domains/ironclaw_web_push/src/subscription.rs`:
- Around line 153-179: Update PushSubscriptionKeys::new to validate the decoded
p256dh bytes as a valid on-curve P-256 public key, not just length and the 0x04
prefix, before returning the subscription keys. Move or expose reusable
validation from crypto::encrypt_payload/agreement handling so subscription
creation rejects invalid recipient material while preserving the existing
InvalidSubscription error behavior.
- Around line 198-226: Replace the raw String subscription_id in
PushSubscriptionRecord with an exported domain newtype, generate it in
PushSubscriptionRecord::new, and update serde usage and affected callers to
preserve the persisted identifier format. For created_at, retain String only if
the crate cannot add a time dependency, but validate the value as RFC 3339 in
the constructor rather than relying solely on documentation. Re-export the
identifier type from lib.rs beside the other subscription types.
In `@crates/domains/ironclaw_web_push/src/vapid.rs`:
- Around line 21-28: Replace the derived Debug implementation on
GeneratedVapidKeyMaterial with a manual redacting implementation that never
emits material_json or its embedded ES256 private key; keep only non-sensitive
metadata such as the public-key field or a redacted placeholder. Update
material_json to use secrecy::SecretString if compatible with the existing
SecretMaterial::from composition, and adjust consumers accordingly without
exposing the secret during formatting or error propagation.
- Around line 70-84: Update validate_vapid_subject to parse the subject with the
existing url dependency and require a valid mailto or https URI, rejecting
malformed values such as "mailto:@" while preserving length and
control-character checks. Extend the validation tests with "https://x",
"mailto:x", and "mailto:@" to cover these cases.
In `@crates/extensions/AGENTS.md`:
- Line 3: Update the extension-family counts in the AGENTS.md routing map from
14 to 15 package directories and from 4 to 5 crate-bearing/workspace package
crates, including the later “four crate-bearing packages” statement. Keep the
dependency and layer descriptions unchanged.
In `@crates/extensions/packages/web-push/src/channel.rs`:
- Around line 209-220: Update the expired-subscription pruning arm around
remove_subscription so the failure branch includes an inline silent-ok marker
naming the intentional fallback operation, and move tally.pruned += 1 into the
successful-removal path. Failed store removals must only log and must not
increment the prune tally.
- Around line 202-256: The 410 pruning test must verify that a subsequent send
does not attempt the removed subscription. Update
gone_push_subscription_is_pruned_after_notice_attempt() to perform a second send
and assert the production adapter reaches its exhausted or no-next-send state,
while retaining coverage that the 404 path does not prune subscriptions.
In `@crates/extensions/packages/web-push/src/targets.rs`:
- Around line 86-87: Update WEB_PUSH_TARGET_PROVIDER_KEY to derive its value
from WEB_PUSH_EXTENSION_ID instead of duplicating the "web-push" literal,
ensuring registry composition stays synchronized with the manifest extension ID.
In `@crates/extensions/packages/web-push/tests/manifest_lockstep.rs`:
- Around line 21-58: Add the shared
ironclaw_web_push::SUPPORTED_PUSH_SERVICE_HOSTS allowlist and extend the
manifest lockstep test to assert each egress host belongs to it, or revise the
module guidance comments so they no longer describe a nonexistent host pin. Keep
the existing HTTPS, credential, injection, and path-prefix assertions unchanged.
In `@crates/kernel/ironclaw_host_runtime/src/egress/vapid.rs`:
- Around line 170-193: Extend
malformed_material_and_non_https_audiences_fail_closed with a valid HTTPS
audience using a non-default explicit port, and assert the generated
authorization payload’s aud field is serialized as https://host:port. If the
implementation includes a k/t consistency check, also add a case with mismatched
key and token material that asserts the expected validation error.
- Around line 82-96: Validate in the VAPID signing flow that
material.public_key_b64url matches key_pair.public_key().as_ref() after
constructing key_pair and before signing or formatting the header. Return
VapidHeaderError::MaterialInvalid when the advertised and derived public keys
differ, while preserving the existing signing path for matching credentials.
In `@crates/product/ironclaw_webui/frontend/public/sw.js`:
- Around line 52-78: Update the notificationclick handler’s URL resolution to
parse and validate the payload URL against self.location.origin before any
navigation. Use the URL’s same-origin path/target when valid, and fall back to
"/" for invalid, malformed, or cross-origin values; keep the existing client
navigation and openWindow flow using this sanitized target.
In `@crates/product/ironclaw_webui/frontend/src/lib/web-push.ts`:
- Around line 110-122: Update the enrollment flow around subscribeWebPush to
roll back any local push subscription when backend registration fails: catch the
rejection, call subscription.unsubscribe(), then rethrow the original error.
Preserve the existing successful registration and enrolled-state behavior.
- Around line 38-45: Update pushRegistration and the surrounding web-push
initialization to reuse the boot-time service-worker registration result instead
of awaiting navigator.serviceWorker.ready indefinitely. Bound any fallback wait
and resolve registration failure as unsupported, ensuring getWebPushBrowserState
and enrollThisBrowser settle when registerServiceWorker returns null or rejects.
Add a test verifying getWebPushBrowserState resolves rather than hangs when
registration fails.
In
`@crates/product/ironclaw_webui/frontend/src/pages/automations/components/notification-channels-panel.test.ts`:
- Around line 748-790: Add a test case in the WebPushDeviceBlock tests for a
not-enrolled device with an empty vapidPublicKey, verify the enroll
button/action is unavailable or disabled, and confirm the device’s enroll hook
is not called. Reuse the existing harness, fakeDevice, and button inspection
patterns.
In
`@crates/product/ironclaw_webui/frontend/src/pages/automations/components/notification-channels-panel.tsx`:
- Around line 33-61: Update WebPushDeviceBlock to handle device.statusError
before rendering subscriptionCount or enrollment controls: show the
webPush.statusFailed translation and avoid presenting fallback backend values or
actionable buttons when the status request fails. Add the
automations.notificationChannels.webPush.statusFailed key consistently to all
twelve locale files, reusing the surrounding panel’s error-rendering pattern.
In `@docs/internal/design/2026-08-08-web-push-notifications.md`:
- Around line 26-30: Update the delivery terminology in the design document to
match the three defined flows: replace “two-lane” with “delivery paths” in this
section and later references, while preserving the existing lane 1, lane 2, and
lane 3 descriptions.
In `@docs/reborn/contracts/triggers.md`:
- Around line 530-535: Update the surrounding WebApp selection contract in the
triggers documentation to remove or explicitly mark as historical any statement
claiming selection persists without external egress. Retain only the current
behavior described by the web-push external destination and notification-channel
delivery flow.
In `@scripts/ci/composition-budget.toml`:
- Line 174: Update the loc_observed_date field associated with loc_observed =
41035 to "2026-08-08", keeping the recorded count unchanged.
In `@tests/integration/delivery_user_journeys.rs`:
- Around line 1975-2001: Consolidate background_run_notifier and
web_push_background_run_notifier into one codec-parameterized helper, reusing a
single RunDeliverySettings construction and accepting the codec list as a
parameter. Update both call sites to supply their required preference target
codecs, preserving the existing Slack-only and Slack-plus-WebPush behavior
without duplicating notifier configuration.
In `@tests/integration/support/group_constructors.rs`:
- Around line 598-613: Extract the repeated channel-connection construction into
a private channel_connection_for_base helper accepting the host runtime,
GroupBase, and constructor label. Move the RebornServices validation,
scope-derived ChannelConnectionTestConfig creation, and
build_channel_connection_for_test call into it, preserving the shared
missing-agent error and formatting the constructor-specific missing-bundle
error. Replace all five duplicated blocks, including
extension_delivery_with_web_push, with this helper and retain each assignment to
self.channel_connection.
🪄 Autofix
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: 33791d70-4614-4776-b08d-dca2785ce69b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (103)
Cargo.tomlFEATURE_PARITY.mdcrates/AGENTS.mdcrates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_architecture_tests/tests/reborn_extension_specificity.rscrates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rscrates/app/ironclaw_architecture_tests/tests/reborn_same_layer_edge_inventory.rscrates/app/ironclaw_cli/AGENTS.mdcrates/app/ironclaw_cli/Cargo.tomlcrates/app/ironclaw_cli/src/first_party/bundles.rscrates/app/ironclaw_cli/src/runtime/mod.rscrates/app/ironclaw_cli/src/runtime/native_extensions.rscrates/app/ironclaw_composition/Cargo.tomlcrates/app/ironclaw_composition/src/factory.rscrates/app/ironclaw_composition/src/factory/production_backend_assembly.rscrates/app/ironclaw_composition/src/factory/production_build_assembly.rscrates/app/ironclaw_composition/src/input.rscrates/app/ironclaw_composition/src/lib.rscrates/app/ironclaw_composition/src/product_surface.rscrates/app/ironclaw_composition/src/runtime.rscrates/app/ironclaw_composition/src/runtime/tests/core.rscrates/app/ironclaw_composition/tests/trigger_poller_e2e.rscrates/contracts/ironclaw_extension_contracts/src/channel.rscrates/contracts/ironclaw_host_api/src/http.rscrates/contracts/ironclaw_product_contracts/AGENTS.mdcrates/contracts/ironclaw_product_contracts/src/lib.rscrates/contracts/ironclaw_product_contracts/src/product_wire.rscrates/contracts/ironclaw_product_contracts/src/web_push.rscrates/domains/AGENTS.mdcrates/domains/ironclaw_web_push/Cargo.tomlcrates/domains/ironclaw_web_push/README.mdcrates/domains/ironclaw_web_push/src/crypto.rscrates/domains/ironclaw_web_push/src/error.rscrates/domains/ironclaw_web_push/src/grammar.rscrates/domains/ironclaw_web_push/src/lib.rscrates/domains/ironclaw_web_push/src/message.rscrates/domains/ironclaw_web_push/src/runtime.rscrates/domains/ironclaw_web_push/src/store.rscrates/domains/ironclaw_web_push/src/subscription.rscrates/domains/ironclaw_web_push/src/vapid.rscrates/extensions/AGENTS.mdcrates/extensions/ironclaw_extension_host/src/deployment_channels.rscrates/extensions/ironclaw_extension_host/src/test_support.rscrates/extensions/packages/web-push/Cargo.tomlcrates/extensions/packages/web-push/README.mdcrates/extensions/packages/web-push/manifest.tomlcrates/extensions/packages/web-push/src/channel.rscrates/extensions/packages/web-push/src/lib.rscrates/extensions/packages/web-push/src/preference_targets.rscrates/extensions/packages/web-push/src/targets.rscrates/extensions/packages/web-push/tests/manifest_lockstep.rscrates/kernel/ironclaw_host_runtime/Cargo.tomlcrates/kernel/ironclaw_host_runtime/src/egress/credential.rscrates/kernel/ironclaw_host_runtime/src/egress/mod.rscrates/kernel/ironclaw_host_runtime/src/egress/vapid.rscrates/kernel/ironclaw_host_runtime/src/first_party_tools/schemas.rscrates/kernel/ironclaw_host_runtime/src/first_party_tools/trigger_management.rscrates/lanes/ironclaw_sandbox/src/plan.rscrates/product/ironclaw_assistant/AGENTS.mdcrates/product/ironclaw_assistant/Cargo.tomlcrates/product/ironclaw_assistant/src/lib.rscrates/product/ironclaw_assistant/src/reborn_services.rscrates/product/ironclaw_assistant/src/reborn_services/product_capability_handlers.rscrates/product/ironclaw_assistant/src/reborn_services/web_push.rscrates/product/ironclaw_webui/CONTRACT.mdcrates/product/ironclaw_webui/frontend/public/sw.jscrates/product/ironclaw_webui/frontend/src/i18n/ar.tscrates/product/ironclaw_webui/frontend/src/i18n/de.tscrates/product/ironclaw_webui/frontend/src/i18n/en.tscrates/product/ironclaw_webui/frontend/src/i18n/es.tscrates/product/ironclaw_webui/frontend/src/i18n/fr.tscrates/product/ironclaw_webui/frontend/src/i18n/hi.tscrates/product/ironclaw_webui/frontend/src/i18n/ja.tscrates/product/ironclaw_webui/frontend/src/i18n/ko.tscrates/product/ironclaw_webui/frontend/src/i18n/pt-BR.tscrates/product/ironclaw_webui/frontend/src/i18n/uk.tscrates/product/ironclaw_webui/frontend/src/i18n/zh-CN.tscrates/product/ironclaw_webui/frontend/src/lib/api.test.tscrates/product/ironclaw_webui/frontend/src/lib/api.tscrates/product/ironclaw_webui/frontend/src/lib/web-push.test.tscrates/product/ironclaw_webui/frontend/src/lib/web-push.tscrates/product/ironclaw_webui/frontend/src/main.tsxcrates/product/ironclaw_webui/frontend/src/pages/automations/components/notification-channels-panel.test.tscrates/product/ironclaw_webui/frontend/src/pages/automations/components/notification-channels-panel.tsxcrates/product/ironclaw_webui/frontend/src/pages/automations/hooks/useWebPushDevice.tscrates/product/ironclaw_webui/src/webui_v2/descriptors.rscrates/product/ironclaw_webui/src/webui_v2/handlers.rscrates/product/ironclaw_webui/src/webui_v2/mod.rscrates/product/ironclaw_webui/src/webui_v2/router.rscrates/product/ironclaw_webui/tests/webui_v2_descriptors_contract.rsdocs/internal/design/2026-08-08-web-push-notifications.mddocs/reborn/contracts/communication-delivery-resolution.mddocs/reborn/contracts/triggers.mddocs/reborn/extension-runtime/overview.mddocs/reborn/target-architecture/PROPOSAL.mdscripts/ci/composition-budget.tomltests/CLAUDE.mdtests/integration/delivery_user_journeys.rstests/integration/support/group_constructors.rstests/integration/support/harness/mod.rstests/integration/support/harness/options.rstests/integration/support/harness/profiles/extension.rstests/integration/webui_v2_product_api.rs
Comment on lines
+683
to
+690
| fn web_push_vapid_subject_from_env() -> Option<String> { | ||
| let raw = std::env::var("IRONCLAW_REBORN_WEBUI_BASE_URL").ok()?; // silent-ok: optional env-derived contact URI, placeholder fallback is safe | ||
| let trimmed = raw.trim().trim_end_matches('/'); | ||
| if trimmed.starts_with("https://") && trimmed.len() > "https://".len() { | ||
| return Some(trimmed.to_string()); | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 \
'IRONCLAW_REBORN_WEBUI_BASE_URL|web_push_vapid_subject_from_env|generate_vapid_key_material' \
crates/app/ironclaw_cli/src \
crates/app/ironclaw_composition/srcRepository: nearai/ironclaw
Length of output: 10582
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- serve_sso relevant URL parsing/validation ---\n'
sed -n '32,120p' crates/app/ironclaw_cli/src/commands/serve_sso.rs
printf '\n--- serve URL validation call sites ---\n'
sed -n '750,810p' crates/app/ironclaw_cli/src/commands/serve.rs
printf '\n--- exact URL parsing implementations references ---\n'
rg -n 'url::Url::parse|Url::parse|validate_webui_public_base_url|webui_public_base_url_from_env' crates/app/ironclaw_cli/src
printf '\n--- test inputs relevant to malformed/upper case ---\n'
rg -n -C 3 'HTTPS://|bad host|invalid| malformed|webui_public_base_url_from_env|WEBUI_BASE_URL_ENV' crates/app/ironclaw_cli/src/commands/serve.rs crates/app/ironclaw_cli/src/commands/serve_sso.rsRepository: nearai/ironclaw
Length of output: 244
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- serve_sso relevant URL parsing/validation ---'
sed -n '32,120p' crates/app/ironclaw_cli/src/commands/serve_sso.rs
printf '%s\n' ''
printf '%s\n' '--- serve URL validation call sites ---'
sed -n '750,810p' crates/app/ironclaw_cli/src/commands/serve.rs
printf '%s\n' ''
printf '%s\n' '--- exact URL parsing implementations references ---'
rg -n 'url::Url::parse|Url::parse|validate_webui_public_base_url|webui_public_base_url_from_env' crates/app/ironclaw_cli/src
printf '%s\n' ''
printf '%s\n' '--- test inputs relevant to malformed/upper case ---'
rg -n -C 3 'HTTPS://|bad host|invalid| malformed|webui_public_base_url_from_env|WEBUI_BASE_URL_ENV' crates/app/ironclaw_cli/src/commands/serve.rs crates/app/ironclaw_cli/src/commands/serve_sso.rs
printf '%s\n' ''
python3 - <<'PY'
from urllib.parse import urlparse
inputs = [
"https://bad host",
"HTTPS://example.com",
"HttpS://example.com",
"https://",
"https://example.com/",
]
for raw in inputs:
trimmed = raw.strip().rstrip("/") or raw
accepts = trimmed.startswith("https://") and len(trimmed) > len("https://")
try:
parsed = urlparse(trimmed)
valid = all([parsed.scheme, parsed.netloc, parsed.scheme in {"https", "http"}])
except Exception:
parsed = None
valid = False
print(f"{raw!r}")
print(f" accepts={accepts}")
print(f" urlparse={parsed!r} valid={valid}")
PYRepository: nearai/ironclaw
Length of output: 38189
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- serve_sso tests that parse URLs ---'
sed -n '420,455p' crates/app/ironclaw_cli/src/commands/serve_sso.rs
printf '%s\n' '--- surrounding URL validation tests with malformed inputs ---'
sed -n '1750,1848p' crates/app/ironclaw_cli/src/commands/serve.rsRepository: nearai/ironclaw
Length of output: 5182
Validate the VAPID subject as a canonical HTTPS URL.
starts_with("https://") accepts values with invalid hosts/path characters and rejects valid uppercase schemes like HTTPS://example.com, which violates the “new ingress must validate bounded payloads before persistence/external behavior” invariant. Parse with the shared URL type or reqwest::Url::parse, normalize the scheme with eq_ignore_ascii_case(), reject non-HTTPS/non-absolute values explicitly, and treat VarError::NotUnicode as an explicit error rather than falling back.
🤖 Prompt for 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.
In `@crates/app/ironclaw_cli/src/runtime/mod.rs` around lines 683 - 690, Update
web_push_vapid_subject_from_env to parse and validate the environment value as
an absolute HTTPS URL using the shared URL type or reqwest::Url; compare the
scheme with eq_ignore_ascii_case(), reject non-HTTPS URLs and invalid host/path
values, and preserve the normalized URL only after successful validation. Handle
std::env::VarError::NotUnicode explicitly instead of treating all environment
lookup failures as absent.
Source: Path instructions
Comment on lines
+182
to
+189
| pub struct VapidCredentialMaterialV1 { | ||
| /// PKCS#8 P-256 private key, base64url (unpadded). | ||
| pub es256_private_key_pkcs8_b64url: String, | ||
| /// Uncompressed P-256 public key (65 bytes), base64url (unpadded) — the | ||
| /// browser-facing `applicationServerKey` and the `k=` parameter. | ||
| pub public_key_b64url: String, | ||
| /// RFC 8292 `sub` claim: a `mailto:` or `https:` operator contact URI. | ||
| pub subject: String, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate persisted VAPID material before use.
deny_unknown_fields only rejects extra JSON fields. It accepts malformed base64url, a non-P-256 PKCS#8 key, an invalid public-point length, and an invalid RFC 8292 subject.
assemble_web_push deserializes this type and immediately exposes public_key_b64url. A corrupted persisted credential can therefore let startup succeed while browser enrollment and delivery fail later.
Add a fallible material validator or constructor. Validate it during composition and before egress signing. Add a regression test that malformed stored material prevents Web Push runtime assembly.
As per coding guidelines, “Keep bootstrap configuration, persisted settings, and encrypted secrets as separate layers; preserve precedence, mediated provider resolution, and fail-closed startup.”
🤖 Prompt for 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.
In `@crates/contracts/ironclaw_host_api/src/http.rs` around lines 182 - 189,
Improve VapidCredentialMaterialV1 with a fallible validator or constructor that
decodes both base64url fields, verifies the private key is a valid P-256 PKCS#8
key, confirms the public key is a valid 65-byte uncompressed P-256 point, and
validates subject as an RFC 8292 mailto: or https: URI. Invoke this validation
during assemble_web_push and again before egress signing so invalid persisted
material fails closed; add a regression test proving malformed stored material
prevents Web Push runtime assembly while preserving existing configuration
precedence and provider resolution.
Source: Coding guidelines
Comment on lines
+26
to
+30
| - Two-lane delivery contract: lane 1 = final reply always lands in the run | ||
| thread; lane 2 = model-called `builtin.outbound_deliver` to one catalog | ||
| target; lane 3 = host-emitted background-run notices (gate/auth/failure — | ||
| **not** Completed) fanned over the notification-channel set by | ||
| `TriggeredRunDeliveryDriver` → `DeliveryCoordinator` → `ChannelAdapter.deliver`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the delivery-lane terminology.
This section calls the model “two-lane” but defines lane 1, lane 2, and lane 3. Later sections also use “two-lane.” Rename this as delivery paths, or place host-emitted background notices explicitly inside lane 2.
🤖 Prompt for 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.
In `@docs/internal/design/2026-08-08-web-push-notifications.md` around lines 26 -
30, Update the delivery terminology in the design document to match the three
defined flows: replace “two-lane” with “delivery paths” in this section and
later references, while preserving the existing lane 1, lane 2, and lane 3
descriptions.
Comment on lines
+530
to
+535
| does not choose, parse, or infer a destination. (✎ 2026-08-08: the retired | ||
| in-app "WebApp selection" sentence above describes the pre-#7157 stored-target | ||
| model; under the shipped two-lane model the `web-push` catalog target is a | ||
| real external destination — a routine that should notify the browser pins it | ||
| in the prompt's delivery step like any channel target, and the | ||
| notification-channel set fans gate/auth/failure notices to it when selected.) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Remove the stale current-sounding WebApp contract statement.
The preceding text still says that “WebApp selection” persists without external egress. The dated note explains the historical meaning, but the surrounding paragraph discusses the shipping contract. Replace the sentence with an explicitly historical statement or remove it. Keep only the current web-push external-target behavior.
Proposed clarification
-WebApp selection persists the result without external egress.
+Historically, the retired WebApp selection persisted the result without external egress.As per path instructions, docs/reborn/contracts/**/*.md is the source of truth; it must not retain ambiguous current behavior statements.
🤖 Prompt for 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.
In `@docs/reborn/contracts/triggers.md` around lines 530 - 535, Update the
surrounding WebApp selection contract in the triggers documentation to remove or
explicitly mark as historical any statement claiming selection persists without
external egress. Retain only the current behavior described by the web-push
external destination and notification-channel delivery flow.
Source: Path instructions
Comment on lines
+1975
to
+2001
| /// The web-push notifier: [`background_run_notifier`]'s services with the | ||
| /// browser channel's codec beside Slack's, so the creator's `web-push` | ||
| /// notification target decodes. | ||
| fn web_push_background_run_notifier( | ||
| harness: &RebornIntegrationHarness, | ||
| services: &RebornRuntime, | ||
| ) -> ironclaw_assistant::TriggeredRunDeliveryDriver { | ||
| ironclaw_assistant::TriggeredRunDeliveryDriver::with_settings( | ||
| slack_run_delivery_services(harness, services), | ||
| ironclaw_assistant::RunDeliverySettings { | ||
| poll_interval: Duration::from_millis(5), | ||
| max_wait: Duration::from_secs(10), | ||
| max_concurrent_deliveries: std::num::NonZeroUsize::new(4).expect("non-zero"), | ||
| max_pending_deliveries: std::num::NonZeroUsize::new(8).expect("non-zero"), | ||
| }, | ||
| services | ||
| .triggered_run_delivery_store_for_test() | ||
| .expect("composed runtime exposes the triggered-delivery outcome store"), | ||
| Arc::new(vec![ | ||
| Arc::new(ironclaw_slack_extension::SlackPreferenceTargetCodec) | ||
| as Arc<dyn PreferenceTargetCodec>, | ||
| Arc::new(ironclaw_web_push_extension::WebPushPreferenceTargetCodec) | ||
| as Arc<dyn PreferenceTargetCodec>, | ||
| ]) as Arc<dyn ActivePreferenceTargetCodecs>, | ||
| slack_agent(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Fold the two notifier constructors into one codec-parameterized helper.
web_push_background_run_notifier repeats background_run_notifier (lines 1339-1356) byte-for-byte except for one extra codec entry. The RunDeliverySettings values are duplicated. A change to poll_interval, max_wait, or either concurrency bound must now be applied twice, and a missed edit produces two silently different delivery drivers in the same bin.
Parameterize on the codec list instead.
♻️ Proposed refactor
+fn run_notifier_with_codecs(
+ harness: &RebornIntegrationHarness,
+ services: &RebornRuntime,
+ codecs: Vec<Arc<dyn PreferenceTargetCodec>>,
+) -> ironclaw_assistant::TriggeredRunDeliveryDriver {
+ ironclaw_assistant::TriggeredRunDeliveryDriver::with_settings(
+ slack_run_delivery_services(harness, services),
+ ironclaw_assistant::RunDeliverySettings {
+ poll_interval: Duration::from_millis(5),
+ max_wait: Duration::from_secs(10),
+ max_concurrent_deliveries: std::num::NonZeroUsize::new(4).expect("non-zero"),
+ max_pending_deliveries: std::num::NonZeroUsize::new(8).expect("non-zero"),
+ },
+ services
+ .triggered_run_delivery_store_for_test()
+ .expect("composed runtime exposes the triggered-delivery outcome store"),
+ Arc::new(codecs) as Arc<dyn ActivePreferenceTargetCodecs>,
+ slack_agent(),
+ )
+}
+
fn web_push_background_run_notifier(
harness: &RebornIntegrationHarness,
services: &RebornRuntime,
) -> ironclaw_assistant::TriggeredRunDeliveryDriver {
- ironclaw_assistant::TriggeredRunDeliveryDriver::with_settings(
- slack_run_delivery_services(harness, services),
- ironclaw_assistant::RunDeliverySettings {
- poll_interval: Duration::from_millis(5),
- max_wait: Duration::from_secs(10),
- max_concurrent_deliveries: std::num::NonZeroUsize::new(4).expect("non-zero"),
- max_pending_deliveries: std::num::NonZeroUsize::new(8).expect("non-zero"),
- },
- services
- .triggered_run_delivery_store_for_test()
- .expect("composed runtime exposes the triggered-delivery outcome store"),
- Arc::new(vec![
+ run_notifier_with_codecs(
+ harness,
+ services,
+ vec![
Arc::new(ironclaw_slack_extension::SlackPreferenceTargetCodec)
as Arc<dyn PreferenceTargetCodec>,
Arc::new(ironclaw_web_push_extension::WebPushPreferenceTargetCodec)
as Arc<dyn PreferenceTargetCodec>,
- ]) as Arc<dyn ActivePreferenceTargetCodecs>,
- slack_agent(),
+ ],
)
}🤖 Prompt for 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.
In `@tests/integration/delivery_user_journeys.rs` around lines 1975 - 2001,
Consolidate background_run_notifier and web_push_background_run_notifier into
one codec-parameterized helper, reusing a single RunDeliverySettings
construction and accepting the codec list as a parameter. Update both call sites
to supply their required preference target codecs, preserving the existing
Slack-only and Slack-plus-WebPush behavior without duplicating notifier
configuration.
Comment on lines
+598
to
+613
| let scope = &base.product_harness.scope; | ||
| let channel_connection = | ||
| ironclaw_composition::test_support::build_channel_connection_for_test( | ||
| host_runtime.reborn_services_for_test().ok_or( | ||
| "extension_delivery_with_web_push harness is missing its RebornServices bundle", | ||
| )?, | ||
| ironclaw_composition::test_support::ChannelConnectionTestConfig { | ||
| tenant_id: scope.tenant_id.as_str().to_string(), | ||
| agent_id: scope | ||
| .agent_id | ||
| .as_ref() | ||
| .map(|agent| agent.as_str().to_string()) | ||
| .ok_or("group product scope is missing an agent id")?, | ||
| }, | ||
| )?; | ||
| self.channel_connection = Some(Arc::new(channel_connection)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the channel-connection block; this is its fourth copy.
extension_lifecycle_with_profile (472-487), extension_runtime_acme (504-519), extension_delivery (534-549), extension_delivery_with_gated_write (568-582), and now this constructor build the identical ChannelConnectionTestConfig from base.product_harness.scope. Only the ok_or string differs. Each new delivery group copies the block again, and a change to the scope derivation must be applied five times.
Extract a private helper taking &base and the constructor label.
♻️ Proposed helper
fn channel_connection_for_base(
host_runtime: &HostRuntimeCapabilityHarness,
base: &GroupBase,
constructor: &str,
) -> HarnessResult<ironclaw_composition::ChannelConnectionService> {
let scope = &base.product_harness.scope;
ironclaw_composition::test_support::build_channel_connection_for_test(
host_runtime
.reborn_services_for_test()
.ok_or_else(|| format!("{constructor} harness is missing its RebornServices bundle"))?,
ironclaw_composition::test_support::ChannelConnectionTestConfig {
tenant_id: scope.tenant_id.as_str().to_string(),
agent_id: scope
.agent_id
.as_ref()
.map(|agent| agent.as_str().to_string())
.ok_or("group product scope is missing an agent id")?,
},
)
.map_err(Into::into)
}🤖 Prompt for 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.
In `@tests/integration/support/group_constructors.rs` around lines 598 - 613,
Extract the repeated channel-connection construction into a private
channel_connection_for_base helper accepting the host runtime, GroupBase, and
constructor label. Move the RebornServices validation, scope-derived
ChannelConnectionTestConfig creation, and build_channel_connection_for_test call
into it, preserving the shared missing-agent error and formatting the
constructor-specific missing-bundle error. Replace all five duplicated blocks,
including extension_delivery_with_web_push, with this helper and retain each
assignment to self.channel_connection.
…oad, account-scoped enrollment, sanitized store errors Triage of the CodeRabbit + IronLoop review on #7398: - Secret exposure (Critical): hand-written redacting `Debug` on `VapidCredentialMaterialV1` and `GeneratedVapidKeyMaterial` so the ES256 private key can never reach a log, panic, or `{:?}`. - VAPID material (High): composition now ensures-then-reads-back the canonical stored keypair so multi-replica cold-start converges on one signing key and the advertised applicationServerKey matches it; material shape is validated at boot and again before egress signing. - Payload budget (Medium): notification body is trimmed by serialized-JSON bytes, not character count, so multi-byte content can't blow the single-record push budget (+ regression test). - Error hygiene (Medium): `WebPushError::Store` is a fixed sanitized category; the backend cause is logged server-side, never rendered into the boundary error. - Account-scoped enrollment (Medium): status projects an `endpoint_digest` (SHA-256 hex) so a shared browser profile distinguishes "enrolled here" from "enrolled for another account" without the endpoint URL leaving the backend; the frontend correlates on it and only offers destructive disable when verified. - Deep-link/tag grammar enforced in the owning crate; SW validates same-origin before navigating; subject parsed via `url`; concurrent-writer CAS test; registry key derived from the extension-id constant; parse-cause preserved; doc/count corrections. The manifest keeps the `web_push_vapid` field (a channel egress credential handle must be declared in [admin_configuration]); it stays host-seeded and not operator-supplied, with the rotation caveat documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 8, 2026 23:28
Destroyed
Collaborator
Author
|
Pushed review fixes (ef5ee69). Triage of every CodeRabbit + IronLoop finding: Fixed
Manifest rotation concern (🟠) — a channel egress Consciously declined
Green after the fixes: clippy (both feature legs), the three web-push integration bins (enrollment round-trip, notice-lane push, 410-prune), architecture suite (ceilings re-captured), i18n consistency, and frontend vitest (1207). |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
crates/kernel/ironclaw_host_runtime/src/egress/vapid.rs (1)
87-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
kis still not bound to the key that signst.
validate_shape()at Line 51 checks base64url shape and the 65-byte0x04point. It does not provepublic_key_b64urlbelongs toes256_private_key_pkcs8_b64url. Line 100 echoes the advertised key; Lines 90-95 sign with the private key. A well-formed but unrelated point passes every check here.The consequence is not partial:
/web-push/statusadvertises this same field asapplicationServerKey, so browsers enroll against key B while every delivery is signed by key A and rejected by the push service. Nothing fails at boot.
EcdsaKeyPair::public_key().as_ref()yields the uncompressed point. Compare it to the decodedpublic_keyand returnMaterialInvalidon mismatch. Lines 70-75 then become redundant withvalidate_shapeand can go.This was raised on an earlier commit and the shape validator does not close it.
🔒 Proposed fix
let key_pair = EcdsaKeyPair::from_pkcs8(&ECDSA_P256_SHA256_FIXED_SIGNING, &private_key_der) .map_err(|_| VapidHeaderError::MaterialInvalid)?; + // The advertised `k` is what browsers enrolled against; if it is not the + // public half of the key signing `t`, every delivery is rejected and + // nothing fails at boot. + if key_pair.public_key().as_ref() != public_key.as_slice() { + return Err(VapidHeaderError::MaterialInvalid); + } let rng = aws_rand::SystemRandom::new();🤖 Prompt for 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. In `@crates/kernel/ironclaw_host_runtime/src/egress/vapid.rs` around lines 87 - 101, Bind the advertised VAPID key to the signing key in the signing flow: decode public_key_b64url, compare it with EcdsaKeyPair::public_key().as_ref(), and return VapidHeaderError::MaterialInvalid on mismatch. Remove the now-redundant standalone public-key shape validation around the key-pair setup, while preserving the existing signature generation and encoded key output.
🤖 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/contracts/ironclaw_host_api/src/http.rs`:
- Around line 249-280: Replace the custom decode_b64url_no_pad implementation,
including its local sextet decoder, with decoding through the shared
base64::URL_SAFE_NO_PAD engine. Preserve the helper’s Option<Vec<u8>> interface
by mapping decode failures to None, and ensure canonical trailing-bit validation
comes from the shared engine.
In `@crates/domains/ironclaw_web_push/src/message.rs`:
- Around line 90-106: Update fit_body_to_byte_budget to guarantee progress when
truncating a one-character body: avoid rebuilding the same ELLIPSIS value,
either clear the final character and retry or stop fitting so to_json_bytes()
returns PayloadTooLarge. Add a regression test covering an oversized non-body
field such as the unbounded URL and verify fitting terminates with the error
propagated.
In `@crates/extensions/packages/web-push/src/targets.rs`:
- Around line 86-88: Update the Web Push CLI binding construction in native
extension runtime setup to use ironclaw_web_push::WEB_PUSH_EXTENSION_ID instead
of the hard-coded "web-push" value. Keep the binding’s existing ExtensionId
conversion and ensure it matches the provider registry and assemble_web_push
lookup key.
In `@crates/product/ironclaw_webui/frontend/src/lib/web-push.ts`:
- Around line 38-54: Update pushRegistration to retain and await the in-flight
registerServiceWorker promise before probing getRegistration, including when
registration is deferred by main.tsx. After registration resolves, wait only a
bounded period for an active registration, then return the registration or null
without hanging. Ensure getWebPushBrowserState and reconcile use this deferred
path so eventual registration reports the registered/enrolled state rather than
unsupported.
---
Duplicate comments:
In `@crates/kernel/ironclaw_host_runtime/src/egress/vapid.rs`:
- Around line 87-101: Bind the advertised VAPID key to the signing key in the
signing flow: decode public_key_b64url, compare it with
EcdsaKeyPair::public_key().as_ref(), and return
VapidHeaderError::MaterialInvalid on mismatch. Remove the now-redundant
standalone public-key shape validation around the key-pair setup, while
preserving the existing signature generation and encoded key output.
🪄 Autofix
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: ab558247-63dc-460c-9c88-7d097896143d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (37)
crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_composition/src/factory/production_backend_assembly.rscrates/contracts/ironclaw_host_api/src/http.rscrates/contracts/ironclaw_product_contracts/src/product_wire.rscrates/domains/ironclaw_web_push/Cargo.tomlcrates/domains/ironclaw_web_push/README.mdcrates/domains/ironclaw_web_push/src/error.rscrates/domains/ironclaw_web_push/src/message.rscrates/domains/ironclaw_web_push/src/store.rscrates/domains/ironclaw_web_push/src/subscription.rscrates/domains/ironclaw_web_push/src/vapid.rscrates/extensions/AGENTS.mdcrates/extensions/packages/web-push/README.mdcrates/extensions/packages/web-push/manifest.tomlcrates/extensions/packages/web-push/src/targets.rscrates/kernel/ironclaw_host_runtime/src/egress/vapid.rscrates/product/ironclaw_assistant/src/lib.rscrates/product/ironclaw_assistant/src/reborn_services/web_push.rscrates/product/ironclaw_webui/frontend/public/sw.jscrates/product/ironclaw_webui/frontend/src/i18n/ar.tscrates/product/ironclaw_webui/frontend/src/i18n/de.tscrates/product/ironclaw_webui/frontend/src/i18n/en.tscrates/product/ironclaw_webui/frontend/src/i18n/es.tscrates/product/ironclaw_webui/frontend/src/i18n/fr.tscrates/product/ironclaw_webui/frontend/src/i18n/hi.tscrates/product/ironclaw_webui/frontend/src/i18n/ja.tscrates/product/ironclaw_webui/frontend/src/i18n/ko.tscrates/product/ironclaw_webui/frontend/src/i18n/pt-BR.tscrates/product/ironclaw_webui/frontend/src/i18n/uk.tscrates/product/ironclaw_webui/frontend/src/i18n/zh-CN.tscrates/product/ironclaw_webui/frontend/src/lib/web-push.test.tscrates/product/ironclaw_webui/frontend/src/lib/web-push.tscrates/product/ironclaw_webui/frontend/src/pages/automations/components/notification-channels-panel.test.tscrates/product/ironclaw_webui/frontend/src/pages/automations/components/notification-channels-panel.tsxcrates/product/ironclaw_webui/frontend/src/pages/automations/hooks/useWebPushDevice.tscrates/product/ironclaw_webui/src/webui_v2/handlers.rstests/integration/webui_v2_product_api.rs
Comment on lines
+249
to
280
| /// Minimal unpadded-base64url decode, dependency-free so this stays in the | ||
| /// contracts leaf. Accepts the RFC 4648 URL-safe alphabet without padding. | ||
| fn decode_b64url_no_pad(value: &str) -> Option<Vec<u8>> { | ||
| fn sextet(byte: u8) -> Option<u8> { | ||
| match byte { | ||
| b'A'..=b'Z' => Some(byte - b'A'), | ||
| b'a'..=b'z' => Some(byte - b'a' + 26), | ||
| b'0'..=b'9' => Some(byte - b'0' + 52), | ||
| b'-' => Some(62), | ||
| b'_' => Some(63), | ||
| _ => None, | ||
| } | ||
| } | ||
| let bytes = value.as_bytes(); | ||
| if bytes.len() % 4 == 1 { | ||
| return None; | ||
| } | ||
| let mut out = Vec::with_capacity(bytes.len() / 4 * 3 + 2); | ||
| for chunk in bytes.chunks(4) { | ||
| let mut acc = 0u32; | ||
| for &byte in chunk { | ||
| acc = (acc << 6) | u32::from(sextet(byte)?); | ||
| } | ||
| let pad = 4 - chunk.len(); | ||
| acc <<= 6 * pad as u32; | ||
| let take = 3 - pad; | ||
| for index in 0..take { | ||
| out.push((acc >> (16 - 8 * index)) as u8); | ||
| } | ||
| } | ||
| Some(out) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Do any contracts-family manifests already take third-party utility deps?
fd -t f 'Cargo.toml' crates/contracts --exec sh -c 'echo "== $1"; sed -n "/\[dependencies\]/,/^\[/p" "$1"' _ {}
# Does the architecture suite pin an allowlist for contracts dependencies?
rg -n -C4 'contracts' crates/app/ironclaw_architecture_tests/tests --iglob '*depend*'Repository: nearai/ironclaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== http.rs relevant occurrences =="
rg -n "decode_b64url_no_pad|base64|URL_SAFE|vapid|no_pad|pad" crates/contracts/ironclaw_host_api/src/http.rs crates/contracts/ironclaw_common/src crates/contracts/ironclaw_product_contracts/src crates/contracts/ironclaw_loop_contracts/src crates/extensions -g '*.rs' --max-count 80
echo
echo "== read relevant decode helper =="
sed -n '230,300p' crates/contracts/ironclaw_host_api/src/http.rs
echo
echo "== read helper call sites =="
rg -n -C6 "decode_b64url_no_pad" crates/contracts/ironclaw_host_api/src/http.rs
echo
echo "== compare with known shared decoder =="
rg -n -C8 "decode_url.*no_pad|base64::URL_SAFE_NO_PAD|URL_SAFE_NO_PAD" crates --g '*.rs' --max-count 80Repository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== base64 workspace dependency declarations =="
rg -n "^\s*base64\s*=" Cargo.toml crates/contracts/ironclaw_host_api/Cargo.toml crates/contracts/ironclaw_common/Cargo.toml packages -g 'Cargo.toml'
echo
echo "== architecture boundary test code framing for contracts crates =="
sed -n '754,832p', 'crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs'Repository: nearai/ironclaw
Length of output: 347
Use shared base64::URL_SAFE_NO_PAD here.
base64 is admissible in contracts and already used by crates/contracts/ironclaw_common for unpadded base64url decoding, so this helper should share that engine instead of a bare-minimum decoder that accepts non-canonical trailing sextets.
🤖 Prompt for 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.
In `@crates/contracts/ironclaw_host_api/src/http.rs` around lines 249 - 280,
Replace the custom decode_b64url_no_pad implementation, including its local
sextet decoder, with decoding through the shared base64::URL_SAFE_NO_PAD engine.
Preserve the helper’s Option<Vec<u8>> interface by mapping decode failures to
None, and ensure canonical trailing-bit validation comes from the shared engine.
Source: Coding guidelines
Comment on lines
+90
to
+106
| fn fit_body_to_byte_budget(&mut self) { | ||
| while self.serialized_len() > MAX_PAYLOAD_JSON_BYTES { | ||
| let char_count = self.body.chars().count(); | ||
| if char_count == 0 { | ||
| break; | ||
| } | ||
| // Drop a proportional chunk to converge quickly on large payloads, | ||
| // always at least one character, then re-mark the truncation. | ||
| let overshoot = self.serialized_len() - MAX_PAYLOAD_JSON_BYTES; | ||
| let drop = (overshoot / 2).max(1).min(char_count); | ||
| let kept = char_count.saturating_sub(drop).saturating_sub(1); | ||
| let mut trimmed: String = self.body.chars().take(kept).collect(); | ||
| if kept < char_count { | ||
| trimmed.push(ELLIPSIS); | ||
| } | ||
| self.body = trimmed; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Terminate fitting when body cannot shrink.
At Line 100, a one-character body becomes ELLIPSIS again. If url, title, or tag still exceeds the budget, the next iteration rebuilds the same body and never returns. The unbounded app-relative URL accepted at Line 132 makes this path reachable.
Clear the final character and retry, or stop fitting and let to_json_bytes() return PayloadTooLarge. Add a regression case with an oversized non-body field.
Proposed fix
while self.serialized_len() > MAX_PAYLOAD_JSON_BYTES {
let char_count = self.body.chars().count();
if char_count == 0 {
break;
}
+ if char_count == 1 {
+ self.body.clear();
+ continue;
+ }
// Drop a proportional chunk to converge quickly on large payloads,As per coding guidelines, apply explicit limits to user-controlled strings and add a regression test for every bug fix. As per path instructions, the “Fail loud” invariant requires failure to propagate instead of entering a non-terminating fallback.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn fit_body_to_byte_budget(&mut self) { | |
| while self.serialized_len() > MAX_PAYLOAD_JSON_BYTES { | |
| let char_count = self.body.chars().count(); | |
| if char_count == 0 { | |
| break; | |
| } | |
| // Drop a proportional chunk to converge quickly on large payloads, | |
| // always at least one character, then re-mark the truncation. | |
| let overshoot = self.serialized_len() - MAX_PAYLOAD_JSON_BYTES; | |
| let drop = (overshoot / 2).max(1).min(char_count); | |
| let kept = char_count.saturating_sub(drop).saturating_sub(1); | |
| let mut trimmed: String = self.body.chars().take(kept).collect(); | |
| if kept < char_count { | |
| trimmed.push(ELLIPSIS); | |
| } | |
| self.body = trimmed; | |
| } | |
| fn fit_body_to_byte_budget(&mut self) { | |
| while self.serialized_len() > MAX_PAYLOAD_JSON_BYTES { | |
| let char_count = self.body.chars().count(); | |
| if char_count == 0 { | |
| break; | |
| } | |
| if char_count == 1 { | |
| self.body.clear(); | |
| continue; | |
| } | |
| // Drop a proportional chunk to converge quickly on large payloads, | |
| // always at least one character, then re-mark the truncation. | |
| let overshoot = self.serialized_len() - MAX_PAYLOAD_JSON_BYTES; | |
| let drop = (overshoot / 2).max(1).min(char_count); | |
| let kept = char_count.saturating_sub(drop).saturating_sub(1); | |
| let mut trimmed: String = self.body.chars().take(kept).collect(); | |
| if kept < char_count { | |
| trimmed.push(ELLIPSIS); | |
| } | |
| self.body = trimmed; | |
| } |
🤖 Prompt for 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.
In `@crates/domains/ironclaw_web_push/src/message.rs` around lines 90 - 106,
Update fit_body_to_byte_budget to guarantee progress when truncating a
one-character body: avoid rebuilding the same ELLIPSIS value, either clear the
final character and retry or stop fitting so to_json_bytes() returns
PayloadTooLarge. Add a regression test covering an oversized non-body field such
as the unbounded URL and verify fitting terminates with the error propagated.
Sources: Coding guidelines, Path instructions
Comment on lines
+86
to
+88
| /// Registry key composition registers the provider under — the extension id, | ||
| /// so the provider key and the manifest identity cannot drift. | ||
| pub const WEB_PUSH_TARGET_PROVIDER_KEY: &str = ironclaw_web_push::WEB_PUSH_EXTENSION_ID; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'WEB_PUSH_EXTENSION_ID' crates
rg -n --type=rust -C2 'from_trusted\("web-push"'
rg -n -C3 'WEB_PUSH_TARGET_PROVIDER_KEY' cratesRepository: nearai/ironclaw
Length of output: 898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching targets/grammar/lib/bundles/native_extensions:\n'
fd -a 'targets\.rs|grammar\.rs|lib\.rs|native_extensions\.rs|bundles\.rs|factory.*assembly\.rs' crates | sed 's#^\./##'
printf '\nRelevant occurrences:\n'
rg -n --type=rust -C3 'from_trusted|allowed_push_hosts|WEB_PUSH_TARGET_PROVIDER_KEY|ExtensionId|runtime/.native-extensions|native_extensions' crates
printf '\nInspect native_extensions.rs around binding:\n'
cat -n crates/app/ironclaw_cli/src/runtime/native_extensions.rs | sed -n '1,180p'
printf '\nInspect targets.rs around key:\n'
cat -n crates/extensions/packages/web-push/src/targets.rs | sed -n '1,150p'Repository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'native_extensions.rs:\n'
cat -n crates/app/ironclaw_cli/src/runtime/native_extensions.rs | sed -n '1,220p'
printf '\nlocal occurrences only:\n'
rg -n --type=rust -C3 'from_trusted\("web-push"|IRONCRAWL.*WEB_PUSH|WEB_PUSH_EXTENSION_ID|allowed_push_hosts|assemble_web_push|WEB_PUSH_TARGET_PROVIDER_KEY' crates/app crates/domains crates/extensions/packages/web-push
printf '\nnative_extensions target extensions definitions:\n'
rg -n -C3 'ExtensionId::from_trusted|channel_bindings|ExtensionTarget|push_hosts|allow.*push|push' crates/app/ironclaw_cli/src/runtime/native_extensions.rsRepository: nearai/ironclaw
Length of output: 23693
Route the Web Push CLI binding through the shared extension id.
crates/app/ironclaw_cli/src/runtime/native_extensions.rs constructs the channel binding with ExtensionId::from_trusted("web-push".to_string()), while the manifest, first-party bundle, provider registry, and assemble_web_push lookup use ironclaw_web_push::WEB_PUSH_EXTENSION_ID. If that shared constant changes, the binding and lookup keys drift silently; the host reads an absent binding and allowed_push_hosts resolves empty, causing Web Push enrollment to fail closed. Use ironclaw_web_push::WEB_PUSH_EXTENSION_ID for the CLI binding too.
🤖 Prompt for 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.
In `@crates/extensions/packages/web-push/src/targets.rs` around lines 86 - 88,
Update the Web Push CLI binding construction in native extension runtime setup
to use ironclaw_web_push::WEB_PUSH_EXTENSION_ID instead of the hard-coded
"web-push" value. Keep the binding’s existing ExtensionId conversion and ensure
it matches the provider registry and assemble_web_push lookup key.
Comment on lines
+38
to
+54
| async function pushRegistration() { | ||
| // Deliberately `getRegistration()`, never `serviceWorker.ready`: `ready` | ||
| // resolves only once SOME registration activates and never rejects, so | ||
| // after a failed boot registration (non-secure origin, /sw.js 404, | ||
| // private mode) it hangs forever and every state probe hangs with it. | ||
| // `getRegistration()` resolves promptly with `undefined` in exactly those | ||
| // cases, which callers surface as "unsupported". | ||
| if (!navigator.serviceWorker || typeof navigator.serviceWorker.getRegistration !== "function") { | ||
| return null; | ||
| } | ||
| try { | ||
| const registration = await navigator.serviceWorker.getRegistration(); | ||
| if (!registration || !registration.pushManager) return null; | ||
| return registration; | ||
| } catch (_) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a '^main\.tsx$' crates/product/ironclaw_webui/frontend | while IFS= read -r file; do
ast-grep outline "$file" --items all
rg -n -C 4 '\bregisterServiceWorker\s*\(' "$file"
done
rg -n -C 5 'pushRegistration|registerServiceWorker|getWebPushBrowserState' \
crates/product/ironclaw_webui/frontend/src/lib/web-push.ts \
crates/product/ironclaw_webui/frontend/src/lib/web-push.test.tsRepository: nearai/ironclaw
Length of output: 17840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant web-push.ts =="
nl -ba crates/product/ironclaw_webui/frontend/src/lib/web-push.ts | sed -n '1,130p'
echo
echo "== registry/browser mock setup =="
nl -ba crates/product/ironclaw_webui/frontend/src/lib/web-push.test.ts | sed -n '1,180p'
echo
echo "== all web-push exports/usages =="
rg -n 'registerServiceWorker|pushRegistration|getWebPushBrowserState|getRegistration|serviceWorker\.ready' crates/product/ironclaw_webui/frontend/src -g '*.ts' -g '*.tsx'Repository: nearai/ironclaw
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant web-push.ts =="
awk '{printf "%6d\t%s\n", NR, $0}' crates/product/ironclaw_webui/frontend/src/lib/web-push.ts | sed -n '1,130p'
echo
echo "== registry/browser mock setup =="
awk '{printf "%6d\t%s\n", NR, $0}' crates/product/ironclaw_webui/frontend/src/lib/web-push.test.ts | sed -n '1,180p'
echo
echo "== all web-push exports/usages =="
rg -n 'registerServiceWorker|pushRegistration|getWebPushBrowserState|getRegistration|serviceWorker\.ready' crates/product/ironclaw_webui/frontend/src -g '*.ts' -g '*.tsx'Repository: nearai/ironclaw
Length of output: 19052
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useWebPushDevice hook =="
awk '{printf "%6d\t%s\n", NR, $0}' crates/product/ironclaw_webui/frontend/src/pages/automations/hooks/useWebPushDevice.ts | sed -n '1,140p'
echo
echo "== candidate React state reconciliation locations =="
rg -n -C 3 'useWebPushDevice|getWebPushBrowserState|Web Push|web push|push state|enrolled|unsupported' crates/product/ironclaw_webui/frontend/src -g '*.ts' -g '*.tsx'
echo
echo "== package runtime context if available =="
if [ -f crates/product/ironclaw_webui/frontend/package.json ]; then
awk '{printf "%6d\t%s\n", NR, $0}' crates/product/ironclaw_webui/frontend/package.json | sed -n '1,220p'
fiRepository: nearai/ironclaw
Length of output: 50372
🌐 Web query:
MDN Navigator ServiceWorkerContainer getRegistration undefined registration pending
💡 Result:
When using navigator.serviceWorker.getRegistration(), receiving undefined as the resolved value indicates that no service worker registration exists for the specified scope (or the default scope of the current page) at the time the method is called [1][2]. Key points regarding this behavior: 1. Immediate Execution: Unlike navigator.serviceWorker.ready, which waits indefinitely until a service worker is active [3], getRegistration() executes immediately [4]. If no registration has been established (via a successful register() call) before getRegistration() is invoked, it will resolve to undefined [1][5]. 2. Pending Registrations: There is no native "pending" state for getRegistration() itself. If a registration process is currently in progress (e.g., the script is still being fetched or parsed), getRegistration() may still return undefined if the registration has not yet been fully created and added to the scope-to-registration map [6]. 3. Recommended Patterns: - If you need to ensure a service worker is active before performing an action, use navigator.serviceWorker.ready [7][3]. This property returns a promise that resolves only when a registration has an active service worker, avoiding the timing issues associated with getRegistration() [3]. - If you must check for an existing registration, ensure that your logic accounts for the possibility of an undefined result by checking the return value before accessing properties or methods on it [1][5]. Example of handling the undefined result: navigator.serviceWorker.getRegistration.then((registration) => { if (registration) { // Registration exists } else { // No registration found, handle accordingly } });
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerContainer/getRegistration
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/api/serviceworkercontainer/getregistration/index.md
- 3: https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerContainer/ready
- 4: [Proposal] ServiceWorkerContainer.installing(scope) promise w3c/ServiceWorker#1364
- 5: https://stackoverflow.com/questions/69284764/getregistration-then-gives-just-undefined
- 6: Using registrations that failed to install a ServiceWorker w3c/ServiceWorker#1491
- 7: https://stackoverflow.com/questions/28789395/difference-between-serviceworker-getregistration-and-serviceworker-ready-then
Wait for the registered service worker before reporting unsupported.
crates/product/ironclaw_webui/frontend/src/main.tsx non-blockingly calls registerServiceWorker() before useWebPushDevice probes state. pushRegistration() uses getRegistration(), so it can return null while /sw.js registration is still pending. That makes the panel render unsupported instead of checking/reconcile.
Store the registration promise and await it before any probe. Bound the subsequent active-registration wait, and cover the deferred-registration path so getWebPushBrowserState() returns the registered/enrolled state when the registration eventually activates.
🤖 Prompt for 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.
In `@crates/product/ironclaw_webui/frontend/src/lib/web-push.ts` around lines 38 -
54, Update pushRegistration to retain and await the in-flight
registerServiceWorker promise before probing getRegistration, including when
registration is deferred by main.tsx. After registration resolves, wait only a
bounded period for an active registration, then return the registration or null
without hanging. Ensure getWebPushBrowserState and reconcile use this deferred
path so eventual registration reports the registered/enrolled state rather than
unsupported.
…ions # Conflicts: # crates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rs # crates/contracts/ironclaw_product_contracts/AGENTS.md # scripts/ci/composition-budget.toml
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 8, 2026 23:56
Destroyed
…raise the chat budget for the feature's i18n copy The merge-lane bundle-budget gate (skipped on the PR lane) flagged the initial /chat JS at 218.2 KB against the 217.0 KB gzip budget. `main.tsx` boot-imported `registerServiceWorker` from `lib/web-push.ts`, which dragged the enrollment lib's api-client and WebCrypto imports into the initial chunk. Extracted the dependency-free `lib/register-sw.ts` for boot; the enrollment API stays in `web-push.ts`, imported only by the already-lazy automations route. That recovered the eager-code weight (218.2 -> 217.4 KB); the residual is the feature's new `en.ts` fallback-pack strings, so the /chat budget is raised 217.0 -> 218.0 KB with rationale, matching how prior features (hosted MCP, Router 8) handled eager localized copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 9, 2026 00:04
Destroyed
…gate The merge-lane product-surface evidence gate (test_journey_coverage.py, skipped on the PR lane) requires every outbound channel manifest to name exact journey evidence. web-push declares `outbound = true`, so it entered the required delivery-target set with none. Added: - `JourneyDeliveryTarget.WEB_PUSH` and a `ProductJourneyCase` citing the existing `blocked_fire_pushes_web_push_notice_to_enrolled_browser` integration test, with unthreaded (`thread_anchor=None`) delivery-address evidence — browser push addresses a per-browser endpoint capability URL, not a conversation thread. - `assert_web_push_delivery_evidence` in delivery_user_journeys.rs, shaped as the gate's citability check requires (literal `expected_conversation_id` gating the count, `expected_thread_anchor = None`), called from that test. Verified: test_journey_coverage.py (68), test_product_surface_coverage.py + test_provider_capability_inventory.py (30), and the cited integration test all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 9, 2026 00:15
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/product/ironclaw_webui/frontend/src/lib/web-push.ts (1)
187-210: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not report local unsubscription before backend removal succeeds.
unenrollThisBrowser()callsawait subscription.unsubscribe();beforeunsubscribeWebPush({ endpoint }), then returnsnot-enrolledeven whenunsubscribeWebPushfails. If the server-side record exists after a failedPOST /web-push/subscriptions/remove, future checks can report stale enrollment. Require the idempotentremovedacknowledgement fromRebornWebPushUnsubscribeResponsebefore reporting local cleanup, and only suppress explicit already-absent responses. Add tests for network and store failures.🤖 Prompt for 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. In `@crates/product/ironclaw_webui/frontend/src/lib/web-push.ts` around lines 187 - 210, Update unenrollThisBrowser to require the removed acknowledgement from RebornWebPushUnsubscribeResponse before returning not-enrolled. Do not unsubscribe locally until unsubscribeWebPush succeeds, and only suppress explicitly already-absent responses; propagate network and store failures instead of treating them as successful cleanup. Add tests covering both failure paths and the successful removed/absent responses.Source: Coding guidelines
🤖 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/contracts/ironclaw_product_contracts/AGENTS.md`:
- Around line 57-58: Reconcile the shipped-module inventory in AGENTS.md with
the 33 public modules declared in src/lib.rs: add the missing inspector row and
any other omitted modules, update the stated count and table total to 33, and
remove or adjust entries only when the corresponding module is no longer
shipped.
---
Outside diff comments:
In `@crates/product/ironclaw_webui/frontend/src/lib/web-push.ts`:
- Around line 187-210: Update unenrollThisBrowser to require the removed
acknowledgement from RebornWebPushUnsubscribeResponse before returning
not-enrolled. Do not unsubscribe locally until unsubscribeWebPush succeeds, and
only suppress explicitly already-absent responses; propagate network and store
failures instead of treating them as successful cleanup. Add tests covering both
failure paths and the successful removed/absent responses.
🪄 Autofix
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: b418900c-0084-4040-a292-92503c70e748
📒 Files selected for processing (16)
crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rscrates/app/ironclaw_architecture_tests/tests/reborn_extension_specificity.rscrates/app/ironclaw_architecture_tests/tests/reborn_restructure_baselines.rscrates/contracts/ironclaw_product_contracts/AGENTS.mdcrates/contracts/ironclaw_product_contracts/src/lib.rscrates/product/ironclaw_assistant/AGENTS.mdcrates/product/ironclaw_assistant/src/lib.rscrates/product/ironclaw_webui/frontend/scripts/check-bundle-budgets.tscrates/product/ironclaw_webui/frontend/src/lib/register-sw.tscrates/product/ironclaw_webui/frontend/src/lib/web-push.tscrates/product/ironclaw_webui/frontend/src/main.tsxdocs/reborn/target-architecture/PROPOSAL.mdscripts/ci/composition-budget.tomltests/integration/delivery_user_journeys.rstests/integration/support/group_constructors.rstests/integration/webui_v2_product_api.rs
Comment on lines
+57
to
58
| | `web_push` | The web-push enrollment operation descriptors: `WEB_PUSH_STATUS_VIEW` and the `web_push.subscribe` / `web_push.unsubscribe` command descriptors (+ ids). Descriptors only — enrollment behavior, storage, and VAPID custody stay in `ironclaw_assistant` / `ironclaw_web_push` / composition. | | ||
| | `shared_admission` | Shared-conversation admission: `SharedConversationAdmission` + `ProductConversationRouteKey` and its request. Fail-closed connected-channel gating, implemented by `ironclaw_extension_host` over `[channel.config]`. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant AGENTS.md section =="
sed -n '1,80p' crates/contracts/ironclaw_product_contracts/AGENTS.md
echo
echo "== pub mods in lib.rs =="
rg -n '^\s*pub\s+mod\s+[A-Za-z_][A-Za-z0-9_]*\s*;' crates/contracts/ironclaw_product_contracts/src/lib.rs
echo
echo "== pub mod count =="
count=$(rg '^\s*pub\s+mod\s+[A-Za-z_][A-Za-z0-9_]*\s*;' crates/contracts/ironclaw_product_contracts/src/lib.rs | wc -l)
echo "$count"
echo
echo "== table rows with module names =="
rg -n '^\| \`[A-Za-z_][A-Za-z0-9_-]*\` \||web|shared_admission|subject_route' crates/contracts/ironclaw_product_contracts/AGENTS.mdRepository: nearai/ironclaw
Length of output: 18215
Update the shipped-module inventory to match src/lib.rs.
crates/contracts/ironclaw_product_contracts/src/lib.rs declares 33 shipped pub mods, but the guide says thirty-one and the table documents 31 rows while omitting inspector. Either add the missing row(s) or update the shipped count, and remove no longer shipped modules from src/lib.rs if they should stay documented in the table.
🤖 Prompt for 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.
In `@crates/contracts/ironclaw_product_contracts/AGENTS.md` around lines 57 - 58,
Reconcile the shipped-module inventory in AGENTS.md with the 33 public modules
declared in src/lib.rs: add the missing inspector row and any other omitted
modules, update the stated count and table total to 33, and remove or adjust
entries only when the corresponding module is no longer shipped.
`cargo fmt --all -- --check` (the first step of the Fast deterministic checks lane, PR-lane-skipped) flagged the iterator chain in `assert_web_push_delivery_evidence`. Formatting only, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 9, 2026 00:22
Destroyed
…install catalog The web-app's browser-push channel is host infrastructure, not a browse-and-install integration, so: - Rename the extension/channel to "Web UI" (manifest name + channel display_name + first-party bundle label; description reworded). - Hide it from the install catalog (/extensions, /extensions/registry, and the Settings channels list) via an explicit built-in-host-surface classification in the product lifecycle projection — deliberately keyed by id, not inferred from channel direction, so it stays correct as the web-app channel later gains inbound/outbound. Its outbound notification target (/outbound/targets) is a separate registry and is unaffected, so it remains a selectable notification channel. Also finalizes the enrollment UI carried from this session: - Align the "This browser" device block with the channel cards (drop the stray left indent). - Lazy-mount the web-push device hook so its status query fires only when a web-push row is present, not on every automations view. - Rename the misleading webOnlyHelper i18n key to noSelectionHelper across all locale packs (the string already dropped the retired "stays in the web app" claim). - Manifest icons declare purpose "any maskable" so Chrome offers the PWA install prompt. Regression coverage extends the production web-push integration test to assert it is absent from the install endpoints yet present in /outbound/targets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 9, 2026 17:22
Destroyed
BenKurrek
enabled auto-merge
August 9, 2026 17:25
9 of 21 tasks
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 10, 2026 07:58
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 10, 2026 08:31
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 10, 2026 08:58
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 10, 2026 09:28
Destroyed
…ions # Conflicts: # crates/product/ironclaw_webui/frontend/scripts/check-bundle-budgets.ts
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 10, 2026 10:00
Destroyed
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/product/ironclaw_webui/frontend/src/i18n/ja.ts (1)
802-803: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the state copy with enrollment evidence.
notification-channels-panel.tsxmapsnot-enrolledto this key, so通知を受信していませんdescribes the wrong state. Theenrolledkey is also rendered forenrolled-unverified, where account correlation is unavailable;通知を受信しますclaims delivery that the backend has not confirmed. Use enrollment wording, or add a separate unverified-state key.As per coding guidelines: “UI success must reflect backend evidence.”
Proposed wording
- "automations.notificationChannels.webPush.notEnrolled": "このブラウザはまだ通知を受信していません。", + "automations.notificationChannels.webPush.notEnrolled": "このブラウザはまだ登録されていません。", - "automations.notificationChannels.webPush.enrolled": "このブラウザは通知を受信します。", + "automations.notificationChannels.webPush.enrolled": "このブラウザでは通知登録が有効です。",🤖 Prompt for 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. In `@crates/product/ironclaw_webui/frontend/src/i18n/ja.ts` around lines 802 - 803, Update the Japanese translations for automations.notificationChannels.webPush.notEnrolled and automations.notificationChannels.webPush.enrolled to describe enrollment state rather than claiming notification receipt or delivery; ensure the enrolled wording remains accurate for both enrolled and enrolled-unverified states.Source: Coding guidelines
crates/product/ironclaw_webui/frontend/src/i18n/en.ts (1)
846-846: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the no-channel warning in all locale packs.
tests/CLAUDE.mdLine [257] records that an empty notification-channel set keeps a blocked fire app-only. These messages say the notices are delivered nowhere. State that no external notification channel receives the notice, or mention the in-app fallback.
crates/product/ironclaw_webui/frontend/src/i18n/en.ts#L846-L846: reviseautomations.notificationChannels.noSelectionHelper.crates/product/ironclaw_webui/frontend/src/i18n/ko.ts#L797-L797: revise the Koreanautomations.notificationChannels.noSelectionHelpertranslation.crates/product/ironclaw_webui/frontend/src/i18n/pt-BR.ts#L797-L797: revise the Portugueseautomations.notificationChannels.noSelectionHelpertranslation.crates/product/ironclaw_webui/frontend/src/i18n/ar.ts#L797-L797: revise the Arabicautomations.notificationChannels.noSelectionHelpertranslation.🤖 Prompt for 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. In `@crates/product/ironclaw_webui/frontend/src/i18n/en.ts` at line 846, Update automations.notificationChannels.noSelectionHelper in crates/product/ironclaw_webui/frontend/src/i18n/en.ts:846-846, crates/product/ironclaw_webui/frontend/src/i18n/ko.ts:797-797, crates/product/ironclaw_webui/frontend/src/i18n/pt-BR.ts:797-797, and crates/product/ironclaw_webui/frontend/src/i18n/ar.ts:797-797 so each translation states that no external notification channel receives notices, or clearly mentions the in-app fallback; preserve the existing meaning and language of each locale.tests/CLAUDE.md (1)
67-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReconcile the checked-in scenario counts.
Line [67] reports 102 Python scenario files and 869 test functions, but Line [313] reports 103 files and 1,141 tests. Recompute the inventory and keep both sections synchronized.
As per coding guidelines, "The counts in each section header are checked-in facts, not estimates. Update them."
🤖 Prompt for 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. In `@tests/CLAUDE.md` at line 67, Recompute the checked-in Python scenario-file and test-function inventory, then update the counts in both sections of tests/CLAUDE.md, including the headers around the reported lines, so they match exactly.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@crates/product/ironclaw_webui/frontend/src/i18n/en.ts`:
- Line 846: Update automations.notificationChannels.noSelectionHelper in
crates/product/ironclaw_webui/frontend/src/i18n/en.ts:846-846,
crates/product/ironclaw_webui/frontend/src/i18n/ko.ts:797-797,
crates/product/ironclaw_webui/frontend/src/i18n/pt-BR.ts:797-797, and
crates/product/ironclaw_webui/frontend/src/i18n/ar.ts:797-797 so each
translation states that no external notification channel receives notices, or
clearly mentions the in-app fallback; preserve the existing meaning and language
of each locale.
In `@crates/product/ironclaw_webui/frontend/src/i18n/ja.ts`:
- Around line 802-803: Update the Japanese translations for
automations.notificationChannels.webPush.notEnrolled and
automations.notificationChannels.webPush.enrolled to describe enrollment state
rather than claiming notification receipt or delivery; ensure the enrolled
wording remains accurate for both enrolled and enrolled-unverified states.
In `@tests/CLAUDE.md`:
- Line 67: Recompute the checked-in Python scenario-file and test-function
inventory, then update the counts in both sections of tests/CLAUDE.md, including
the headers around the reported lines, so they match exactly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ce0b6f7-5d6a-464e-bd48-6158842019e5
📒 Files selected for processing (14)
crates/product/ironclaw_webui/CONTRACT.mdcrates/product/ironclaw_webui/frontend/scripts/check-bundle-budgets.tscrates/product/ironclaw_webui/frontend/src/i18n/ar.tscrates/product/ironclaw_webui/frontend/src/i18n/de.tscrates/product/ironclaw_webui/frontend/src/i18n/en.tscrates/product/ironclaw_webui/frontend/src/i18n/es.tscrates/product/ironclaw_webui/frontend/src/i18n/fr.tscrates/product/ironclaw_webui/frontend/src/i18n/hi.tscrates/product/ironclaw_webui/frontend/src/i18n/ja.tscrates/product/ironclaw_webui/frontend/src/i18n/ko.tscrates/product/ironclaw_webui/frontend/src/i18n/pt-BR.tscrates/product/ironclaw_webui/frontend/src/i18n/uk.tscrates/product/ironclaw_webui/frontend/src/i18n/zh-CN.tstests/CLAUDE.md
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7398
August 10, 2026 10:29
Destroyed
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/product/ironclaw_webui/frontend/src/i18n/fr.ts (1)
803-804: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse enrollment terminology for
webPush.notEnrolledandwebPush.enrolled. Both translations describe notification receipt instead of subscription registration state.
crates/product/ironclaw_webui/frontend/src/i18n/fr.ts#L803-L804: State that the browser is not yet registered and is registered to receive notifications.crates/product/ironclaw_webui/frontend/src/i18n/ja.ts#L803-L804: State that the browser is not yet registered and is registered to receive notifications.🤖 Prompt for 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. In `@crates/product/ironclaw_webui/frontend/src/i18n/fr.ts` around lines 803 - 804, Update the webPush.notEnrolled and webPush.enrolled translations to use browser enrollment/registration terminology rather than notification receipt: modify both entries in crates/product/ironclaw_webui/frontend/src/i18n/fr.ts lines 803-804 and crates/product/ironclaw_webui/frontend/src/i18n/ja.ts lines 803-804 so they state that the browser is not yet registered and is registered to receive notifications, respectively.crates/product/ironclaw_webui/frontend/src/i18n/hi.ts (1)
803-804: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep Web Push state labels about enrollment, not delivery.
All three locale packs translate
notEnrolledandenrolledas whether the browser receives notifications. These keys represent enrollment state. Use natural translations of “not enrolled” and “enrolled” so the UI does not claim provider delivery.
crates/product/ironclaw_webui/frontend/src/i18n/hi.ts#L803-L804: revise the Hindi state labels.crates/product/ironclaw_webui/frontend/src/i18n/zh-CN.ts#L802-L803: revise the Simplified Chinese state labels.crates/product/ironclaw_webui/frontend/src/i18n/ko.ts#L803-L804: revise the Korean state labels.🤖 Prompt for 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. In `@crates/product/ironclaw_webui/frontend/src/i18n/hi.ts` around lines 803 - 804, Revise the automations.notificationChannels.webPush.notEnrolled and enrolled labels in hi.ts (lines 803-804), zh-CN.ts (lines 802-803), and ko.ts (lines 803-804) to use natural translations of “not enrolled” and “enrolled,” describing enrollment state rather than whether notifications are received.
🤖 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.
Outside diff comments:
In `@crates/product/ironclaw_webui/frontend/src/i18n/fr.ts`:
- Around line 803-804: Update the webPush.notEnrolled and webPush.enrolled
translations to use browser enrollment/registration terminology rather than
notification receipt: modify both entries in
crates/product/ironclaw_webui/frontend/src/i18n/fr.ts lines 803-804 and
crates/product/ironclaw_webui/frontend/src/i18n/ja.ts lines 803-804 so they
state that the browser is not yet registered and is registered to receive
notifications, respectively.
In `@crates/product/ironclaw_webui/frontend/src/i18n/hi.ts`:
- Around line 803-804: Revise the
automations.notificationChannels.webPush.notEnrolled and enrolled labels in
hi.ts (lines 803-804), zh-CN.ts (lines 802-803), and ko.ts (lines 803-804) to
use natural translations of “not enrolled” and “enrolled,” describing enrollment
state rather than whether notifications are received.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9fad608e-5a6f-4c25-a083-2291b01af526
📒 Files selected for processing (11)
crates/product/ironclaw_webui/frontend/src/i18n/ar.tscrates/product/ironclaw_webui/frontend/src/i18n/de.tscrates/product/ironclaw_webui/frontend/src/i18n/en.tscrates/product/ironclaw_webui/frontend/src/i18n/es.tscrates/product/ironclaw_webui/frontend/src/i18n/fr.tscrates/product/ironclaw_webui/frontend/src/i18n/hi.tscrates/product/ironclaw_webui/frontend/src/i18n/ja.tscrates/product/ironclaw_webui/frontend/src/i18n/ko.tscrates/product/ironclaw_webui/frontend/src/i18n/pt-BR.tscrates/product/ironclaw_webui/frontend/src/i18n/uk.tscrates/product/ironclaw_webui/frontend/src/i18n/zh-CN.ts
serrrfirat
approved these changes
Aug 10, 2026
20 tasks
6 of 21 tasks
This was referenced Aug 13, 2026
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
…irst-party notification channel (nearai#7398) * feat(web-push): browser push notifications + PWA — the web app as a first-party notification channel The web app becomes a real, selectable notification route for automations, at parity with Slack/Telegram: a bundled first-party channel extension (web-push) delivers W3C Web Push (RFC 8030/8291/8292) to the user's enrolled browsers through the existing catalog → notification-channel set → notifier → delivery coordinator → channel adapter → policy-enforced egress chain, with zero new routing machinery and the two-lane delivery contract untouched. The WebUI ships as an installable PWA (root-scope service worker with push + notification-click deep links; manifest and icons already existed), and the automations page's notification-channels panel replaces the always-on "web app" placeholder with a real toggleable row plus a per-browser enroll/disable flow. New crates: ironclaw_web_push (domain: subscription records + CAS store, RFC 8291 aes128gcm encryption pinned to the RFC's Appendix A vector, VAPID key-material generation, transport-free request planning, channel identity grammar, late-bound runtime slot) and ironclaw_web_push_extension (channel package: manifest with vapid_authorization egress injection, adapter with 404/410 pruning and honest Sent-without-ref evidence, personal-DM codec, owner-scoped catalog provider). One generic host addition: the RuntimeCredentialTarget::VapidAuthorization egress injection kind — the host signs the RFC 8292 ES256 JWT at the existing credential chokepoint with the audience derived from the request's own push-service origin; adapters never see key bytes. VAPID material is auto-generated and seeded at composition boot. Enrollment is an authenticated product surface (three new /api/webchat/v2/web-push routes) with descriptors declared in ironclaw_product_contracts::web_push per the transport/product boundary, and endpoints validate against the manifest-declared push-service hosts. Also fixes a boot bug the new integration tests exposed: DeploymentChannelBinding rejected outbound-only channels, which would have failed the runtime build at serve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-push): address review — redact VAPID secret, byte-budget payload, account-scoped enrollment, sanitized store errors Triage of the CodeRabbit + IronLoop review on nearai#7398: - Secret exposure (Critical): hand-written redacting `Debug` on `VapidCredentialMaterialV1` and `GeneratedVapidKeyMaterial` so the ES256 private key can never reach a log, panic, or `{:?}`. - VAPID material (High): composition now ensures-then-reads-back the canonical stored keypair so multi-replica cold-start converges on one signing key and the advertised applicationServerKey matches it; material shape is validated at boot and again before egress signing. - Payload budget (Medium): notification body is trimmed by serialized-JSON bytes, not character count, so multi-byte content can't blow the single-record push budget (+ regression test). - Error hygiene (Medium): `WebPushError::Store` is a fixed sanitized category; the backend cause is logged server-side, never rendered into the boundary error. - Account-scoped enrollment (Medium): status projects an `endpoint_digest` (SHA-256 hex) so a shared browser profile distinguishes "enrolled here" from "enrolled for another account" without the endpoint URL leaving the backend; the frontend correlates on it and only offers destructive disable when verified. - Deep-link/tag grammar enforced in the owning crate; SW validates same-origin before navigating; subject parsed via `url`; concurrent-writer CAS test; registry key derived from the extension-id constant; parse-cause preserved; doc/count corrections. The manifest keeps the `web_push_vapid` field (a channel egress credential handle must be declared in [admin_configuration]); it stays host-seeded and not operator-supplied, with the rotation caveat documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-push): keep SW registration out of the initial /chat bundle; raise the chat budget for the feature's i18n copy The merge-lane bundle-budget gate (skipped on the PR lane) flagged the initial /chat JS at 218.2 KB against the 217.0 KB gzip budget. `main.tsx` boot-imported `registerServiceWorker` from `lib/web-push.ts`, which dragged the enrollment lib's api-client and WebCrypto imports into the initial chunk. Extracted the dependency-free `lib/register-sw.ts` for boot; the enrollment API stays in `web-push.ts`, imported only by the already-lazy automations route. That recovered the eager-code weight (218.2 -> 217.4 KB); the residual is the feature's new `en.ts` fallback-pack strings, so the /chat budget is raised 217.0 -> 218.0 KB with rationale, matching how prior features (hosted MCP, Router 8) handled eager localized copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(web-push): register the browser channel in the journey coverage gate The merge-lane product-surface evidence gate (test_journey_coverage.py, skipped on the PR lane) requires every outbound channel manifest to name exact journey evidence. web-push declares `outbound = true`, so it entered the required delivery-target set with none. Added: - `JourneyDeliveryTarget.WEB_PUSH` and a `ProductJourneyCase` citing the existing `blocked_fire_pushes_web_push_notice_to_enrolled_browser` integration test, with unthreaded (`thread_anchor=None`) delivery-address evidence — browser push addresses a per-browser endpoint capability URL, not a conversation thread. - `assert_web_push_delivery_evidence` in delivery_user_journeys.rs, shaped as the gate's citability check requires (literal `expected_conversation_id` gating the count, `expected_thread_anchor = None`), called from that test. Verified: test_journey_coverage.py (68), test_product_surface_coverage.py + test_provider_capability_inventory.py (30), and the cited integration test all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(web-push): rustfmt the journey-evidence helper `cargo fmt --all -- --check` (the first step of the Fast deterministic checks lane, PR-lane-skipped) flagged the iterator chain in `assert_web_push_delivery_evidence`. Formatting only, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web-push): present the channel as "Web UI" and hide it from the install catalog The web-app's browser-push channel is host infrastructure, not a browse-and-install integration, so: - Rename the extension/channel to "Web UI" (manifest name + channel display_name + first-party bundle label; description reworded). - Hide it from the install catalog (/extensions, /extensions/registry, and the Settings channels list) via an explicit built-in-host-surface classification in the product lifecycle projection — deliberately keyed by id, not inferred from channel direction, so it stays correct as the web-app channel later gains inbound/outbound. Its outbound notification target (/outbound/targets) is a separate registry and is unaffected, so it remains a selectable notification channel. Also finalizes the enrollment UI carried from this session: - Align the "This browser" device block with the channel cards (drop the stray left indent). - Lazy-mount the web-push device hook so its status query fires only when a web-push row is present, not on every automations view. - Rename the misleading webOnlyHelper i18n key to noSelectionHelper across all locale packs (the string already dropped the retired "stays in the web app" claim). - Manifest icons declare purpose "any maskable" so Chrome offers the PWA install prompt. Regression coverage extends the production web-push integration test to assert it is absent from the install endpoints yet present in /outbound/targets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(web-push): fix stale helper name in install-catalog filter comment CodeRabbit flagged that the filter comment still named the removed `is_host_managed_channel` helper and described the old outbound-only heuristic. Point it at `is_builtin_host_surface` and the id-based classification the code actually uses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(web-push): pin notification-helper i18n key as noSelectionHelper The P1 rename webOnlyHelper -> noSelectionHelper missed two test assertions that pin the i18n key, failing the affected-3 Reborn crate bucket (Tests (Reborn) rolls it up): - crates/app/ironclaw_composition/tests/webui_v2_serve.rs (served bundle) - crates/product/ironclaw_webui/src/webui_v2/static_assets/assets.rs (live source) Also renames the now-misleading test fn and corrects a stale bundle-budget comment. Test-only; no production behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(channels): notifications as a first-class channel capability Separates "notification target" from "final-reply / model-delivery target" so the web app is a notification channel (blocked-automation notices) without being a place the model or a run's final reply is delivered to. Folds the notifications-capability work into the web-push PR (nearai#7398). Model / declarative: - ChannelDescriptor gains a `notifications` capability (serde default), so a manifest can independently declare inbound / outbound / notifications. The web-push manifest declares `notifications = true` (kept `outbound = true` for now: it is what registers the channel's delivery binding; outbound thread-creation is a later capability). Pinned by manifest_lockstep. Delivery authority (the load-bearing separation): - DeliveryTargetCapabilities gains a `notifications` bool, and a new `OutboundDeliveryTargetProvider::resolve_notification_target` resolves a target the caller may receive blocked-automation notices on, gated on `notifications` (not `final_replies`). Default impl + registry/mutable-registry overrides; the generic channel provider reuses its id-resolution. - web-push target caps flip to `final_replies: false, notifications: true`: browser push is where a run's reply already lands, never a destination the model/final-reply path delivers *to*. - The notification-channel picker (`/outbound/targets`) and the notifier + set-validation resolution now gate on the notifications capability; the model-facing delivery list (`builtin.outbound_delivery_targets_list`) narrows to `final_replies`, so a notification-only target (browser push) is invisible to the model until it gains outbound delivery. Why this shape: production blocked-automation notices run through TriggeredRunDeliveryDriver -> DeliveryCoordinator -> WebPushChannelAdapter and select targets by capability resolution; the `ThreadNotificationPolicy` / `progress` push-plan in ironclaw_outbound is dormant (no production writer or caller), so gating on `progress` would not have changed delivery. Slack and Telegram are both final-reply and notification targets and are unaffected. Regression coverage: web-push resolves as a notification target but NOT as a final-reply/model target (targets.rs); existing notifier + notification-channel integration journeys stay green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(channels): cargo fmt + drop vendor names from generic-code comments - cargo fmt reflow in notification_channel_resolution.rs. - Reword comments in ironclaw_composition/runtime.rs and ironclaw_extension_host/channel_outbound_targets.rs so generic code does not name Slack/Telegram (reborn_extension_specificity gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs+test(channels): fix notifications doc + serde-default coverage Addresses CodeRabbit on the notifications-capability commit: - channel.rs: reword ChannelDescriptor.notifications doc so it no longer claims the web app declines outbound (the manifest keeps outbound = true for the delivery binding). Net-zero line count (contracts size ceiling unchanged). - delivery_resolution.rs: assert notifications defaults false, and add a legacy-payload (omitted notifications) deserialize test for #[serde(default)]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(outbound): decouple notification-picker list from model-delivery list Addresses the CodeRabbit "Major": final_replies and notifications were coupled through one shared base filter. Now RebornOutboundDeliveryTargetCapabilities carries a wire-level `notifications`; list_outbound_delivery_targets returns the union (final_replies || notifications); the notification picker (build_outbound_delivery_targets_view) filters notifications and the model list (list_outbound_delivery_targets_for_model) filters final_replies — independently. A final-reply-only target is now visible to the model but not the picker; a notification-only target (web-push) shows in the picker but never to the model. Verified: assistant, ironclaw_webui, frontend (1208), delivery_user_journeys (25), web-push round-trip, architecture suite (41). Chose "in nearai#7398" per the overnight directive to fix review comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(outbound): cover divergent notifications/final_replies at the filter seam Addresses CodeRabbit's follow-up: the unit capability-filter seam couldn't construct divergent targets (the fixture set notifications = final_replies). Adds `target_entry_with_caps` (independent notifications) and a divergent-combo test asserting the base list is the union — a final-reply-only target and a notification-only target both survive, a neither-capable target is excluded. Existing coincident-case tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
…irst-party notification channel (nearai#7398) * feat(web-push): browser push notifications + PWA — the web app as a first-party notification channel The web app becomes a real, selectable notification route for automations, at parity with Slack/Telegram: a bundled first-party channel extension (web-push) delivers W3C Web Push (RFC 8030/8291/8292) to the user's enrolled browsers through the existing catalog → notification-channel set → notifier → delivery coordinator → channel adapter → policy-enforced egress chain, with zero new routing machinery and the two-lane delivery contract untouched. The WebUI ships as an installable PWA (root-scope service worker with push + notification-click deep links; manifest and icons already existed), and the automations page's notification-channels panel replaces the always-on "web app" placeholder with a real toggleable row plus a per-browser enroll/disable flow. New crates: ironclaw_web_push (domain: subscription records + CAS store, RFC 8291 aes128gcm encryption pinned to the RFC's Appendix A vector, VAPID key-material generation, transport-free request planning, channel identity grammar, late-bound runtime slot) and ironclaw_web_push_extension (channel package: manifest with vapid_authorization egress injection, adapter with 404/410 pruning and honest Sent-without-ref evidence, personal-DM codec, owner-scoped catalog provider). One generic host addition: the RuntimeCredentialTarget::VapidAuthorization egress injection kind — the host signs the RFC 8292 ES256 JWT at the existing credential chokepoint with the audience derived from the request's own push-service origin; adapters never see key bytes. VAPID material is auto-generated and seeded at composition boot. Enrollment is an authenticated product surface (three new /api/webchat/v2/web-push routes) with descriptors declared in ironclaw_product_contracts::web_push per the transport/product boundary, and endpoints validate against the manifest-declared push-service hosts. Also fixes a boot bug the new integration tests exposed: DeploymentChannelBinding rejected outbound-only channels, which would have failed the runtime build at serve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-push): address review — redact VAPID secret, byte-budget payload, account-scoped enrollment, sanitized store errors Triage of the CodeRabbit + IronLoop review on nearai#7398: - Secret exposure (Critical): hand-written redacting `Debug` on `VapidCredentialMaterialV1` and `GeneratedVapidKeyMaterial` so the ES256 private key can never reach a log, panic, or `{:?}`. - VAPID material (High): composition now ensures-then-reads-back the canonical stored keypair so multi-replica cold-start converges on one signing key and the advertised applicationServerKey matches it; material shape is validated at boot and again before egress signing. - Payload budget (Medium): notification body is trimmed by serialized-JSON bytes, not character count, so multi-byte content can't blow the single-record push budget (+ regression test). - Error hygiene (Medium): `WebPushError::Store` is a fixed sanitized category; the backend cause is logged server-side, never rendered into the boundary error. - Account-scoped enrollment (Medium): status projects an `endpoint_digest` (SHA-256 hex) so a shared browser profile distinguishes "enrolled here" from "enrolled for another account" without the endpoint URL leaving the backend; the frontend correlates on it and only offers destructive disable when verified. - Deep-link/tag grammar enforced in the owning crate; SW validates same-origin before navigating; subject parsed via `url`; concurrent-writer CAS test; registry key derived from the extension-id constant; parse-cause preserved; doc/count corrections. The manifest keeps the `web_push_vapid` field (a channel egress credential handle must be declared in [admin_configuration]); it stays host-seeded and not operator-supplied, with the rotation caveat documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-push): keep SW registration out of the initial /chat bundle; raise the chat budget for the feature's i18n copy The merge-lane bundle-budget gate (skipped on the PR lane) flagged the initial /chat JS at 218.2 KB against the 217.0 KB gzip budget. `main.tsx` boot-imported `registerServiceWorker` from `lib/web-push.ts`, which dragged the enrollment lib's api-client and WebCrypto imports into the initial chunk. Extracted the dependency-free `lib/register-sw.ts` for boot; the enrollment API stays in `web-push.ts`, imported only by the already-lazy automations route. That recovered the eager-code weight (218.2 -> 217.4 KB); the residual is the feature's new `en.ts` fallback-pack strings, so the /chat budget is raised 217.0 -> 218.0 KB with rationale, matching how prior features (hosted MCP, Router 8) handled eager localized copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(web-push): register the browser channel in the journey coverage gate The merge-lane product-surface evidence gate (test_journey_coverage.py, skipped on the PR lane) requires every outbound channel manifest to name exact journey evidence. web-push declares `outbound = true`, so it entered the required delivery-target set with none. Added: - `JourneyDeliveryTarget.WEB_PUSH` and a `ProductJourneyCase` citing the existing `blocked_fire_pushes_web_push_notice_to_enrolled_browser` integration test, with unthreaded (`thread_anchor=None`) delivery-address evidence — browser push addresses a per-browser endpoint capability URL, not a conversation thread. - `assert_web_push_delivery_evidence` in delivery_user_journeys.rs, shaped as the gate's citability check requires (literal `expected_conversation_id` gating the count, `expected_thread_anchor = None`), called from that test. Verified: test_journey_coverage.py (68), test_product_surface_coverage.py + test_provider_capability_inventory.py (30), and the cited integration test all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(web-push): rustfmt the journey-evidence helper `cargo fmt --all -- --check` (the first step of the Fast deterministic checks lane, PR-lane-skipped) flagged the iterator chain in `assert_web_push_delivery_evidence`. Formatting only, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web-push): present the channel as "Web UI" and hide it from the install catalog The web-app's browser-push channel is host infrastructure, not a browse-and-install integration, so: - Rename the extension/channel to "Web UI" (manifest name + channel display_name + first-party bundle label; description reworded). - Hide it from the install catalog (/extensions, /extensions/registry, and the Settings channels list) via an explicit built-in-host-surface classification in the product lifecycle projection — deliberately keyed by id, not inferred from channel direction, so it stays correct as the web-app channel later gains inbound/outbound. Its outbound notification target (/outbound/targets) is a separate registry and is unaffected, so it remains a selectable notification channel. Also finalizes the enrollment UI carried from this session: - Align the "This browser" device block with the channel cards (drop the stray left indent). - Lazy-mount the web-push device hook so its status query fires only when a web-push row is present, not on every automations view. - Rename the misleading webOnlyHelper i18n key to noSelectionHelper across all locale packs (the string already dropped the retired "stays in the web app" claim). - Manifest icons declare purpose "any maskable" so Chrome offers the PWA install prompt. Regression coverage extends the production web-push integration test to assert it is absent from the install endpoints yet present in /outbound/targets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(web-push): fix stale helper name in install-catalog filter comment CodeRabbit flagged that the filter comment still named the removed `is_host_managed_channel` helper and described the old outbound-only heuristic. Point it at `is_builtin_host_surface` and the id-based classification the code actually uses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(web-push): pin notification-helper i18n key as noSelectionHelper The P1 rename webOnlyHelper -> noSelectionHelper missed two test assertions that pin the i18n key, failing the affected-3 Reborn crate bucket (Tests (Reborn) rolls it up): - crates/app/ironclaw_composition/tests/webui_v2_serve.rs (served bundle) - crates/product/ironclaw_webui/src/webui_v2/static_assets/assets.rs (live source) Also renames the now-misleading test fn and corrects a stale bundle-budget comment. Test-only; no production behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(channels): notifications as a first-class channel capability Separates "notification target" from "final-reply / model-delivery target" so the web app is a notification channel (blocked-automation notices) without being a place the model or a run's final reply is delivered to. Folds the notifications-capability work into the web-push PR (nearai#7398). Model / declarative: - ChannelDescriptor gains a `notifications` capability (serde default), so a manifest can independently declare inbound / outbound / notifications. The web-push manifest declares `notifications = true` (kept `outbound = true` for now: it is what registers the channel's delivery binding; outbound thread-creation is a later capability). Pinned by manifest_lockstep. Delivery authority (the load-bearing separation): - DeliveryTargetCapabilities gains a `notifications` bool, and a new `OutboundDeliveryTargetProvider::resolve_notification_target` resolves a target the caller may receive blocked-automation notices on, gated on `notifications` (not `final_replies`). Default impl + registry/mutable-registry overrides; the generic channel provider reuses its id-resolution. - web-push target caps flip to `final_replies: false, notifications: true`: browser push is where a run's reply already lands, never a destination the model/final-reply path delivers *to*. - The notification-channel picker (`/outbound/targets`) and the notifier + set-validation resolution now gate on the notifications capability; the model-facing delivery list (`builtin.outbound_delivery_targets_list`) narrows to `final_replies`, so a notification-only target (browser push) is invisible to the model until it gains outbound delivery. Why this shape: production blocked-automation notices run through TriggeredRunDeliveryDriver -> DeliveryCoordinator -> WebPushChannelAdapter and select targets by capability resolution; the `ThreadNotificationPolicy` / `progress` push-plan in ironclaw_outbound is dormant (no production writer or caller), so gating on `progress` would not have changed delivery. Slack and Telegram are both final-reply and notification targets and are unaffected. Regression coverage: web-push resolves as a notification target but NOT as a final-reply/model target (targets.rs); existing notifier + notification-channel integration journeys stay green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(channels): cargo fmt + drop vendor names from generic-code comments - cargo fmt reflow in notification_channel_resolution.rs. - Reword comments in ironclaw_composition/runtime.rs and ironclaw_extension_host/channel_outbound_targets.rs so generic code does not name Slack/Telegram (reborn_extension_specificity gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs+test(channels): fix notifications doc + serde-default coverage Addresses CodeRabbit on the notifications-capability commit: - channel.rs: reword ChannelDescriptor.notifications doc so it no longer claims the web app declines outbound (the manifest keeps outbound = true for the delivery binding). Net-zero line count (contracts size ceiling unchanged). - delivery_resolution.rs: assert notifications defaults false, and add a legacy-payload (omitted notifications) deserialize test for #[serde(default)]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(outbound): decouple notification-picker list from model-delivery list Addresses the CodeRabbit "Major": final_replies and notifications were coupled through one shared base filter. Now RebornOutboundDeliveryTargetCapabilities carries a wire-level `notifications`; list_outbound_delivery_targets returns the union (final_replies || notifications); the notification picker (build_outbound_delivery_targets_view) filters notifications and the model list (list_outbound_delivery_targets_for_model) filters final_replies — independently. A final-reply-only target is now visible to the model but not the picker; a notification-only target (web-push) shows in the picker but never to the model. Verified: assistant, ironclaw_webui, frontend (1208), delivery_user_journeys (25), web-push round-trip, architecture suite (41). Chose "in nearai#7398" per the overnight directive to fix review comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(outbound): cover divergent notifications/final_replies at the filter seam Addresses CodeRabbit's follow-up: the unit capability-filter seam couldn't construct divergent targets (the fixture set notifications = final_replies). Adds `target_entry_with_caps` (independent notifications) and a divergent-combo test asserting the base list is the union — a final-reply-only target and a notification-only target both survive, a neither-capable target is excluded. Existing coincident-case tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…irst-party notification channel (nearai#7398) * feat(web-push): browser push notifications + PWA — the web app as a first-party notification channel The web app becomes a real, selectable notification route for automations, at parity with Slack/Telegram: a bundled first-party channel extension (web-push) delivers W3C Web Push (RFC 8030/8291/8292) to the user's enrolled browsers through the existing catalog → notification-channel set → notifier → delivery coordinator → channel adapter → policy-enforced egress chain, with zero new routing machinery and the two-lane delivery contract untouched. The WebUI ships as an installable PWA (root-scope service worker with push + notification-click deep links; manifest and icons already existed), and the automations page's notification-channels panel replaces the always-on "web app" placeholder with a real toggleable row plus a per-browser enroll/disable flow. New crates: ironclaw_web_push (domain: subscription records + CAS store, RFC 8291 aes128gcm encryption pinned to the RFC's Appendix A vector, VAPID key-material generation, transport-free request planning, channel identity grammar, late-bound runtime slot) and ironclaw_web_push_extension (channel package: manifest with vapid_authorization egress injection, adapter with 404/410 pruning and honest Sent-without-ref evidence, personal-DM codec, owner-scoped catalog provider). One generic host addition: the RuntimeCredentialTarget::VapidAuthorization egress injection kind — the host signs the RFC 8292 ES256 JWT at the existing credential chokepoint with the audience derived from the request's own push-service origin; adapters never see key bytes. VAPID material is auto-generated and seeded at composition boot. Enrollment is an authenticated product surface (three new /api/webchat/v2/web-push routes) with descriptors declared in ironclaw_product_contracts::web_push per the transport/product boundary, and endpoints validate against the manifest-declared push-service hosts. Also fixes a boot bug the new integration tests exposed: DeploymentChannelBinding rejected outbound-only channels, which would have failed the runtime build at serve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-push): address review — redact VAPID secret, byte-budget payload, account-scoped enrollment, sanitized store errors Triage of the CodeRabbit + IronLoop review on nearai#7398: - Secret exposure (Critical): hand-written redacting `Debug` on `VapidCredentialMaterialV1` and `GeneratedVapidKeyMaterial` so the ES256 private key can never reach a log, panic, or `{:?}`. - VAPID material (High): composition now ensures-then-reads-back the canonical stored keypair so multi-replica cold-start converges on one signing key and the advertised applicationServerKey matches it; material shape is validated at boot and again before egress signing. - Payload budget (Medium): notification body is trimmed by serialized-JSON bytes, not character count, so multi-byte content can't blow the single-record push budget (+ regression test). - Error hygiene (Medium): `WebPushError::Store` is a fixed sanitized category; the backend cause is logged server-side, never rendered into the boundary error. - Account-scoped enrollment (Medium): status projects an `endpoint_digest` (SHA-256 hex) so a shared browser profile distinguishes "enrolled here" from "enrolled for another account" without the endpoint URL leaving the backend; the frontend correlates on it and only offers destructive disable when verified. - Deep-link/tag grammar enforced in the owning crate; SW validates same-origin before navigating; subject parsed via `url`; concurrent-writer CAS test; registry key derived from the extension-id constant; parse-cause preserved; doc/count corrections. The manifest keeps the `web_push_vapid` field (a channel egress credential handle must be declared in [admin_configuration]); it stays host-seeded and not operator-supplied, with the rotation caveat documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-push): keep SW registration out of the initial /chat bundle; raise the chat budget for the feature's i18n copy The merge-lane bundle-budget gate (skipped on the PR lane) flagged the initial /chat JS at 218.2 KB against the 217.0 KB gzip budget. `main.tsx` boot-imported `registerServiceWorker` from `lib/web-push.ts`, which dragged the enrollment lib's api-client and WebCrypto imports into the initial chunk. Extracted the dependency-free `lib/register-sw.ts` for boot; the enrollment API stays in `web-push.ts`, imported only by the already-lazy automations route. That recovered the eager-code weight (218.2 -> 217.4 KB); the residual is the feature's new `en.ts` fallback-pack strings, so the /chat budget is raised 217.0 -> 218.0 KB with rationale, matching how prior features (hosted MCP, Router 8) handled eager localized copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(web-push): register the browser channel in the journey coverage gate The merge-lane product-surface evidence gate (test_journey_coverage.py, skipped on the PR lane) requires every outbound channel manifest to name exact journey evidence. web-push declares `outbound = true`, so it entered the required delivery-target set with none. Added: - `JourneyDeliveryTarget.WEB_PUSH` and a `ProductJourneyCase` citing the existing `blocked_fire_pushes_web_push_notice_to_enrolled_browser` integration test, with unthreaded (`thread_anchor=None`) delivery-address evidence — browser push addresses a per-browser endpoint capability URL, not a conversation thread. - `assert_web_push_delivery_evidence` in delivery_user_journeys.rs, shaped as the gate's citability check requires (literal `expected_conversation_id` gating the count, `expected_thread_anchor = None`), called from that test. Verified: test_journey_coverage.py (68), test_product_surface_coverage.py + test_provider_capability_inventory.py (30), and the cited integration test all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(web-push): rustfmt the journey-evidence helper `cargo fmt --all -- --check` (the first step of the Fast deterministic checks lane, PR-lane-skipped) flagged the iterator chain in `assert_web_push_delivery_evidence`. Formatting only, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(web-push): present the channel as "Web UI" and hide it from the install catalog The web-app's browser-push channel is host infrastructure, not a browse-and-install integration, so: - Rename the extension/channel to "Web UI" (manifest name + channel display_name + first-party bundle label; description reworded). - Hide it from the install catalog (/extensions, /extensions/registry, and the Settings channels list) via an explicit built-in-host-surface classification in the product lifecycle projection — deliberately keyed by id, not inferred from channel direction, so it stays correct as the web-app channel later gains inbound/outbound. Its outbound notification target (/outbound/targets) is a separate registry and is unaffected, so it remains a selectable notification channel. Also finalizes the enrollment UI carried from this session: - Align the "This browser" device block with the channel cards (drop the stray left indent). - Lazy-mount the web-push device hook so its status query fires only when a web-push row is present, not on every automations view. - Rename the misleading webOnlyHelper i18n key to noSelectionHelper across all locale packs (the string already dropped the retired "stays in the web app" claim). - Manifest icons declare purpose "any maskable" so Chrome offers the PWA install prompt. Regression coverage extends the production web-push integration test to assert it is absent from the install endpoints yet present in /outbound/targets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(web-push): fix stale helper name in install-catalog filter comment CodeRabbit flagged that the filter comment still named the removed `is_host_managed_channel` helper and described the old outbound-only heuristic. Point it at `is_builtin_host_surface` and the id-based classification the code actually uses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(web-push): pin notification-helper i18n key as noSelectionHelper The P1 rename webOnlyHelper -> noSelectionHelper missed two test assertions that pin the i18n key, failing the affected-3 Reborn crate bucket (Tests (Reborn) rolls it up): - crates/app/ironclaw_composition/tests/webui_v2_serve.rs (served bundle) - crates/product/ironclaw_webui/src/webui_v2/static_assets/assets.rs (live source) Also renames the now-misleading test fn and corrects a stale bundle-budget comment. Test-only; no production behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(channels): notifications as a first-class channel capability Separates "notification target" from "final-reply / model-delivery target" so the web app is a notification channel (blocked-automation notices) without being a place the model or a run's final reply is delivered to. Folds the notifications-capability work into the web-push PR (nearai#7398). Model / declarative: - ChannelDescriptor gains a `notifications` capability (serde default), so a manifest can independently declare inbound / outbound / notifications. The web-push manifest declares `notifications = true` (kept `outbound = true` for now: it is what registers the channel's delivery binding; outbound thread-creation is a later capability). Pinned by manifest_lockstep. Delivery authority (the load-bearing separation): - DeliveryTargetCapabilities gains a `notifications` bool, and a new `OutboundDeliveryTargetProvider::resolve_notification_target` resolves a target the caller may receive blocked-automation notices on, gated on `notifications` (not `final_replies`). Default impl + registry/mutable-registry overrides; the generic channel provider reuses its id-resolution. - web-push target caps flip to `final_replies: false, notifications: true`: browser push is where a run's reply already lands, never a destination the model/final-reply path delivers *to*. - The notification-channel picker (`/outbound/targets`) and the notifier + set-validation resolution now gate on the notifications capability; the model-facing delivery list (`builtin.outbound_delivery_targets_list`) narrows to `final_replies`, so a notification-only target (browser push) is invisible to the model until it gains outbound delivery. Why this shape: production blocked-automation notices run through TriggeredRunDeliveryDriver -> DeliveryCoordinator -> WebPushChannelAdapter and select targets by capability resolution; the `ThreadNotificationPolicy` / `progress` push-plan in ironclaw_outbound is dormant (no production writer or caller), so gating on `progress` would not have changed delivery. Slack and Telegram are both final-reply and notification targets and are unaffected. Regression coverage: web-push resolves as a notification target but NOT as a final-reply/model target (targets.rs); existing notifier + notification-channel integration journeys stay green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(channels): cargo fmt + drop vendor names from generic-code comments - cargo fmt reflow in notification_channel_resolution.rs. - Reword comments in ironclaw_composition/runtime.rs and ironclaw_extension_host/channel_outbound_targets.rs so generic code does not name Slack/Telegram (reborn_extension_specificity gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs+test(channels): fix notifications doc + serde-default coverage Addresses CodeRabbit on the notifications-capability commit: - channel.rs: reword ChannelDescriptor.notifications doc so it no longer claims the web app declines outbound (the manifest keeps outbound = true for the delivery binding). Net-zero line count (contracts size ceiling unchanged). - delivery_resolution.rs: assert notifications defaults false, and add a legacy-payload (omitted notifications) deserialize test for #[serde(default)]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(outbound): decouple notification-picker list from model-delivery list Addresses the CodeRabbit "Major": final_replies and notifications were coupled through one shared base filter. Now RebornOutboundDeliveryTargetCapabilities carries a wire-level `notifications`; list_outbound_delivery_targets returns the union (final_replies || notifications); the notification picker (build_outbound_delivery_targets_view) filters notifications and the model list (list_outbound_delivery_targets_for_model) filters final_replies — independently. A final-reply-only target is now visible to the model but not the picker; a notification-only target (web-push) shows in the picker but never to the model. Verified: assistant, ironclaw_webui, frontend (1208), delivery_user_journeys (25), web-push round-trip, architecture suite (41). Chose "in nearai#7398" per the overnight directive to fix review comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(outbound): cover divergent notifications/final_replies at the filter seam Addresses CodeRabbit's follow-up: the unit capability-filter seam couldn't construct divergent targets (the fixture set notifications = final_replies). Adds `target_entry_with_caps` (independent notifications) and a divergent-combo test asserting the base list is the union — a final-reply-only target and a notification-only target both survive, a neither-capable target is excluded. Existing coincident-case tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
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
web-pushchannel extension delivers W3C Web Push (RFC 8030 transport, RFC 8291aes128gcmencryption, RFC 8292 VAPID) to the user's enrolled browsers, riding the existing two-lane delivery machinery unchanged (catalog target +ChannelAdapter+ delivery coordinator + notification-channel set)./sw.js, push + notification-click deep links; the manifest and maskable icons already shipped), boot-time registration, and a per-browser enroll/disable flow on the automations page's notification-channels panel — the old "Notifications stay in the web app" always-on placeholder is replaced by a real, toggleable Web app row.ironclaw_web_pushdomain crate owns subscription records (CAS filesystem store, per-user cap), RFC 8291 encryption (pinned to the RFC's Appendix A vector), VAPID key-material generation, and transport-free push request planning — zero new heavy dependencies (aws-lc-rswas already in the lockfile).vapid_authorization: the host computesAuthorization: vapid t=…,k=…at the existing credential chokepoint, with the JWT audience derived from the request's own push-service origin; adapters never see key bytes. VAPID material is auto-generated and seeded at composition boot (no operator input, never in config files).GET /api/webchat/v2/web-push/status,POST …/web-push/subscriptions,POST …/web-push/subscriptions/remove, validated against the manifest-declared push-service hosts (one source of truth with the egress allowlist).Change Type
Linked Issue
None (feature requested directly; see
docs/internal/design/2026-08-08-web-push-notifications.mdin this PR for the full decision record).Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings(plus the default-features leg:cargo clippy --all --tests --examples -- -D warnings)cargo build(via check/clippy/test across the workspace)ironclaw_web_push(22),ironclaw_web_push_extension(+ manifest lockstep),ironclaw_host_runtime(full, incl.egress::vapid),ironclaw_assistant(full +reborn_services_module_charter),ironclaw_webui(full + descriptor/handlers contracts),ironclaw_extension_host(391, incl. the outbound-only deployment-binding regression),ironclaw_composition(full, incl.webui_v2_serve66),ironclaw(CLI),ironclaw_sandbox,ironclaw_product_contracts,ironclaw_extension_support,ironclaw_architecture_tests(full),reborn_integration_webui_v2_product_api(33),reborn_integration_delivery_user_journeys(25),reborn_integration_outbound_target(20); frontendpnpm test(1198 tests) +vite buildcargo test -p <owning-crate> --features integration— Not applicable: the subscription store rides the shared scoped-filesystem plane; no crate-level DB integration feature was touchedscripts/ci/check-target-tree.py(66/66),scripts/ci/check-composition-budget.sh,scripts/ci/docs_publication_boundary.pyreview-pr/pr-shepherd— not run (unavailable in this session)Layer-by-layer map (every crate in the diff)
New crates (2):
crates/domains/ironclaw_web_push(substrates):PushSubscriptionRecord/PushEndpoint/keys grammar;WebPushSubscriptionStore+ scoped-filesystem CAS impl (one JSON doc per tenant/user, 16-browser cap); RFC 8291 encryption (aws-lc-rs; Appendix A vector test incl. exact ECDH/CEK/nonce intermediates + receiver-side round trip); VAPID key-material generation; push request planner; channel identity grammar (web-push/v1/<tenant>/<user>binding refs, constant owner-scopedweb-pushtarget id); late-boundWebPushRuntimeSlot.crates/extensions/packages/web-push(ironclaw_web_push_extension, products):reborn.extension_manifest.v3manifest — outbound-only[channel],[[channel.egress]]for the three push-service hosts withvapid_authorizationinjection, auto-seeded[admin_configuration]secret;WebPushChannelAdapter(renders envelope parts into one coalesced notification, encrypts per enrolled browser, POSTs via restricted egress, prunes 404/410 subscriptions, reportsSent { vendor_message_ref: None }— push services acknowledge acceptance without a readable message ref, and the report never claims more);WebPushPreferenceTargetCodec(every target is a personal DM surface, admitting auth prompts);WebPushOutboundTargetProvider(constant per-user "Web app" catalog entry).Contracts:
ironclaw_host_api:RuntimeCredentialTarget::VapidAuthorization+VapidCredentialMaterialV1schema (ceiling 18 799 → 18 832, declarations only).ironclaw_extension_contracts: manifest-schema validation arm for the new injection kind (7 748 → 7 752).ironclaw_product_contracts:RebornWebPush*wire DTO family (15 800 → 15 840).Kernel:
ironclaw_host_runtime:egress/vapid.rs— RFC 8292 ES256 signing at the credential-injection chokepoint (HTTPS-gated like every injection; audience = the request's own origin); unit tests verify the JWT against the advertised public key and fail-closed paths.ironclaw_sandbox: the new enum variant joins the sandbox plan's rejected credential-target set.TRIGGER_CREATE_DESCRIPTION+ the trigger prompt-field schema no longer claim "no web-app target exists"; they now steer explicit browser-notification asks to the catalog's browser-push target (pinned phrases preserved).Product:
ironclaw_assistant:WebPushProductService(trait + fail-closed default + production impl over the store; enrollment validates endpoints against the manifest-derived host allowlist),WEB_PUSH_STATUS_VIEW,web_push.subscribe/web_push.unsubscribeproduct commands (same webui-only, no-approval-gate class asoutbound.notification_channels_set); gate-pinnedreborn_servicescharter map gains aweb-pushrow.ironclaw_webui: three route descriptors + handlers (under theoutboundcharter owner), CONTRACT.md route rows,worker-srcunchanged (shell CSP already admits same-origin workers viadefault-src 'self').public/sw.js(push + notificationclick; deliberately no fetch/caching handler), boot-time SW registration,lib/web-push.ts(browser state machine, enroll/unenroll, VAPID key handling), api client functions, notification-channels panel device block (unsupported / permission-denied / not-enrolled / enrolled states, enrolled-browser count), reworded empty-set helper, new i18n keys in all 11 locale packs.App:
ironclaw_cli: third channel binding (adapter + codec + target provider) built around the runtime slot; the web-push manifest bundle ships from the binary's bundle table (crate-localinclude_str!— the §11.2.7 cross-crate include inventory stays at 16); VAPIDsubderived fromIRONCLAW_REBORN_WEBUI_BASE_URLwhen https.ironclaw_composition: generic binding-borne outbound-target-provider registration; deployment-channel filter widened to admit outbound-only channels (delivery resolution needs the adapter without an installation record);assemble_web_push(store construction on the scoped-filesystem plane, slot install, VAPID seeding/read-back through the secret store at the channel-egress scope, manifest-derived push-host extraction);/web-pushper-user mount alias;RebornWebPushProductServicewiring.ironclaw_architecture_tests: CLI exact-dependency set + contracts ceilings raised with rationale (above);web-pushadded toNON_VENDOR_PROVIDER_PACKAGE_DIRS(it is the IETF protocol name and first-party deployment infrastructure — the same class as the provider-neutral memory packages; generic code does not hardcode push hosts, which are read from the resolved manifest).composition-budget.toml:loc_ceiling40 747 → 41 035 (assembly-only growth, rationale in the file).Docs (dated corrections):
communication-delivery-resolution.md§5 (empty set = no external route;web-pushis a real target, not the retired pseudo-target),triggers.md§9,extension-runtime/overview.md§5.4,FEATURE_PARITY.md, PROPOSAL §5 tree (+2 crates), family/crate guidance tables, new design recorddocs/internal/design/2026-08-08-web-push-notifications.md.Two-lane contract: unchanged
Completed fires still deliver nothing from the host notifier (lane 1 owns the result). Selecting Web app in the notification-channels set delivers gate/auth/failure notices as pushes; "push me the result" routines pin the
web-pushcatalog target and the model delivers viabuiltin.outbound_deliver(lane 2), exactly like Slack/Telegram.Compatibility, rollback, follow-ups
/web-push/subscriptions.jsondocuments; one VAPID secret under the channel-egress scope). The retiredbuiltin:web_appid remains non-addressable (existing pin untouched). Existing notification-channel sets are unaffected until a user selects the new row. Wire additions are new routes/DTOs only.Authorizationheaders); egress rides the existing declared-host, policy-enforced channel path; payloads are encrypted end-to-end per RFC 8291.Sentwith no vendor ref. Deep links currently open/automations; per-thread links are a polish follow-up.Test Strategy
User behavior: A user opens the automations page, sees "Web app" as a notification channel beside Slack/Telegram, enables notifications in their browser (permission prompt → enrollment), selects any combination of channels (or none), and receives OS-level browser push notifications for automation approval/auth/failure notices — and automation results when a routine says to notify the browser — including with the app closed. The app installs as a PWA.
Risk areas:
Tests added or updated:
ironclaw_web_push(22 — RFC 8291 Appendix A vector with exact intermediates + receiver round trip, VAPID material round trip, endpoint/keys validation, CAS store contract incl. scope isolation + cap, runtime slot);ironclaw_host_runtimeegress::vapid(JWT verifies against advertised key, aud/exp/sub claims, fail-closed material/audience);ironclaw_web_push_extension(codec DM semantics, provider owner-scoping, manifest lockstep pins);ironclaw_webuidescriptors contract (3 new routes with exact policies), handlers charter, assistantreborn_servicescharter.webui_v2_product_api::web_push_enrollment_and_notification_channel_round_trip_through_production_facade— fullbuild_reborn_runtimecomposition (VAPID auto-seeded, slot installed), real HTTP routes: status → catalog row → enroll → refresh → endpoint redaction (full URL absent from every response) → undeclared-host 400 →set_notification_channels(["web-push"])reads backavailable→ remove.delivery_user_journeys::blocked_fire_pushes_web_push_notice_to_enrolled_browser— gated triggered fire through the real notifier/coordinator/adapter/policy-enforced egress produces exactly one POST to the enrolled endpoint with host-injectedauthorization: vapid t=…, k=…,content-encoding: aes128gcm,ttl, and a well-formed RFC 8188 body (body[20] == 65), plus a durable delivery attempt, with the run still parked and approvable.…::gone_push_subscription_is_pruned_after_notice_attempt— a 410 endpoint is attempted once then pruned. Harness gainedRebornIntegrationGroup::extension_delivery_with_web_push()mirroring the binary's binding table.DeploymentChannelBinding::newrejected any channel without inbound ingress, so an outbound-only channel failed the entire runtime build — the shipping binary would have failed identically atserveboot. Fixed inironclaw_extension_host::deployment_channels(inbound still requires ingress; outbound-only binds with nothing to mount; direction-less channels still rejected) with a unit regression (outbound_only_channel_binds_without_ingress_and_never_resolves_ingress).tool_surface_contract.rssubstring tests, which still pass.What the tests prove: the encryption is byte-correct against the RFC vector; the VAPID header is a verifiable ES256 JWT scoped to the push origin; enrollment round-trips through the real HTTP routes with endpoint redaction and allowlist rejection; a gated triggered fire fans a notice out as an encrypted, VAPID-authorized POST to the enrolled endpoint through the production notifier → coordinator → adapter → policy-enforced egress chain; dead subscriptions prune on 410; the catalog/panel wire exposes and persists the web-push channel like any other; the retired
builtin:web_appid stays dead.Commands run:
Update — folded in: "Web UI" presentation + notifications as a first-class channel capability
Two follow-on chunks were folded into this PR (keeping the stack at 4 PRs).
1. Present the channel as "Web UI" and hide it from the install catalog
display_name+ first-party bundle label)./extensions,/extensions/registry, Settings channels) via an explicit built-in-host-surface classification in the product lifecycle projection — keyed by id (is_builtin_host_surface), not inferred from channel direction, so it stays correct as the web-app channel later gains inbound/outbound. Its outbound notification target (/outbound/targets) is a separate registry and remains a selectable notification channel.webOnlyHelperi18n key tonoSelectionHelperacross all locales, and declare PWA iconspurpose: "any maskable".2. Notifications as a first-class channel capability (the "PR-A" separation)
Separates notification target from final-reply / model-delivery target, so the web app is a notification channel without being a place the model or a run's final reply is delivered to.
ChannelDescriptorgains anotificationscapability (serde default) — a manifest can now independently declare inbound / outbound / notifications. The web-push manifest declaresnotifications = true(keepsoutbound = true, which registers the delivery binding; outbound thread-creation is a later capability).DeliveryTargetCapabilities.notifications+OutboundDeliveryTargetProvider::resolve_notification_target(default + registry/mutable-registry + generic-channel impls), gated onnotifications.final_replies: false, notifications: true./outbound/targets), the notifier resolution, and set-validation gate on the notifications capability; the model's delivery list (builtin.outbound_delivery_targets_list) narrows tofinal_replies— so a notification-only target (browser push) is invisible to the model until it gains outbound delivery.progresspush-plan: production blocked-automation notices run throughTriggeredRunDeliveryDriver → DeliveryCoordinator → WebPushChannelAdapterand select targets by capability resolution; theThreadNotificationPolicy/progresspush-plan inironclaw_outboundis dormant (no production writer or caller), so gating onprogresswould not have changed delivery. Slack and Telegram are both final-reply and notification targets and are unaffected.Added test evidence: web-push resolves as a notification target but not as a final-reply/model target (
web-push/src/targets.rs);manifest_locksteppinsnotifications = true; the contracts size-ceiling ratchet was bumped7_752 → 7_758with rationale. All delivery user-journeys (incl.blocked_fire_pushes_web_push_notice_to_enrolled_browser) and the web-push enrollment round-trip stay green.🤖 Generated with Claude Code