Skip to content

fix(ci): scope the /benchmark suite pre-check to BENCHMARKS_DISPATCH_TOKEN - #6588

Merged
pranavraja99 merged 2 commits into
mainfrom
fix-benchmark-canary-suite-precheck-token
Jul 23, 2026
Merged

pranavraja99 merged 2 commits into
mainfrom
fix-benchmark-canary-suite-precheck-token

Conversation

@pranavraja99

Copy link
Copy Markdown
Contributor

Follow-up to #6587 (also fix(ci) — bypassing the regression-test bot's fix-PR heuristic the same way, label applied below).

Live-testing #6587 on the dummy canary PR (#6582) got past the fixed dispatch mechanism — real jobs now run, confirming the workflow_dispatch approach works — but failed one step later: the parse job's pre-dispatch suite-existence check reads nearai/benchmarks' contents via the ambient GITHUB_TOKEN (scoped to this repo only), which 404s now that benchmarks is private.

Fixes it the same way as #6587: route just those two gh api repos/nearai/benchmarks/... calls through BENCHMARKS_DISPATCH_TOKEN via a per-command override, leaving the rest of the step (gh pr view/comment on this repo) on the ambient token.

Needs one more thing before this works: the existing BENCHMARKS_DISPATCH_TOKEN fine-grained PAT needs Contents: Read-only added on nearai/benchmarks (alongside its existing Actions: Read/write). Fine-grained PAT permissions can be edited in place — no need to regenerate the token or touch the stored secret.

…stence check

The parse job's pre-dispatch check ('does suites/<name>.toml exist on
nearai/benchmarks main?') reads that repo's contents via the ambient
GITHUB_TOKEN, which is scoped to nearai/ironclaw only. Now that
nearai/benchmarks is private, that call 404s (confirmed live: PR #6582's
/benchmark comment got past the fixed dispatch mechanism from #6587 but
failed here instead). Scopes the two affected gh api calls to
BENCHMARKS_DISPATCH_TOKEN via a per-command GH_TOKEN override, leaving
the rest of the step (gh pr view/comment on this repo) on the ambient
token as before.

Requires the existing BENCHMARKS_DISPATCH_TOKEN fine-grained PAT to also
carry Contents: Read-only on nearai/benchmarks (in addition to its
existing Actions: Read and write) — editable in place on the same token,
no secret value change needed.
Copilot AI review requested due to automatic review settings July 23, 2026 17:38
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@ironloopai

ironloopai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

🔎 IronLoop Review Status

Head: 1d334bd39f6855fbd108b76fb7084104f4852586
Result: One or more review results were superseded by a newer PR head.
Next: Run @ironloopai review on the latest PR head.
Updated: 2026-07-23T17:43:24.487Z

Current reviewers:

Reviewer State Verdict Findings Last update
ironloop/common-reviewer (reviewer) Superseded N/A N/A 2026-07-23T17:43:24.474Z
Reviewer summaries
Reviewer Detail
ironloop/common-reviewer (reviewer) Superseded by a newer PR head. New head: 1d334bd. Previous verdict: Needs validation.
Recent activity
Time Reviewer State Detail
2026-07-23T17:38:55.225Z ironloop/common-reviewer (reviewer) Queued Accepted review request for head fe7100c.
2026-07-23T17:38:55.225Z ironloop/common-reviewer (reviewer) Queued Waiting for this reviewer lane to become available.
2026-07-23T17:38:55.850Z ironloop/common-reviewer (reviewer) Started Reviewer worker started.
2026-07-23T17:38:59.154Z ironloop/common-reviewer (reviewer) Workspace ready Prepared isolated checkout (merge_ref) at 409c049.
2026-07-23T17:41:37.873Z ironloop/common-reviewer (reviewer) Result captured Needs validation; 0 blocking findings.
2026-07-23T17:41:37.873Z ironloop/common-reviewer (reviewer) Completed Review completed and terminal status was persisted.
2026-07-23T17:43:24.474Z ironloop/common-reviewer (reviewer) Superseded A newer PR head replaced this review (1d334bd).
Available commands
  • @ironloopai help
  • @ironloopai agents
  • @ironloopai review
  • @ironloopai review --agent <agent>
Run metadata

Admission: webhook accepted the request and IronLoop persisted reviewer state before this projection.

@pranavraja99 pranavraja99 added the skip-regression-check Bypass regression test CI gate (tests exist but not in tests/ dir) label Jul 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6588 July 23, 2026 17:39 Destroyed
@github-actions github-actions Bot added scope: ci CI/CD workflows size: S 10-49 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Jul 23, 2026
@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b5b7f78c-b7b9-4259-a560-892f89545f7c

📥 Commits

Reviewing files that changed from the base of the PR and between 62a4907 and fe7100c.

📒 Files selected for processing (1)
  • .github/workflows/nearai-bench.yml

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved benchmark command validation for private benchmark suites.
    • Ensured suite existence checks use the appropriate access credentials, reducing false “unknown suite” errors.

Walkthrough

The benchmark workflow now uses BENCHMARKS_DISPATCH_TOKEN for pre-dispatch suite existence checks against the private nearai/benchmarks repository.

