fix(live-qa): verify triggered Slack delivery through the two-lane contract - #7389
Merged
Merged
Conversation
…ntract Since #7157 a triggered fire's result is never pushed by the completion driver: the fire itself calls builtin.outbound_deliver, and the background-run notifier's triggered-run-delivery record describes NOTICE delivery only — a cleanly completed fire records `skipped`. The delivery cases still required that record to say `delivered`, which no longer exists for results, so qa_3d/qa_8d/qa_9b/qa_9d hard-failed every scheduled run from the first post-#7157 canary (2026-08-08 00:24 UTC) even though all four live fires verifiably delivered (three had the marker sitting in Slack history; the fourth was provider-confirmed). The waiter now verifies what the product actually guarantees: - success = the fire's durable outbound/deliveries model-delivery record for the exact run (delivered, expected DM) PLUS the independent Slack history read-back finding the marker; - notifier records: `skipped`/`no_default_configured`/`delivered` are healthy terminals, only `failed`/`denied` fail the case, and unknown future vocabulary surfaces through timeout diagnostics; - a completed outbound_deliver whose composed content lacks the marker fails deterministically (the qa_8d mode: the stale prompt bound the marker to the final answer, which is no longer the delivered payload); - the readback-inconclusive flake classification accepts an exactly-one-verified-send through either lane. Case prompts now bind the marker to the delivered Slack message itself (and still to the final answer), via one shared prompt-requirement helper. Also fixes the QA 6D-6E strict-scrub false positive: progressive tool disclosure (#6958) records tool_search output in traces, and the builtin.extension_register_hosted_mcp description's "bearer for a static API token or PAT sent as a Bearer token" prose tripped the bearer pattern, deleting the trace and failing the shard with all cases green. The bearer pattern now requires 16+ token-alphabet characters. All delivery-wait decision logic is pinned by new unit tests against the production record shapes captured from the failing canary artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🚅 Deployed to the ironclaw-pr-7389 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7389
August 8, 2026 04:05
Destroyed
github-actions
Bot
added
scope: docs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughSlack live QA now verifies delivery through durable records and independent Slack history. It records sanitized evidence, updates prompt requirements, expands contract tests, documents the model, and applies a 16-character bearer-token threshold. ChangesSlack delivery verification
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LiveQA
participant DeliveryRecords
participant Slack
LiveQA->>DeliveryRecords: inspect expected-channel model delivery
LiveQA->>Slack: read message history for marker
DeliveryRecords-->>LiveQA: return delivery evidence
Slack-->>LiveQA: return bot-authored marker
LiveQA-->>LiveQA: classify success, failure, or inconclusive
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Contributor
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟥 Final result · Could not complete
Automatic trigger · attempt 1 of 3 · failed after 7s IronLoop could not complete the review for this Run. Failure details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@scripts/live-canary/scrub-artifacts.sh`:
- Around line 214-219: The Python bearer pattern in REDACT_PATTERNS must match
the shell guardrail by requiring at least 16 token-alphabet characters after
“bearer”. Update that regex in emit_results_json.py and add regression coverage
for prose such as “bearer for a static API token” remaining unchanged while a
valid long bearer credential is still redacted, including the required
workflow/test update for the CI behavior change.
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 3959-3998: Update the settled-fire condition after
_classify_triggered_notice_outcome(outcome) to use the appropriate
settled/healthy terminal notice_state, including gate-resumed delivered
outcomes, instead of rechecking raw outcome values. Preserve the existing
deliver-evidence marker validation and deterministic AssertionError behavior for
every classifier-recognized settled fire.
- Around line 3813-3820: Update the marker_deliver_count increment in the
preview-processing logic to occur only when the preview status is "completed",
matching completed_deliver_count. Keep marker matching unchanged for completed
previews and preserve the existing counter behavior for other statuses.
- Around line 4027-4031: Namespace the vendor and deliver evidence under
separate keys in the merged evidence assigned to “delivery_readback_evidence”,
preserving both “parse_error_count” values instead of allowing the deliver value
to overwrite the vendor value. Update assertions in
“test_slack_delivery_routine_readback_miss_is_infrastructure_inconclusive” and
any other consumers to read the nested lane-specific keys while leaving the
evidence dict pass-through unchanged.
In `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py`:
- Around line 10328-10349: The _create_store method duplicates the
root_filesystem_entries schema manually. Replace its inline CREATE TABLE
statement with the existing run_live_qa._root_filesystem_create_table helper,
reusing that canonical schema while preserving the current database connection,
commit, and returned path behavior.
- Around line 10519-10549: Add a third outbound deliver preview record in
test_outbound_deliver_evidence_counts_marker_content with status="failed" and
the same marker, then retain assertions that completed_deliver_count is 2 and
marker_deliver_count is 1. Ensure the fixture verifies marker counting only for
completed deliveries.
🪄 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: 3a5b6af4-1072-43bd-8a10-ea976105d95a
📒 Files selected for processing (5)
docs/internal/live-canary.mdscripts/live-canary/scrub-artifacts.shscripts/live-canary/test_scrub_artifacts.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
8 tasks done
serrrfirat
previously approved these changes
Aug 8, 2026
- Gate marker_deliver_count on completed previews: a failed or in-flight outbound_deliver whose content carries the marker never reached Slack, and counting it could fake the exactly-one-verified-send inconclusive classification or suppress the deterministic markerless red. Fixture gains a failed marker-bearing preview, observed red before the fix. - Align emit_results_json.py's bearer pattern with the scrub script's 16-char floor so description prose in results.json is not mangled to "Bearer <REDACTED>"; prose-preservation regression added. - Namespace the readback-inconclusive evidence per lane (vendor_evidence/deliver_evidence) — both dicts carry parse_error_count and the flat merge let one overwrite the other. - Reuse the production root_filesystem schema helpers in the new test fixtures instead of a hand-written CREATE TABLE. - Document why the deterministic content check keys on skipped/no_default_configured rather than the whole healthy_terminal class: `delivered` includes a fire parked on an approval gate whose run resumes — and may deliver — after the notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7389
August 8, 2026 12:27
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)
scripts/reborn_webui_v2_live_qa/run_live_qa.py (1)
3809-3821: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSkip non-completed previews before parsing
input_summary.Line 3809 parses
input_summarybefore Line 3817 checksstatus. A failed preview with malformedinput_summaryincrementsparse_error_countinstead of being ignored.If the same run has a completed markerless delivery, that parse error suppresses the deterministic marker failure. The case then waits until timeout.
Move the status guard before parsing
input_summary. Add a regression fixture with a failed preview that has malformedinput_summary.Proposed fix
if preview.get("capability_id") != "builtin.outbound_deliver": continue + if str(preview.get("status") or "") != "completed": + continue input_summary = json_object(preview.get("input_summary")) if input_summary is None: evidence["parse_error_count"] = int(evidence["parse_error_count"]) + 1 continue - if str(preview.get("status") or "") != "completed": - continue evidence["completed_deliver_count"] = ( int(evidence["completed_deliver_count"]) + 1 )🤖 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 `@scripts/reborn_webui_v2_live_qa/run_live_qa.py` around lines 3809 - 3821, In the preview-processing flow, move the completed-status check before the json_object(preview.get("input_summary")) call so failed or in-flight previews are ignored without affecting parse_error_count. Preserve completed preview parsing and evidence counting, and add a regression fixture covering a failed preview with malformed input_summary.
🤖 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 `@scripts/live-canary/test_emit_results_json.py`:
- Around line 332-343: Extend test_bearer_prose_survives_redaction with an
explicit boundary fixture containing exactly 15 token-alphabet characters after
“bearer” and assert it remains unchanged, then add a 16-character case that
exercises the redaction threshold as intended. Keep the existing
prose-preservation assertion and use emit.redact for both boundary cases.
---
Outside diff comments:
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 3809-3821: In the preview-processing flow, move the
completed-status check before the json_object(preview.get("input_summary")) call
so failed or in-flight previews are ignored without affecting parse_error_count.
Preserve completed preview parsing and evidence counting, and add a regression
fixture covering a failed preview with malformed input_summary.
🪄 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: 6ff5bbae-0b68-4dad-b6c4-0048650529dd
📒 Files selected for processing (4)
scripts/live-canary/emit_results_json.pyscripts/live-canary/test_emit_results_json.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
…ules
The prose-preservation tests prove prose survives but not the threshold
itself — a {15,} regression would have passed both. Pin the 15/16
boundary explicitly in the emitter suite and the shell scrubber suite,
since the two rule sets are documented as kept in sync.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7389
August 8, 2026 12:34
Destroyed
serrrfirat
approved these changes
Aug 8, 2026
serrrfirat
added a commit
that referenced
this pull request
Aug 9, 2026
- Merge origin/main (#7377 run-acts-as-invoker, #7323, #7382, #6938, #7280, #7393, #7389, #7364, #7228, #7371, #7399). - main's #7377 landed a narrower terminal arm (generic failure notice for TurnStatus::Failed only); keep the #6896 arm, which covers Failed and RecoveryRequired with sanitized per-category summaries plus Cancelled and the timeout grace path, and adapt to the Option<String> notice_discriminator main introduced. - Re-seed the composition budget to the merged-tree measurement (40811 -> 40861, the run-failure settlement observer lands +50 governed LOC); the arch-test record moves with the manifest.
personal-upstream-sync Bot
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Aug 10, 2026
…rai#6896) (nearai#7131) * fix(run_delivery): deliver triggered run failures to the creator (nearai#6896) Scheduled/triggered runs that ended in Failed, Cancelled, or RecoveryRequired produced no user-visible notification: the triggered delivery driver minted notifications only for Completed / BlockedApproval / BlockedAuth and recorded every other terminal status as Skipped. A run that timed out before reaching an actionable state only logged a warn and recorded Failed, leaving the creator in silence. Delivery: - triggered_notification_for_state now mints a FinalReplyReady notification for Failed and RecoveryRequired using the existing per-category failure summaries (reborn_failure_summary_for_category) over state.failure.category(), with a generic fallback when no category is present. - Cancelled mints the same notification, preferring a failure-category summary when one is present and falling back to a fixed cancellation notice otherwise. - The RunWaitTimedOut branch with no prior blocked marker now delivers the timeout notice as a terminal reply instead of recording Failed. - The wildcard arm is replaced with explicit non-actionable statuses (Queued, Running, CancelRequested, BlockedResource, BlockedDependentRun, BlockedExternalTool) so a future status fails to compile rather than silently skipping. Observer: - TriggerFireSettlementObserver gains on_failed_fire_settled as a default no-op method, plus a TriggerFailedFireSettlement event carrying tenant/trigger/fire-slot/run-id/history-status. Noop and existing implementors keep compiling. - The active-cleanup sweep fires on_failed_fire_settled when clear_active_fire succeeds with TriggerRunHistoryStatus::Error, so post-accept failures are observable for automation health. Ok, Running, and already-cleared fires do not fire the hook. Tests: - run_delivery_contract: Failed+model_error, Failed without category, Cancelled, and timeout-before-actionable all assert a Delivered outcome with the expected notice text and footer. - worker tests: a terminal-Error active fire fires exactly one on_failed_fire_settled; a terminal-Ok active fire fires none. The larger retry/redrive budget for failed post-accept fires (retry_disposition has zero production callers) is intentionally left for a follow-up; it is out of scope for this surgical delivery fix. * style: cargo fmt the nearai#6896 delivery fix * fix(triggers): address terminal delivery review feedback * fix(assistant): drop unused UserId import after merge * fix(run_delivery): address multi-agent review findings - Extract shared terminal-notice helpers (final_reply_notice, outcome_for_delivery_failure, deliver_terminal_notice) so the timeout, OAuth-backstop, and generic failure arms share one notice shape and outcome taxonomy instead of a third hand-rolled copy. - Add a bounded race-grace window after the wait backstop: a run that crosses into a terminal state during the final wait (cancellation in flight, failure landing after the last poll) now delivers the correct terminal notice instead of the timeout copy. - Cancelled runs always deliver the fixed cancellation notice; the failure-category branch was unreachable in production and would have mislabeled a host/operator cancel as a failure. - Update the stale invariant doc, the five-output surface contract count, and the exhaustiveness-only comment on the non-actionable arm. - Document the cheap/non-blocking contract on TriggerFireSettlementObserver (the worker awaits it inline in the poller sweep) and note it at the active-cleanup call site. - Add contract coverage for the timeout arm's delivery-failure outcome (Failed) and a regression test proving the race-grace path delivers the cancellation notice; the cancelled-with-category test now asserts the cancellation notice wins. * fix(run_delivery): address review comments and restore CI gates Review fixes (CodeRabbit on 01e887f/f8af109): - Grace loop fails loud: log the bound TurnError on state-poll failure and the RunDeliveryError on terminal-notice build failure before falling back to the timeout copy, with silent-ok markers on both intentional fallbacks. - Hoist TriggeredReplyTargetAuthority, CodecChannelTargetResolver, and TriggeredNotificationContext to one construction before the watcher loop; the race-grace arm, timeout arm, and loop body now share it. - Collapse the duplicated failure-summary expression into one closure and name TurnStatus::Failed explicitly so future statuses are compiler-visible. - Drop the stale "Only three states" count from the surface-contract doc. - Test fixture: encode the late-terminal flip as one Option<(usize, ScriptedRunState)> field instead of two correlated Options with an expect. - Terminal-crossing test: document why flip_after=30 deterministically outruns the wait poll budget and assert the grace loop issues no cancellation (cancel_calls == 0). CI: - composition-budget: re-seed loc_ceiling 40432 -> 40593 (measured on the merged tree; the nearai#7131 settlement observer adds +161 governed LOC of wiring) and move the arch-test record with it. - trigger_poller: use the colon-form tracing target required by nearai#7146. * ci: re-trigger pull_request workflows for c2460ed * fix(composition): capture the settlement health warn in the observer test The traced_test default filter is {crate}=trace, which drops events whose metadata target is `ironclaw::reborn::…`. The observer warning is emitted with the colon-form target (required by nearai#7146 — the equals form recorded a field and never matched RUST_LOG target filters), so the test saw an empty buffer. Enable tracing-test's no-env-filter feature, the same pattern the capabilities/host-runtime/mcp/loop crates use for cross-target assertions. Re-seed the composition budget to the merged-tree measurement (40747 -> 40867): nearai#7131's observer wiring lands on top of post-measurement mainline inflow; measured with the gate, set to current. The arch-test record moves with the manifest. * fix(run_delivery): merge main and adapt to notice_discriminator String - Merge origin/main (nearai#7377 run-acts-as-invoker, nearai#7323, nearai#7382, nearai#6938, nearai#7280, nearai#7393, nearai#7389, nearai#7364, nearai#7228, nearai#7371, nearai#7399). - main's nearai#7377 landed a narrower terminal arm (generic failure notice for TurnStatus::Failed only); keep the nearai#6896 arm, which covers Failed and RecoveryRequired with sanitized per-category summaries plus Cancelled and the timeout grace path, and adapt to the Option<String> notice_discriminator main introduced. - Re-seed the composition budget to the merged-tree measurement (40811 -> 40861, the run-failure settlement observer lands +50 governed LOC); the arch-test record moves with the manifest.
43 of 48 tasks
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
…ntract (nearai#7389) * fix(live-qa): verify triggered Slack delivery through the two-lane contract Since nearai#7157 a triggered fire's result is never pushed by the completion driver: the fire itself calls builtin.outbound_deliver, and the background-run notifier's triggered-run-delivery record describes NOTICE delivery only — a cleanly completed fire records `skipped`. The delivery cases still required that record to say `delivered`, which no longer exists for results, so qa_3d/qa_8d/qa_9b/qa_9d hard-failed every scheduled run from the first post-nearai#7157 canary (2026-08-08 00:24 UTC) even though all four live fires verifiably delivered (three had the marker sitting in Slack history; the fourth was provider-confirmed). The waiter now verifies what the product actually guarantees: - success = the fire's durable outbound/deliveries model-delivery record for the exact run (delivered, expected DM) PLUS the independent Slack history read-back finding the marker; - notifier records: `skipped`/`no_default_configured`/`delivered` are healthy terminals, only `failed`/`denied` fail the case, and unknown future vocabulary surfaces through timeout diagnostics; - a completed outbound_deliver whose composed content lacks the marker fails deterministically (the qa_8d mode: the stale prompt bound the marker to the final answer, which is no longer the delivered payload); - the readback-inconclusive flake classification accepts an exactly-one-verified-send through either lane. Case prompts now bind the marker to the delivered Slack message itself (and still to the final answer), via one shared prompt-requirement helper. Also fixes the QA 6D-6E strict-scrub false positive: progressive tool disclosure (nearai#6958) records tool_search output in traces, and the builtin.extension_register_hosted_mcp description's "bearer for a static API token or PAT sent as a Bearer token" prose tripped the bearer pattern, deleting the trace and failing the shard with all cases green. The bearer pattern now requires 16+ token-alphabet characters. All delivery-wait decision logic is pinned by new unit tests against the production record shapes captured from the failing canary artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(live-qa): close review findings on the two-lane delivery contract - Gate marker_deliver_count on completed previews: a failed or in-flight outbound_deliver whose content carries the marker never reached Slack, and counting it could fake the exactly-one-verified-send inconclusive classification or suppress the deterministic markerless red. Fixture gains a failed marker-bearing preview, observed red before the fix. - Align emit_results_json.py's bearer pattern with the scrub script's 16-char floor so description prose in results.json is not mangled to "Bearer <REDACTED>"; prose-preservation regression added. - Namespace the readback-inconclusive evidence per lane (vendor_evidence/deliver_evidence) — both dicts carry parse_error_count and the flat merge let one overwrite the other. - Reuse the production root_filesystem schema helpers in the new test fixtures instead of a hand-written CREATE TABLE. - Document why the deterministic content check keys on skipped/no_default_configured rather than the whole healthy_terminal class: `delivered` includes a fire parked on an approval gate whose run resumes — and may deliver — after the notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-qa): pin the exact 16-char bearer floor on both redaction rules The prose-preservation tests prove prose survives but not the threshold itself — a {15,} regression would have passed both. Pin the 15/16 boundary explicitly in the emitter suite and the shell scrubber suite, since the two rule sets are documented as kept in sync. Co-Authored-By: Claude Fable 5 <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
…ntract (nearai#7389) * fix(live-qa): verify triggered Slack delivery through the two-lane contract Since nearai#7157 a triggered fire's result is never pushed by the completion driver: the fire itself calls builtin.outbound_deliver, and the background-run notifier's triggered-run-delivery record describes NOTICE delivery only — a cleanly completed fire records `skipped`. The delivery cases still required that record to say `delivered`, which no longer exists for results, so qa_3d/qa_8d/qa_9b/qa_9d hard-failed every scheduled run from the first post-nearai#7157 canary (2026-08-08 00:24 UTC) even though all four live fires verifiably delivered (three had the marker sitting in Slack history; the fourth was provider-confirmed). The waiter now verifies what the product actually guarantees: - success = the fire's durable outbound/deliveries model-delivery record for the exact run (delivered, expected DM) PLUS the independent Slack history read-back finding the marker; - notifier records: `skipped`/`no_default_configured`/`delivered` are healthy terminals, only `failed`/`denied` fail the case, and unknown future vocabulary surfaces through timeout diagnostics; - a completed outbound_deliver whose composed content lacks the marker fails deterministically (the qa_8d mode: the stale prompt bound the marker to the final answer, which is no longer the delivered payload); - the readback-inconclusive flake classification accepts an exactly-one-verified-send through either lane. Case prompts now bind the marker to the delivered Slack message itself (and still to the final answer), via one shared prompt-requirement helper. Also fixes the QA 6D-6E strict-scrub false positive: progressive tool disclosure (nearai#6958) records tool_search output in traces, and the builtin.extension_register_hosted_mcp description's "bearer for a static API token or PAT sent as a Bearer token" prose tripped the bearer pattern, deleting the trace and failing the shard with all cases green. The bearer pattern now requires 16+ token-alphabet characters. All delivery-wait decision logic is pinned by new unit tests against the production record shapes captured from the failing canary artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(live-qa): close review findings on the two-lane delivery contract - Gate marker_deliver_count on completed previews: a failed or in-flight outbound_deliver whose content carries the marker never reached Slack, and counting it could fake the exactly-one-verified-send inconclusive classification or suppress the deterministic markerless red. Fixture gains a failed marker-bearing preview, observed red before the fix. - Align emit_results_json.py's bearer pattern with the scrub script's 16-char floor so description prose in results.json is not mangled to "Bearer <REDACTED>"; prose-preservation regression added. - Namespace the readback-inconclusive evidence per lane (vendor_evidence/deliver_evidence) — both dicts carry parse_error_count and the flat merge let one overwrite the other. - Reuse the production root_filesystem schema helpers in the new test fixtures instead of a hand-written CREATE TABLE. - Document why the deterministic content check keys on skipped/no_default_configured rather than the whole healthy_terminal class: `delivered` includes a fire parked on an approval gate whose run resumes — and may deliver — after the notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-qa): pin the exact 16-char bearer floor on both redaction rules The prose-preservation tests prove prose survives but not the threshold itself — a {15,} regression would have passed both. Pin the 15/16 boundary explicitly in the emitter suite and the shell scrubber suite, since the two rule sets are documented as kept in sync. Co-Authored-By: Claude Fable 5 <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
…ntract (nearai#7389) * fix(live-qa): verify triggered Slack delivery through the two-lane contract Since nearai#7157 a triggered fire's result is never pushed by the completion driver: the fire itself calls builtin.outbound_deliver, and the background-run notifier's triggered-run-delivery record describes NOTICE delivery only — a cleanly completed fire records `skipped`. The delivery cases still required that record to say `delivered`, which no longer exists for results, so qa_3d/qa_8d/qa_9b/qa_9d hard-failed every scheduled run from the first post-nearai#7157 canary (2026-08-08 00:24 UTC) even though all four live fires verifiably delivered (three had the marker sitting in Slack history; the fourth was provider-confirmed). The waiter now verifies what the product actually guarantees: - success = the fire's durable outbound/deliveries model-delivery record for the exact run (delivered, expected DM) PLUS the independent Slack history read-back finding the marker; - notifier records: `skipped`/`no_default_configured`/`delivered` are healthy terminals, only `failed`/`denied` fail the case, and unknown future vocabulary surfaces through timeout diagnostics; - a completed outbound_deliver whose composed content lacks the marker fails deterministically (the qa_8d mode: the stale prompt bound the marker to the final answer, which is no longer the delivered payload); - the readback-inconclusive flake classification accepts an exactly-one-verified-send through either lane. Case prompts now bind the marker to the delivered Slack message itself (and still to the final answer), via one shared prompt-requirement helper. Also fixes the QA 6D-6E strict-scrub false positive: progressive tool disclosure (nearai#6958) records tool_search output in traces, and the builtin.extension_register_hosted_mcp description's "bearer for a static API token or PAT sent as a Bearer token" prose tripped the bearer pattern, deleting the trace and failing the shard with all cases green. The bearer pattern now requires 16+ token-alphabet characters. All delivery-wait decision logic is pinned by new unit tests against the production record shapes captured from the failing canary artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(live-qa): close review findings on the two-lane delivery contract - Gate marker_deliver_count on completed previews: a failed or in-flight outbound_deliver whose content carries the marker never reached Slack, and counting it could fake the exactly-one-verified-send inconclusive classification or suppress the deterministic markerless red. Fixture gains a failed marker-bearing preview, observed red before the fix. - Align emit_results_json.py's bearer pattern with the scrub script's 16-char floor so description prose in results.json is not mangled to "Bearer <REDACTED>"; prose-preservation regression added. - Namespace the readback-inconclusive evidence per lane (vendor_evidence/deliver_evidence) — both dicts carry parse_error_count and the flat merge let one overwrite the other. - Reuse the production root_filesystem schema helpers in the new test fixtures instead of a hand-written CREATE TABLE. - Document why the deterministic content check keys on skipped/no_default_configured rather than the whole healthy_terminal class: `delivered` includes a fire parked on an approval gate whose run resumes — and may deliver — after the notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-qa): pin the exact 16-char bearer floor on both redaction rules The prose-preservation tests prove prose survives but not the threshold itself — a {15,} regression would have passed both. Pin the 15/16 boundary explicitly in the emitter suite and the shell scrubber suite, since the two rule sets are documented as kept in sync. Co-Authored-By: Claude Fable 5 <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
…rai#6896) (nearai#7131) * fix(run_delivery): deliver triggered run failures to the creator (nearai#6896) Scheduled/triggered runs that ended in Failed, Cancelled, or RecoveryRequired produced no user-visible notification: the triggered delivery driver minted notifications only for Completed / BlockedApproval / BlockedAuth and recorded every other terminal status as Skipped. A run that timed out before reaching an actionable state only logged a warn and recorded Failed, leaving the creator in silence. Delivery: - triggered_notification_for_state now mints a FinalReplyReady notification for Failed and RecoveryRequired using the existing per-category failure summaries (reborn_failure_summary_for_category) over state.failure.category(), with a generic fallback when no category is present. - Cancelled mints the same notification, preferring a failure-category summary when one is present and falling back to a fixed cancellation notice otherwise. - The RunWaitTimedOut branch with no prior blocked marker now delivers the timeout notice as a terminal reply instead of recording Failed. - The wildcard arm is replaced with explicit non-actionable statuses (Queued, Running, CancelRequested, BlockedResource, BlockedDependentRun, BlockedExternalTool) so a future status fails to compile rather than silently skipping. Observer: - TriggerFireSettlementObserver gains on_failed_fire_settled as a default no-op method, plus a TriggerFailedFireSettlement event carrying tenant/trigger/fire-slot/run-id/history-status. Noop and existing implementors keep compiling. - The active-cleanup sweep fires on_failed_fire_settled when clear_active_fire succeeds with TriggerRunHistoryStatus::Error, so post-accept failures are observable for automation health. Ok, Running, and already-cleared fires do not fire the hook. Tests: - run_delivery_contract: Failed+model_error, Failed without category, Cancelled, and timeout-before-actionable all assert a Delivered outcome with the expected notice text and footer. - worker tests: a terminal-Error active fire fires exactly one on_failed_fire_settled; a terminal-Ok active fire fires none. The larger retry/redrive budget for failed post-accept fires (retry_disposition has zero production callers) is intentionally left for a follow-up; it is out of scope for this surgical delivery fix. * style: cargo fmt the nearai#6896 delivery fix * fix(triggers): address terminal delivery review feedback * fix(assistant): drop unused UserId import after merge * fix(run_delivery): address multi-agent review findings - Extract shared terminal-notice helpers (final_reply_notice, outcome_for_delivery_failure, deliver_terminal_notice) so the timeout, OAuth-backstop, and generic failure arms share one notice shape and outcome taxonomy instead of a third hand-rolled copy. - Add a bounded race-grace window after the wait backstop: a run that crosses into a terminal state during the final wait (cancellation in flight, failure landing after the last poll) now delivers the correct terminal notice instead of the timeout copy. - Cancelled runs always deliver the fixed cancellation notice; the failure-category branch was unreachable in production and would have mislabeled a host/operator cancel as a failure. - Update the stale invariant doc, the five-output surface contract count, and the exhaustiveness-only comment on the non-actionable arm. - Document the cheap/non-blocking contract on TriggerFireSettlementObserver (the worker awaits it inline in the poller sweep) and note it at the active-cleanup call site. - Add contract coverage for the timeout arm's delivery-failure outcome (Failed) and a regression test proving the race-grace path delivers the cancellation notice; the cancelled-with-category test now asserts the cancellation notice wins. * fix(run_delivery): address review comments and restore CI gates Review fixes (CodeRabbit on 01e887f/f8af109): - Grace loop fails loud: log the bound TurnError on state-poll failure and the RunDeliveryError on terminal-notice build failure before falling back to the timeout copy, with silent-ok markers on both intentional fallbacks. - Hoist TriggeredReplyTargetAuthority, CodecChannelTargetResolver, and TriggeredNotificationContext to one construction before the watcher loop; the race-grace arm, timeout arm, and loop body now share it. - Collapse the duplicated failure-summary expression into one closure and name TurnStatus::Failed explicitly so future statuses are compiler-visible. - Drop the stale "Only three states" count from the surface-contract doc. - Test fixture: encode the late-terminal flip as one Option<(usize, ScriptedRunState)> field instead of two correlated Options with an expect. - Terminal-crossing test: document why flip_after=30 deterministically outruns the wait poll budget and assert the grace loop issues no cancellation (cancel_calls == 0). CI: - composition-budget: re-seed loc_ceiling 40432 -> 40593 (measured on the merged tree; the nearai#7131 settlement observer adds +161 governed LOC of wiring) and move the arch-test record with it. - trigger_poller: use the colon-form tracing target required by nearai#7146. * ci: re-trigger pull_request workflows for c2460ed * fix(composition): capture the settlement health warn in the observer test The traced_test default filter is {crate}=trace, which drops events whose metadata target is `ironclaw::reborn::…`. The observer warning is emitted with the colon-form target (required by nearai#7146 — the equals form recorded a field and never matched RUST_LOG target filters), so the test saw an empty buffer. Enable tracing-test's no-env-filter feature, the same pattern the capabilities/host-runtime/mcp/loop crates use for cross-target assertions. Re-seed the composition budget to the merged-tree measurement (40747 -> 40867): nearai#7131's observer wiring lands on top of post-measurement mainline inflow; measured with the gate, set to current. The arch-test record moves with the manifest. * fix(run_delivery): merge main and adapt to notice_discriminator String - Merge origin/main (nearai#7377 run-acts-as-invoker, nearai#7323, nearai#7382, nearai#6938, nearai#7280, nearai#7393, nearai#7389, nearai#7364, nearai#7228, nearai#7371, nearai#7399). - main's nearai#7377 landed a narrower terminal arm (generic failure notice for TurnStatus::Failed only); keep the nearai#6896 arm, which covers Failed and RecoveryRequired with sanitized per-category summaries plus Cancelled and the timeout grace path, and adapt to the Option<String> notice_discriminator main introduced. - Re-seed the composition budget to the merged-tree measurement (40811 -> 40861, the run-failure settlement observer lands +50 governed LOC); the arch-test record moves with the manifest.
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
reborn-webui-v2-live-qalane has failed every run since feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted #7157 merged: the delivery cases (qa_3d,qa_8d,qa_9b,qa_9d) still require the retired completion-driver push record (triggered-run-deliveryoutcome ==delivered), which post-feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted #7157 is never written for results — a cleanly completed fire recordsskippedby design ("Completed delivers NOTHING", spec §8).builtin.outbound_deliverwithprovider_confirmed: trueand real Slacktsrefs; three had the marker verifiably sitting in Slack history while the runner declared failure on the stale record.outbound/deliveries/model-delivery record for the exact run (statusdelivered, target = the expected DM) plus the independent Slackconversations.historyread-back finding the marker. Notifier recordsskipped/no_default_configured/deliveredare healthy terminals; onlyfailed/deniedfail a case; unknown vocabulary surfaces in timeout diagnostics instead of hard-failing.qa_8d's fire delivered a message whose content lacked the marker because the case prompt bound the marker to "the routine's final answer" — which pre-feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted #7157 was the delivered payload and no longer is. Case prompts now bind the marker to the delivered Slack message itself (shared_delivery_marker_prompt_requirementhelper), and a completedoutbound_deliverwhose content lacks the marker fails immediately with a distinct error rather than timing out.ironclaw.tool_searchoutput lands in traces, and thebuiltin.extension_register_hosted_mcpdescription prose "bearer for a static API token or PAT sent as a Bearer token" tripped thebearerpattern → strict scrub deleted the trace and exited 1. The pattern now requires 16+ token-alphabet characters (every real bearer credential shape is far longer than any English word that can follow "bearer").Change Type
Linked Issue
None (canary regression diagnosed from run 31229982850 artifacts; full evidence below).
Root-cause evidence (from the failing run's own artifacts)
qa_3d: runner erroroutcome={'outcome': 'skipped', …}while its own history probe recordedfound: True, marker_found: True, bot_authored_marker_matches: 1— delivery succeeded, assertion contract stale.qa_9b/qa_9d: same —marker_found: Truein the error's own history payload;qa_9d's persisted routine prompt contains the pinnedbuiltin__outbound_deliverstep with the resolved target id.qa_8d: runtime DB in the artifact showsbuiltin.outbound_delivercompleted with{"delivered":true,"provider_confirmed":true,"provider_message_refs":["1786148900.639239"], …}to the pinned DM target; the composed content had no marker (prompt phrasing), final answer did.ironclaw_assistant/src/run_delivery/triggered.rs): completed run → planNone→ recordsSkipped;Deliveredis reserved for notices (e.g. parked-gate prompts). Nothing writesdeliveredfor results.qa_2ecreation timeouts, QA 7/10 model-behavior probes) — those chronic flakes are unchanged by this PR and keep their existing classifications.Validation
python3 scripts/reborn_webui_v2_live_qa/test_run_live_qa.py— 217 tests OK (13 new)python3 scripts/live-canary/test_scrub_artifacts.py— 18 tests OK (1 new)cargo fmt/clippy/build: Not applicable — no Rust changes.Test Strategy
User behavior: scheduled live-canary verification of routine→Slack delivery; no product behavior changes.
Risk areas:
Tests added or updated:
TwoLaneDeliveryContractTests(13 tests) pin the outcome-vocabulary classifier, the durable model-delivery record reader (against the production record shape captured from the artifact, neutralized ids), the outbound_deliver evidence reader, the observed/inconclusive deciders, the waiter's pass/keep-polling/fail-fast/deterministic-content-fail/timeout paths, and that the case-builder prompt binds the marker to the delivered message.test_strict_scrub_ignores_bearer_prose_in_tool_descriptionsreproduces the exact 18:20 trace false positive byte-shape.reborn-webui-v2-live-qarun is the end-to-end proof; delivery cases stayretry_policy = "never".What the tests prove: the runner passes exactly when the fire durably delivered to the expected DM and the marker is independently read back; healthy notifier records can no longer fail a case; marker-less delivered content is a deterministic red, not a flake; prose after "bearer" no longer kills green shards.
Commands run: the two suites above.
Security Impact
Scrub guardrail: the
bearerpattern now requires 16+ token-alphabet chars after the keyword. Known real bearer-credential shapes (JWTeyJ…,xox[baprs]-…,ya29.…,gh[pousr]_…,sk-…,AKIA…) are all ≥16 and remain caught by this and their dedicated patterns; JSON-quoted"access_token": "…"shapes are unchanged. What is deliberately no longer flagged: ≤15-char words after "bearer" — i.e. English prose from model-visible tool descriptions, which strict mode was deleting from green shards. Canary results never persist message bodies/targets: the new record/evidence readers emit aggregate counts only (pinned by a sanitization assertion).Database Impact
None (read-only queries against the per-shard local runtime DB the runner already reads).
Blast Radius
scripts/reborn_webui_v2_live_qa/run_live_qa.pydelivery waiter + delivery-case prompts;scripts/live-canary/scrub-artifacts.shbearer pattern; their test suites;docs/internal/live-canary.md. No production code. Worst case: a delivery case misclassifies again on the next scheduled run — visible immediately in the lane.Rollback Plan
git revertof this single commit restores the previous runner and scrub behavior (the lane then returns to deterministic post-#7157 red).Review Follow-Through
Two follow-ups are product-side and intentionally NOT in this PR (different layer/rollback): (1)
builtin.outbound_deliver+builtin.outbound_delivery_targets_listare not inCORE_TOOL_NAMES, so on catalogs past the defer threshold the delivery pair drops out of the visible surface anddelivery.mdsteering is silently not injected (delivery_tools_visible=false) — observed producing a live web-created routine whose prompt instructedslack.send_messageto reach the requester; (2)TRIGGER_CREATE_DESCRIPTION's "never call builtin__outbound_deliver in a web-app-created routine" clause is over-applied by weaker models when a destination IS named (verbatim in qa_8d's creation reasoning). Companion PR follows.Review track: C (CI guardrail + canary contract)
🤖 Generated with Claude Code