Changes

Benchmark dispatch authentication

Layer / File(s) Summary
Private suite validation token
.github/workflows/nearai-bench.yml
The parse job defines BENCH_GH_TOKEN from secrets.BENCHMARKS_DISPATCH_TOKEN and applies it to the gh api suite existence check.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: copilot, ilblackdragon

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is prose-only and misses the repository's required template sections, validation, test strategy, and impact fields. Rewrite it to match the template: Summary, Change Type, Linked Issue, Validation, Test Strategy, Security Impact, Blast Radius, Rollback, and Review Follow-Through.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title uses Conventional Commits and accurately summarizes the CI token scope change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

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

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ IronLoop Review: reviewer

Review at a glance

Verdict Blocking Notes Inline Head
⚠️ Needs validation 0 0 0 fe7100c61d7f

Head: fe7100c61d7fe913f64be41633c5bc81576168fa
Next: Human review or validation is required before merging.

Run details

Status: Current
Needs human: no
Needs validation: yes

Summary

No in-repository correctness or security defect found. The two private-repository contents requests are correctly scoped to the dispatch PAT while same-repository PR operations retain the ambient token.

Findings

None.

Developer follow-up

After fixing this feedback:

  1. Push the fix to this PR branch.
  2. Re-run this reviewer with @ironloopai review --agent reviewer if you only changed this reviewer's findings.
  3. Re-run all reviewers with @ironloopai review when the fix may affect multiple areas.

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-6588 July 23, 2026 17:43 Destroyed
@railway-app

railway-app Bot commented Jul 23, 2026

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-6588 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jul 23, 2026 at 5:43 pm

@pranavraja99
pranavraja99 merged commit a41ec5f into main Jul 23, 2026
43 checks passed
@pranavraja99
pranavraja99 deleted the fix-benchmark-canary-suite-precheck-token branch July 23, 2026 19:47
BenKurrek added a commit that referenced this pull request Jul 23, 2026
…capability evidence + #6588 CI) into #6520

Doctrine: main's structure/vocabulary wins (ironclaw_product_workflow -> ironclaw_product
crate rename; product_adapters/product_adapter_registry folded into ironclaw_product +
ironclaw_host_api::product_adapter; WebUiAuthenticatedCaller -> ProductSurfaceCaller,
RebornServicesError -> ProductSurfaceError, *_OPERATION -> *_COMMAND, WebUi*Request ->
Product*Request; generic host_api ProductSurface {invoke,query,stream_events} with
BoundProductSurface + descriptor invoke_on/query_on idiom). This PR's lifecycle model
wins semantically: no Activate action/endpoint (EXTENSION_ACTIVATE_CAPABILITY exports
dropped, activate handler stays deleted), caller-private installs, required
client_action_id gesture idempotency on install (parse_client_action_id widened to pub +
re-exported; extension_lifecycle_activity_id restored in webui handlers;
InstallExtensionBody.client_action_id restored), install membership read-back ported to
the Bound idiom (install_extension_on_surface now takes &BoundProductSurface), event-driven
delivery (RunDeliverySettings/triggered_run_delivery_settings exports stay removed;
event router/current-target/trigger-router exports kept), channel-config deletion upheld
(ChannelConfigFacade/RebornChannelConfigField dropped from the renamed exports;
composition channel_config.rs stays deleted).

78 conflicted files resolved (mechanical rename hunks auto-resolved via base-aware
3-way identity; ~87 semantic double-edits resolved by hand). Test doubles retargeted to
the new traits (ChannelInboundProductSurface admission stubs; generic ProductSurface stub
impls). Telegram extension re-gained ironclaw_product/ironclaw_turns deps its
preference-target codec file needs. Stale specificity-allowlist slack row for the folded
host_api outbound.rs removed (gate green). Workspace + tests compile clean; fmt applied;
cargo test -p ironclaw_architecture green; ironclaw_product test suites green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV
BenKurrek added a commit that referenced this pull request Jul 23, 2026
) into the PR branch

Adopts the agent-resolved merge commit 461e348 (78 conflicted files:
ironclaw_product_workflow -> ironclaw_product crate rename, adapters folded
into ironclaw_product/host_api, ProductSurfaceCaller/ProductSurfaceError
vocabulary, generic ProductSurface trait in host_api) and repoints the
post-merge test fixes to the renamed crate. #6520 semantics preserved: no
Activate anywhere, required client_action_id install gesture, event-driven
delivery, channel-config deletion upheld.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV
BenKurrek added a commit that referenced this pull request Jul 24, 2026
…6520)

* fix(reborn): unify extension lifecycle and channel delivery

* Finish generic extension lifecycle and delivery correctness

* fix(reborn): repair integration + triggers test compilation

The generic extension/channel refactor renamed/removed several APIs but did
not propagate the changes into the shared integration test-support harness or
two new journey tests, so `cargo check --workspace --all-targets` failed with
101 errors (the whole `ironclaw_reborn_integration_tests` suite plus the
`ironclaw_triggers` lib-test), meaning none of the P0 journey tests could
build.

- Drop the removed `inbound_payload_classifier` / `classifier` fields from the
  test-support `ChannelExtensionBinding` and two `ChannelInboundSinkConfig`
  literals — gate-command classification now runs generically in
  `GenericChannelInboundSink`, so the per-binding classifier is obsolete.
- Remove the now-unused `RunDeliverySettings` and `ProductWorkflow` imports and
  point the stale `RebornServicesApi` import at the current `ProductSurface`
  trait that exposes `query`.
- Widen the `InMemoryTriggerRepository` history/lock helpers to `pub(crate)` so
  the relocated `src/tests.rs` module can reach them.
- `cargo fmt` (also fixes a pre-existing import-order drift in
  composition `outbound/mod.rs`).

No production behavior changes; this restores the deleted-then-relocated test
coverage so the suite compiles and can run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(mcp): skip shape-invalid tools instead of bricking the catalog

Hosted-MCP discovery rejected the entire advertised catalog on the first
shape-nonconforming tool (e.g. a non-lowercase name), so 23 valid tools +
1 bad tool made the whole integration unusable on first install with no
prior generation to fall back to.

parse_tools_list_result now distinguishes shape-only, non-security defects
(invalid tool name / description / annotations) from security/bounds
violations (missing or unsafe input schema, over-cap catalog). Shape-only
defects skip just the offending tool and publish the rest, emitting a
bounded debug record (tool index + stable cause token only). Security and
bounds violations still fail the whole generation with the unchanged stable
subcause; per-tool checks evaluate the schema first so a co-occurring
cosmetic defect cannot downgrade a security failure to a skip. A catalog
where every tool is shape-invalid still fails non-retryably (nothing to
publish), while an empty provider list stays an empty result.

Test-first regressions in mcp_adapter_contract.rs (survivors publish
end-to-end through the real client) and lib unit tests (skip+record,
security-amid-valid fails whole catalog, all-invalid fails, empty preserved).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(outbound): guard crash-recovery Sending->Unknown against clobbering a delivered send

`recover_interrupted_deliveries` listed `Sending` attempts from a point-in-time
snapshot and then blindly called `update_delivery_status(Unknown)` for each,
which unconditionally set the status. A concurrent worker that completed egress
and wrote `Delivered` could then be overwritten back to `Unknown` by a stale
recovery, durably losing a successful delivery (latent duplicate-send risk).

Add a dedicated `recover_interrupted_delivery_attempt` store method that
re-reads the attempt inside the same CAS the write commits against and
transitions `Sending -> Unknown` only when it is still `Sending` (mirroring the
`Prepared` guard on `claim_delivery_attempt_for_send`), returning `false`
otherwise. Route the coordinator's recovery through it so the guard, not the
list snapshot, decides. `update_delivery_status` stays an unconditional setter
for legitimate forward egress-result writes.

Regression: `recovery_transition_never_clobbers_delivered` in
`outbound_state_store_contract` drives record->claim(Sending)->Delivered, then
recovery, and asserts the row stays `Delivered` (and that a still-`Sending`
attempt is still recovered to `Unknown`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(extension-host): fence hosted-MCP discovery on stable credential authority

The discovery authority fence compared the whole `Vec<CredentialAccount>`
via the fence's derived `PartialEq`, which includes each account's volatile
`created_at`/`updated_at`. A benign write to the credential row between the
pre-discovery capture and the post-discovery recheck therefore tripped the
fence and forced a spurious `discovery_recheck` transient — a hard retry
loop if anything keeps touching the row mid-discovery.

`still_authorizes` now compares a stable, timestamp-free projection of each
account (id, status, access/refresh secret handles, scopes) alongside the
existing package, manifest digest, and tool ceiling. The fence stays
fail-closed on any real authority change (scope, secret, or status). The
fence's derived `PartialEq` is removed so no caller can reintroduce the
timestamp-sensitive comparison by accident. `CredentialAccount`'s own
derived `PartialEq` is deliberately left unchanged.

Test-first regressions through the real caller (`run_extension_activation`):
a benign `updated_at` bump activates without a spurious recheck; a scope,
secret, or status change still fails the fence.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(run-delivery): gate completed-run handoff materialization + freeze Final dedup cursor

Two durable-delivery correctness fixes in the lifecycle-event router.

Fix 1 (HIGH) — unsettled completed-run handoffs leak durably and are re-scanned
every drain. `materialize_completed_handoffs` wrote a `RunFinalReplyHandoffRecord`
for EVERY `Completed` event on the global bus, but a handoff is only settled by a
live SOURCE channel handler. Runs no such handler owns — pure WebUI→WebApp
completions (no adapter), ScheduledTrigger runs (served by the volatile triggered
driver), and context-less runs — never settled, so under normal WebUI chat usage
every completion leaked one permanent pending row, and every drain (fires on every
lifecycle publish, from cursor 0) re-read and re-fanned-out all N leaked rows.
Unbounded durable growth + O(N)-per-event work that can starve real inbound
delivery. Gate materialization: give the durable replay a `TurnStateStore` seam and
materialize a handoff only when the completed run actually needs channel delivery —
`Inbound` origin always (source route or external target; blocked-on-OAuth channel
runs still deliver), or `WebUi` origin only with a sealed `External` target. Skip
WebApp/None-destination WebUI answers, scheduled triggers, and context-less runs.
The classification is fail-open toward delivery: any run-state/target lookup error
materializes, so a real channel reply is never dropped. The cursor still advances
past skipped events, so they are never re-scanned.

Fix 2 (MED) — the Final delivery's durable at-most-once identity was derived from
the re-fetched live `state.event_cursor`, stable only because Completed is currently
terminal. A future post-Completed cursor bump would drift the `ProjectionUpdateRef`
→ a new `delivery_id` → the CAS dedup misses → double final send. Key the Final
projection epoch off the frozen `event.cursor` the drain already loaded and
validated, making at-most-once structural rather than incidental.

Tests (run_delivery_contract), red-then-green:
- completed_webui_webapp_run_does_not_leak_a_durable_handoff and
  completed_scheduled_trigger_run_does_not_leak_a_durable_handoff — assert
  list_pending_run_final_reply_handoffs stays empty and the cursor advances; both
  FAIL (leaked pending handoff) with the gate neutered.
- completed_final_delivery_dedups_across_a_post_completed_cursor_advance — replays a
  Completed event after a simulated later cursor advance and asserts one send; FAILS
  (double "exactly-once final") when the epoch keys off live state.
- Existing channel-delivery journeys (delayed-OAuth final, duplicate-events-deliver-
  once, cross-channel, crash-replay) unchanged; the paginated-drain test reworked to
  keep Inbound fillers (which still materialize) instead of context-less ones.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* style(product-workflow): clear pre-existing clippy 1.96 findings blocking -D warnings

The corrective PR's own commits left clippy 1.96 findings that fail the required
`cargo clippy -p ironclaw_product_workflow -p ironclaw_outbound --all-targets --
-D warnings` gate (unrelated to the delivery bug fixes; surfaced once the gate was
run). Cleared as drive-bys so the gate is green:

- lifecycle_auth_continuation.rs: collapse nested `if let { if }` into a let-chain
  (collapsible_if); let-chains are already used elsewhere in the workspace.
- reborn_services_contract.rs: drop `.clone()` on the `Copy` `ActivityId` (clone_on_copy).
- channel_pairing_contract.rs: allow type_complexity on a test-only field.

No behavior change; all touched test suites still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(reborn): route discovery-time provider 401/403 back to OAuth

Discovery credentials are checked present pre-discovery, but if the provider
rejects them mid-`tools/list` (token expired/revoked) the concrete MCP client
returns `McpClientError::AuthRequired`. The composition discovery classifier
folded every non-catalog error (via a catch-all) into `Transient`, so the
extension stayed `setup_needed` and was retried forever, re-hitting the same
401 and never escalating to re-OAuth.

`classify_discovery_error` now matches `AuthRequired` explicitly and maps it to
a new `HostedMcpDiscoveryError::ReAuthRequired`; genuinely transient failures
(timeouts, 5xx) stay `Transient` and an invalid catalog stays `Permanent`. The
activation transaction's discovery step returns a `HostedMcpDiscoveryOutcome`
so the composition impl can turn `ReAuthRequired` into the same
credentials-missing outcome the pre-discovery missing-credential path uses
(re-deriving the extension's declared requirements from the package). The user
is routed back through OAuth with credential blockers, nothing publishes
(no false activation), the staged discovery authority is still revoked, and
credentials are not discarded from the store.

Test-first regressions: classifier unit test (AuthRequired -> ReAuthRequired,
not Transient), transaction-level routing test (CredentialsRejected ->
CredentialsMissing, no false-active, authority revoked), and a composition
end-to-end test driving a real 401 mid-`tools/list` through the concrete
client (Ok re-auth response with credential blockers, capability unpublished)
-- both shown red against the old fold. The `..._when_credential_epoch_changes`
fixture now rotates a real authority input (access secret) rather than only a
timestamp, matching the corrected discovery-fence semantics.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(webui): correct chat gate/card affordances, prune dead auth+i18n paths

Lane F of PR #6520 review polish (all under ironclaw_webui/frontend, test-first):

- gates.ts/chat.tsx: derive the channel-connection (pairing) gate through one
  shared `channelConnectionFromGate` predicate so the composer affordance and
  the pairing-card selector can never disagree. A `manual_token` gate carrying
  a stray `connection` now stays token-paste in both places instead of the
  composer promising pairing. Pinned by a caller-level chat.test.ts regression
  (mutation-verified) + gates.test.ts unit coverage; backend invariant
  (auth_prompt.rs: connection only on challenge_kind==pairing) documented.

- auth-generic-card.tsx: delete the dead `gate?.remediation ||` branch — neither
  AuthPromptView nor AuthPromptContextView carries a remediation field, so
  gates.ts never populates one. Test now pins the neutral fallback is always used.

- onboarding-pairing-card.tsx + i18n: rename the stale "paste"/"code" fallbacks
  (pairing.openAndPaste -> pairing.connectInstructions, pairing.checkCodeAndRetry
  -> pairing.connectFailedRetry) to match the QR/deep-link/web-code flow, and
  drop orphaned keys (pairing.title/instructions/placeholder/approve/success/
  error/none/resumeFailed, pairing.web.copyUsername) consistently across all 11
  locales. New i18n.test.ts case pins retired-absent / renamed-present parity.

- configuration-tab.test.ts: cover the post-save reseed branch where `save`
  resolves WITH the saved group — reseeds non-secret values from the SAVED
  group, secrets stay blank (mutation-verified; matches production save shape).

pnpm test (849 pass), pnpm lint, pnpm build all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* test(reborn): cover OAuth install Blocked::Auth path; stop boot re-attempt of Failed extensions

Item 1 (coverage): install_extension_on_surface accepts a dispatch-time
Blocked::Auth as success only when the caller-scoped membership readback
proves the exact package is now visible — pinning the join-before-block
ordering OAuth installs depend on. Adds two contract cases through the
real RebornServices seam (BlockedAuthExtensionInstallInvoker):
Blocked::Auth WITH membership visible -> success; WITHOUT membership ->
retryable 503. Break-to-confirm: removing the Blocked::Auth arm turns
case (a) into a 503, proving the test pins that branch. Also drops two
pre-existing clone_on_copy on ActivityId (Copy) in this owned test file.

Item 2 (fix): generic extension-host boot restore re-published every
installed non-hosted-MCP extension regardless of durable state, which
re-ran activation for a terminally-failed (Unhealthy) installation on
every boot and — on success — masked the failure as active, breaking the
InstallationState::Failed "does not auto-retry" contract (overview
§6.1). build_generic_extension_host now skips boot re-publication of an
installation whose durable health is Unhealthy, leaving it
installed-but-not-served and remediable; healthy and hosted-MCP restore
paths are unchanged. Regression test asserts a durably-failed install is
not re-published while a healthy sibling still restores.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(reborn): trust-gate [admin_configuration] to first-party manifests

An untrusted extension could declare an `[admin_configuration]` group
colliding with a first-party group id (e.g. `extension.slack`). The
composition fold registered admin descriptors from every catalog package
— including filesystem-discovered, non-first-party ones — so a colliding
third-party manifest either aborted every boot with a DescriptorConflict
(byte-different) or was silently registered as a consumer of the
first-party group's non-secret routing (byte-identical): a boot-DoS and a
non-secret confused-deputy.

Fail closed toward the safer behavior:
- Trust-gate `[admin_configuration]` at parse (v3): only host-bundled
  (first-party) manifests may declare an admin group. A filesystem
  manifest that declares one now fails to parse and is skipped by the
  existing fail-open catalog loader instead of aborting boot.
- Filter the composition fold (`admin_configuration_uses`) to first-party
  sources as defense in depth: a non-first-party package can never be
  folded as a consumer or reach the descriptor service.

Coverage:
- The v3 trust gate rejects InstalledLocal/RegistryInstalled admin
  groups; a filesystem package is skipped without aborting boot while
  the first-party group still resolves; the fold excludes non-first-party
  sources.
- The four previously-uncovered validate_channel_admin_configuration
  branches (egress credential, egress body credential, connection
  deep-link placeholder, wrong secret flag) now fail closed under test.
- The ordinary-user (lifecycle/setup) projection carries no admin
  material; the admin-configuration routes 403 a non-operator caller.

test_support::resolve now stamps HostBundled: its channel fixtures
declare admin-config-backed signing secrets, which only first-party
manifests may do; resolved output is source-independent for their
third-party trust class.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(reborn): stop the shared ingress classifier from swallowing normal chat

The channel-neutral gate-command classifier runs generically on every
inbound message. Any message whose first token was a command verb
(`approve`/`deny`/`auth`) but which was NOT the reserved gate-command
shape classified to `NoOp` and settled silently — no turn, no
user-visible feedback. A normal chat message like "approve this design"
was lost, and a bare `auth deny` (missing ref) or `auth deny gate:x
extra` was pulled out of the conversation the same way.

Distinguish a *confident* gate command — the reserved shape the system
advertises: a bare `approve`/`deny`, or any verb carrying a `gate:<ref>`
(`approve gate:<ref>`, `auth deny gate:<ref>`) — from ambiguous natural
language that merely starts with a verb. Confident commands still
classify to their resolution payload, so the real approve/deny/auth gate
flow is unchanged. Everything else now returns `Ok(None)` and falls
through to normal turn handling instead of being silently classified out
of the conversation as a no-op. (`Err` is still returned only when a
confident command carries a hostile/invalid ref that fails payload
validation.)

Because the classifier is the single channel-neutral definition, the fix
applies uniformly across Slack, Telegram, and the generic sink — no
per-channel drift.

Tested at two seams:
- classifier grammar: ambiguous verb-first text ("approve this design",
  bare `auth deny`, `auth deny gate:x extra`, "deny that idea") routes as
  a user message; confident `approve gate:<ref>`, bare `deny`, and
  `auth deny gate:<ref>` still parse to their resolution payloads.
- GenericChannelInboundSink ingress: ambiguous verb-first chat reaches
  the workflow as a UserMessage (a turn IS submitted, not swallowed);
  confident `approve gate:<ref>` still reaches it as an ApprovalResolution
  and bare `deny` as a ScopedApprovalResolution.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(6520): harden trigger self-mutation + OAuth reconcile + origin/delivery coverage (lane T)

Review-pass items 1-6 for PR #6520:

1 (serve.rs): document that poller-on wires the TenantMembership grant
  unconditionally by design — fire access is gated on ACTIVE canonical
  membership at fire time (IdentityMembershipTriggerFireChecker denies
  suspended/unknown/wrong-tenant), not the auth method. Enforcement is already
  tested at the checker seam and the policy-shape regression test already exists.

2 (trigger_management.rs): broaden the routine self-mutation backstop to deny
  the Automation origin as well as ScheduledLoopRun (matches the descriptors'
  automation=Forbidden gate matrix); document the defense layering (the runner's
  scheduled_trigger surface-deny and the subagent flavor tool allowlist are the
  structural guarantees; this origin check is the backstop, which cannot see a
  subagent's lost ScheduledTrigger lineage — the surface exclusion protects that
  path, and spawn_subagent is globally disabled pending #4147). +5 crate-tier
  tests through dispatch().

3 (lifecycle_auth_continuation.rs): do NOT durably fence a SetupIncomplete OAuth
  continuation — return a retryable error so the caller leaves the flow
  un-fenced and a later cross-replica reconcile completes the fan-out once
  readiness is Active. auth.rs documents the caller side. +unit re-drive test +
  composition route reconcile-recovery test.

4 (scope.rs): document resolved_origin's LoopRun-only fallback invariant — a
  scheduled run always stamps ScheduledLoopRun upstream, so it never reaches or
  is downgraded by the fallback. +tests. A debug_assert/fail-closed was rejected:
  it breaks the pinned transitional-compat contract in ironclaw_capabilities::host.

5 (trigger_management.rs): test that an explicit delivery_target_id wins over a
  source run context and the implicit resolver is never consulted.

6 (invocation.rs): correct the stale "nothing wired into the dispatch path yet"
  doc (Invocation/InvocationOrigin are consumed live). The group_triggers
  external-source scenario now asserts the persisted delivery_target_id EQUALS
  the registered source target, not just is_some().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* chore(reborn): recapture composition pub-use snapshot + drop dead OperatorSetupCall alias

Two pre-existing merge blockers exposed by the strict gates (both on the base
PR head, not introduced by the correctness fixes):

- docs/plans/composition-pubuse.snapshot still listed `InboundPayloadClassifier`,
  which the generic refactor removed from the composition public surface;
  `ironclaw_architecture::composition_public_pub_use_surface_matches_snapshot`
  failed. Recaptured to match `lib.rs`.
- `type OperatorSetupCall` in webui_v2_handlers_contract.rs was unused, tripping
  `clippy -D warnings` (dead_code). Removed.

Result: `cargo clippy --workspace --all-targets --all-features -- -D warnings`
now exits clean across the workspace.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK1qoiKDmXh2KzrgeMfTPf

* fix(reborn): finish #6442 test-suite reconciliation to the #6520 contracts

Production wiring restored (dropped in the merge, surfaced as dead code):
boot-time hosted-MCP reconcile after restore, the composed
RecipeAuthChallengeProvider (pairing + product-auth challenges) feeding
projections and channel delivery, current-delivery-target resolver assembly
attach, and manifest-derived account-setup descriptors (catalog + injected
extras) so /start pairing and connect notices compose again.

Test reconciliation (no test deleted or weakened): #6442-side suites
rewritten to the no-Activate three-state lifecycle (install drives
readiness; installs are caller-private, so surface tests install as the
surface user), caller-scoped 3-arg project with the production credential
gate, LifecyclePublicState assertions, admin-configuration seeding via the
composed resolver, RunFinalReplyDestination::External, and the 1-arg
active-extension authority in the integration harness. Three #6520 test
accessors ported onto the flat RebornRuntime. Ratchet maintenance:
HostedMcpDiscoveryOutcome frozen-name entry, specificity allowlist
re-pathing for merge-moved files, stale entries dropped, and the
banned "mcp_server" literal swapped out of a lifecycle test fixture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* chore(reborn): relocate channel-host e2e tests under the test-only path convention

The no-panics checker exempts src/**/tests/* and cannot see cross-file
cfg(test) gating; a #[path] attribute keeps the module name and its
super:: references unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(reborn): honor typed auth denial end-to-end + compose the lifecycle OAuth continuation

Executor/runner denial tests rewritten to the typed-denial contract (the
host terminalizes the exact durable invocation through the capability
port; unrelated batch calls dispatch alongside). Three harness
HostRuntime wrappers forward the new decline_auth_capability trait
method (its default fails closed and was killing every denied-auth-resume
run at the integration tier).

Production fixes surfaced by the suites: the admin-configuration
resolver treats an extension with no declared [admin_configuration] (or
one imported after boot) as empty config instead of failing activation
with UnknownExtension; the restored hosted-MCP boot reconcile skips
unresolvable orphan installation rows instead of failing boot (§6.5);
and the factory now composes lifecycle_auth_continuation_dispatcher over
the base product-auth dispatcher via a two-phase build (dependencies
clone first, dispatcher wrap after the lifecycle facade exists) — an
extension-card OAuth completion re-enters the canonical lifecycle
install/readiness command instead of being durably fenced un-activated.
Regression: completed_lifecycle_activation_continuation_installs_the_extension
drives flow create → claim → complete → reconcile through the composed
runtime and pins install + membership + published tools + the fence.

Golden payload snapshots regenerated for the intended
ResultReference success-observation shape, with minted result refs
normalized alongside the existing volatile fields. Isolation fixture
supplies the bundled first-party surface (test-support builds skip the
cfg(test)-only injection); restart test expects caller-private scope
per the membership model.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* style(reborn): format merged sources; carry frontend test reconciliation

The merge commit missed the unstaged frontend test rewrites (client
action id wire, no-Activate) and post-merge rustfmt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(reborn): burn down post-merge CI failures — restore dropped wirings, reconcile tests to #6520 contracts

Production fixes (dropped in the main merge, restored from the PR side):
- factory.rs: register the product-owned RunFinalReplyRoutingService over the
  fail-closed UnavailableRunFinalReplyRouter so the model-facing
  builtin.outbound_delivery_target_route_current capability can route a run's
  final reply again
- factory.rs: seed the host-owned WebApp final-reply target in the outbound
  delivery target registry (host_owned_outbound_delivery_target_registry)

Test/harness reconciliations to the merged #6520 + client_action_id contracts:
- webui handlers contract: prime membership read-back for second installs,
  add required client_action_id to install bodies, reconcile retired
  "unsupported" phase literal to setup_needed (118/118)
- webui lib: update SPA shape tests to the Lane F channelConnectionFromGate
  predicate + split configure-modal expression (187/187)
- frontend: drop dead ./pairing-api import (tsc/lint green)
- composition: auth_tests lifecycle-activation callback test installs github
  first and asserts Active projection; core.rs auth-gate test seeds the
  caller-phase notion account (secretless) so the surface is model-visible
- webui_v2_e2e: uninstalled-phase literals, no-Activate reconciliations
  (install owns fail-closed provider-instance 400; OAuth-complete asserts
  active projection instead of the deleted activate route) (14/14)
- webui_v2_serve: setup projection phase literal (62/62)
- CLI extension: search envelope neutral phase (6/6)
- integration: client_action_id on isolation/product_api lifecycle bodies;
  harness github fixture gains a /tenants mount + filesystem run-state store so
  typed auth-gate deny/resume terminalizes durably (auth_gate 17/17); fake
  outbound facade mirrors the production always-present web_app target;
  QA smoke counts distinct install gestures (hosted-MCP bounded retries
  re-dispatch the same activity id)
- live-QA canary harness: send required client_action_id on install/setup
  submit (4 sites) — root cause of the 12/12 canary wipeout

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(e2e): skip quarantined retired-activation traces in manifest loaders

The PR quarantined the retired-activation fixtures (files moved under
quarantined_retired_activation/, manifest gained quarantined_model_cases)
but the three python selected_cases consumers still read every case file at
collection time, so pytest died on FileNotFoundError before running a single
scenario (the WebUI v2 smoke lane wipeout).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* merge: fold main c6696ba (#6583 product-surface vocabulary, #6580, #6588) into the PR branch

Adopts the agent-resolved merge commit 461e348 (78 conflicted files:
ironclaw_product_workflow -> ironclaw_product crate rename, adapters folded
into ironclaw_product/host_api, ProductSurfaceCaller/ProductSurfaceError
vocabulary, generic ProductSurface trait in host_api) and repoints the
post-merge test fixes to the renamed crate. #6520 semantics preserved: no
Activate anywhere, required client_action_id install gesture, event-driven
delivery, channel-config deletion upheld.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(reborn): route channel-pairing completions through the lifecycle continuation dispatcher

Pairing completions dispatch AuthContinuationRef::LifecycleActivation, but
composition handed every ChannelPairingService a freshly built base
turn-resume dispatcher instead of the lifecycle-wrapped one product-auth
uses, so completing a pairing never re-ran readiness reconciliation /
runtime publication. Live repro on the demo stack: telegram remove →
install (activation parks on the connection requirement, unpublished) →
pair (bot replies paired) → card stuck at setup_needed forever.

Both product-auth and the pairing registry now share one wrapped dispatcher.
Regression pinned by pointer identity at the composition seam
(channel_pairing_completions_run_the_lifecycle_wrapped_continuation_dispatcher;
verified red pre-fix), via new test-support dispatcher accessors.

Also: live-QA canary harness reconciled to the post-#6520 wire (slack OAuth
start sends the manifest requirement handle; retired readiness booleans
dropped in favor of installation_state; non-secret setup values route
through the operator extension-configuration surface).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(ci): burn down the remaining red lanes at 6e948cb

Production wiring restored (dropped in the merges):
- factory.rs: LocalRuntimeTriggerCreatorPairingHook regains the #6520
  resolve_implicit_delivery_target override through a restored
  TriggerFinalReplyTargetService, reading a new late-bound
  TurnStateStore slot (trigger_source_turn_state_store) that the
  test-support repoint seam swaps alongside the snapshot slot — a
  trigger created from an externally-sourced turn inherits that turn's
  reply destination again
- trigger_final_reply_target.rs: the implicit-inference arm seals no
  target when the source run's reply binding maps to no current
  outbound target (explicit targets still fail closed); previously it
  failed the whole trigger_create

Test reconciliations to the post-#6520 contracts:
- composition core.rs: webui bundle target listing pins the
  always-present host-owned web_app destination (was: empty)
- scenario_trigger_self_create_denied: typed Resolution::Denied
  (PolicyDenied) pin + denied-before-dispatch re-pinned on capability
  RESULT absence (the authorize consolidation records port invocations)
- outbound_target: inventory = two seeded targets + builtin:web_app
- mcp.rs (main #6580): nearai journey reconciled to no-Activate —
  account seeded before install, install owns readiness and publication
- qa smoke bundled-surface: hosted-MCP nearai.web_search excluded from
  the binary-tier static-surface assert (no discovery egress seam
  there); its contract is pinned end-to-end by reborn_integration_mcp
- event-driven delivery wire asserts poll with the files' bounded 30s
  deadline instead of one post-idle snapshot (4 sites: telegram pairing
  + coordinated reply, slack final reply, cross-channel immediate)
- model_replay assert_provider_tools re-checks the surface for up to
  10s (hosted-MCP discovery publishes asynchronously)

New executable evidence (inventory gate):
- ProviderOperationCase entries for github get_authenticated_user /
  get_repo / list_releases and google-calendar list_calendars, whose
  harvested journeys were quarantined with the retired activation flow

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* merge: fold CI burn-down batch — trigger delivery-target capture restored, red lanes reconciled

Folds the agent-resolved CI fixes (2026299): restores the dropped
trigger-create implicit delivery-target capture (TriggerFinalReplyTargetService
over a late-bound TurnStateStore slot — the live "joke trigger delivered to
the web app instead of telegram" defect), reconciles the retired-contract
asserts (typed Denied verdicts, web_app always-present inventory, no-Activate
nearai evidence), converts four post-idle wire snapshots to the established
bounded poll under event-driven delivery, and re-covers the four
quarantine-orphaned provider capabilities with typed cases. Post-merge fmt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix: drop concrete extension name from generic composition comment; withdraw red repro pending its fix

The specificity gate rejects generic composition code naming a concrete
extension even in comments. Also withdraws the deliberately-red
member-remove repro test that was pushed prematurely in the previous
commit — it returns together with the identity fix that turns it green
(in flight), keeping the branch honest about what is currently pinned.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(ci): cycle-2 burn-down at 7011672

- facade/tests.rs: restore the `Blocked` import lost in the fold — the
  test-support/all-features compile error behind BOTH the Code Style
  clippy lane and the composition-core bucket (exact lane re-run: zero
  findings; bucket flags compile clean; facade tests 11/11)
- first-party coverage ratchet: `builtin.outbound_delivery_target_route_current`
  is a real registered builtin again (#6520 router restore); its e2e
  coverage is named — reborn_integration_delivery_user_journeys
  ROUTE_CURRENT journeys (26/26 ratchet suite green)
- webui_v2_product_api legacy canonicalization test: install body gains
  the required #6520 client_action_id gesture (29/29)

Responses API `test_reborn_responses_rejects_wrong_external_tool_call_id`:
unreproducible at this head — green locally alone, full-file, and in the
CI two-file shared-session shape (28/28); the one CI occurrence failed
server-side fast with the response error field truncated from the log.
Left untouched; if it recurs, capture the response error detail first.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

* fix(webui): carry the client gesture id on extension remove — stop permanent input deduplication

Root cause of the live "remove says success but stays installed" defect: the
#6596 merge restored main's remove_extension, which derives its ActivityId
from (caller, capability, input) with no client gesture component. The
product-capability invoker replays the durably recorded resolution per
activity id, so after one successful remove of an extension, EVERY later
remove of the same extension by the same caller — including after a
reinstall — replayed the first remove's recorded success without
dispatching: HTTP 200, no new product-result row, durable membership
untouched. First-vs-repeat remove per (user, extension) exactly matched the
live timeline (notion's first remove worked and recorded; telegram/slack
replayed their 21:5xZ successes forever).

remove_extension now takes RemoveExtensionBody with a required
client_action_id and derives extension_lifecycle_activity_id, mirroring
install (#6520 gesture idempotency: distinct gestures dispatch, response-lost
retries replay).

Regression tests (all verified red before their fix):
- webui_v2_handlers_contract: remove gesture-idempotency test (distinct
  gestures -> distinct activity ids; retry -> same id)
- facade/tests: channel-extension remove through the real product dispatch
  asserts the DURABLE membership row is deleted (the prior test asserted only
  the resolution verdict), plus caller-scoped projection read-back (installer
  sees telegram, another member does not — acceptance-contract member
  binding)
- factory/tests: re-land the withdrawn member-remove repro, green over the
  factory-tier channel-disconnect slot fill

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WF1EiatUVLjKKs3eYGMbKV

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-6588 — 1d334bd3 Deployed Jul 23, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: ci CI/CD workflows size: S 10-49 changed lines skip-regression-check Bypass regression test CI gate (tests exist but not in tests/ dir)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants