fix(reborn): Slack delivery routing + tool-surface overhaul (identity, status, errors, threads, membership) - #5898
Merged
Merged
Conversation
…names, exactly once
Fixes the three recurring Slack automation failures from the tool-surface
audit, each pinned by tests that failed first:
1. Wrong-channel delivery: triggers gain an optional per-trigger
delivery_target_id (validated at create against the outbound target
registry, resolved again at fire time, delivered via the
TriggeredFromSourceRoute origin the resolution engine already
prefers). One automation's routing can no longer be clobbered by
another automation or a later change to the user-global preference;
unresolvable targets fail closed as TargetUnavailable instead of
falling back to another conversation.
2. Raw user IDs in digests: the Slack WASM tool now resolves message
authors and DM counterparts to user_display_name inside the tool
(one users.info per distinct id, best-effort so reads never fail on
name resolution), so human-readable output is the default path, not
something the model must remember. Rebuilt slack_user_tool.wasm and
added real output schemas for the read capabilities.
3. Duplicate delivery (bot + user identity): the scheduled-trigger and
inbound origin prompt lines now state that the final reply is
delivered automatically and forbid re-sending the run's result with
messaging capabilities, while explicitly allowing messaging-as-task
automations ("send Firat a joke") with recipients pinned at creation
time. slack.send_message, trigger_create, and the outbound target
tools' descriptions teach the same single-delivery contract, and the
embedded routine-advisor/delegation skills no longer teach retired
v1 routine tools (now gated by a zero-legacy test).
Regression coverage: cross-backend trigger repository round-trip,
dispatch-tier trigger_create accept/reject, composition-tier triggered
delivery routing (target-beats-preference, no-preference, fail-closed),
WASM runtime contract tests against the rebuilt artifact, prompt/
description contract tests, and an int-tier fail-closed group scenario.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 9, 2026 19:30
Destroyed
github-actions
Bot
added
scope: docs
📝 WalkthroughWalkthroughPer-trigger delivery targets now flow through trigger creation, validation, storage, firing, and Slack delivery. Slack capabilities add pagination, thread retrieval, identity enrichment, structured errors, and capability-specific schemas. WASM dispatch errors can expose sanitized guest error codes. ChangesPer-trigger delivery routing
Slack capability enrichment
WASM error summaries
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TriggerCreate
participant TriggerCreateHook
participant TriggerRepository
participant TriggerEvaluator
participant TriggeredRunDeliveryDriver
participant OutboundTargetProvider
TriggerCreate->>TriggerCreateHook: validate delivery_target_id
TriggerCreate->>TriggerRepository: persist TriggerRecord.delivery_target
TriggerRepository->>TriggerEvaluator: load TriggerRecord
TriggerEvaluator->>TriggeredRunDeliveryDriver: emit TriggerFire.delivery_target
TriggeredRunDeliveryDriver->>OutboundTargetProvider: resolve per-trigger route
OutboundTargetProvider-->>TriggeredRunDeliveryDriver: ReplyTargetBindingRef
sequenceDiagram
participant SlackCapability
participant SlackApi
participant UsersInfo
participant HostRuntime
SlackCapability->>SlackApi: request history, thread, search, or conversations
SlackApi-->>SlackCapability: Slack response
SlackCapability->>UsersInfo: resolve distinct user ids
UsersInfo-->>SlackCapability: display names or errors
SlackCapability->>HostRuntime: return enriched output or structured error
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
/canary |
Contributor
|
Started Reborn WebUI v2 live canary for |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces per-trigger outbound delivery routing, allowing trigger results to be delivered to specific targets rather than relying solely on user-global preferences. It also enriches Slack conversation history and DM lists by resolving raw user IDs to human-readable display names via a best-effort, cached lookup. Database schemas (PostgreSQL and libSQL) have been updated to support the new delivery_target field. The reviewer identified a performance risk in the display name resolution loop, where sequential synchronous network requests could cause latency or timeouts, and suggested limiting the number of lookups.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Collaborator
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_reborn_composition/src/slack/slack_host_beta.rs (1)
628-699: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
SlackOutboundTargetProviderConfigconstruction — extract a shared helper.The
tenant_id/agent_id/project_id/installation_id/team_id/configured_channel_routesmapping is copy-pasted verbatim betweenbuild_triggered_run_delivery_hook_from_parts(lines 678-696, newly added) andbuild_slack_host_beta_mounts(lines 836-854, pre-existing). A test helper (outbound_target_provider_config, line 3883) already implements this exact mapping — the two production call sites should share it instead of hand-duplicating the field list, so a future field addition toSlackHostBetaConfigcan't silently drift between the trigger-delivery-hook provider and the runtime-registered provider.♻️ Proposed fix
+fn slack_outbound_target_provider_config( + config: &SlackHostBetaConfig, +) -> SlackOutboundTargetProviderConfig { + SlackOutboundTargetProviderConfig { + tenant_id: config.tenant_id.clone(), + agent_id: config.agent_id.clone(), + project_id: config.project_id.clone(), + installation_id: config.installation_id.clone(), + team_id: config.team_id.clone(), + configured_channel_routes: config + .channel_routes + .iter() + .map(|route| { + SlackConfiguredChannelRoute::new( + route.channel_id.clone(), + route.subject_user_id.clone(), + ) + }) + .collect(), + } +} + fn build_triggered_run_delivery_hook_from_parts( ... - let outbound_target_provider: Arc<dyn OutboundDeliveryTargetProvider> = - Arc::new(SlackHostBetaOutboundTargetProvider::new( - SlackOutboundTargetProviderConfig { - tenant_id: config.tenant_id.clone(), - agent_id: config.agent_id.clone(), - project_id: config.project_id.clone(), - installation_id: config.installation_id.clone(), - team_id: config.team_id.clone(), - configured_channel_routes: config - .channel_routes - .iter() - .map(|route| { - SlackConfiguredChannelRoute::new( - route.channel_id.clone(), - route.subject_user_id.clone(), - ) - }) - .collect(), - }, - host_state.clone(), - host_state, - )); + let outbound_target_provider: Arc<dyn OutboundDeliveryTargetProvider> = + Arc::new(SlackHostBetaOutboundTargetProvider::new( + slack_outbound_target_provider_config(config), + host_state.clone(), + host_state, + ));Also applies to: 712-857
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/slack/slack_host_beta.rs` around lines 628 - 699, The `SlackOutboundTargetProviderConfig` setup is duplicated between `build_triggered_run_delivery_hook_from_parts` and `build_slack_host_beta_mounts`, so refactor both call sites to use the existing shared helper `outbound_target_provider_config` instead of rebuilding the same `tenant_id`/`agent_id`/`project_id`/`installation_id`/`team_id`/`configured_channel_routes` mapping inline. Keep the helper as the single source of truth and have `SlackHostBetaOutboundTargetProvider::new` receive its config from that shared path so future `SlackHostBetaConfig` field changes don’t drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/factory.rs`:
- Around line 2494-2524: In validate_trigger_delivery_target_against_registry,
stop discarding the underlying errors from RebornOutboundDeliveryTargetId::new
and resolve_outbound_delivery_target. Preserve the parse error when mapping
DeliveryTargetInvalid, and include or log the backend error in the
TriggerError::Backend path so the real cause is visible instead of a fixed
generic reason. Use the existing invalid helper, target_id parsing, and
registry.resolve_outbound_delivery_target match to keep the specific error
context attached to each failure.
In `@crates/ironclaw_reborn_composition/src/slack/slack_delivery.rs`:
- Around line 2900-2934: In resolve_per_trigger_delivery_route, error causes are
being discarded when parsing the target id and when the provider call fails.
Update the RebornOutboundDeliveryTargetId::new and
resolve_outbound_delivery_target paths to preserve or log the bound error before
converting to TriggeredRunDeliveryOutcomeKind::TargetUnavailable or Failed. Make
sure the provider/backend error and malformed-id case remain distinguishable for
diagnostics, using the resolve_per_trigger_delivery_route and
TriggeredRunDeliveryOutcomeKind flow to locate the fix.
In `@crates/ironclaw_triggers/src/lib.rs`:
- Around line 272-339: `TriggerDeliveryTargetId` is missing the canonical
validated-newtype shape. Update the type to use the shared validation path
through a reusable `validate(&str)` helper and keep construction in
`TriggerDeliveryTargetId::new` and `TryFrom<String>`, then add `#[serde(try_from
= "String")]` so serde uses the existing validation instead of a manual
`Deserialize` impl. Also add the missing `into_inner()` boundary method
alongside `as_str()` and `AsRef<str>`, and remove the custom `Deserialize`
implementation while keeping `Serialize` as-is.
In `@skills/routine-advisor/SKILL.md`:
- Around line 77-93: The routine guidance is still telling the model to use the
user-wide outbound delivery default for a specific routine, which can route
results to the wrong channel. Update the “Creating Routines” and “Delivering
Results” guidance in SKILL.md so `builtin__trigger_create` explicitly carries
the per-trigger `delivery_target_id`, and make it clear that
`builtin__outbound_delivery_target_set` is only for setting the global default,
not for routing one routine. Keep the prompt focused on
`builtin__trigger_create`, `builtin__trigger_list`, and the delivery target
contract reflected by `TRIGGER_CREATE_DESCRIPTION`.
---
Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/slack/slack_host_beta.rs`:
- Around line 628-699: The `SlackOutboundTargetProviderConfig` setup is
duplicated between `build_triggered_run_delivery_hook_from_parts` and
`build_slack_host_beta_mounts`, so refactor both call sites to use the existing
shared helper `outbound_target_provider_config` instead of rebuilding the same
`tenant_id`/`agent_id`/`project_id`/`installation_id`/`team_id`/`configured_channel_routes`
mapping inline. Keep the helper as the single source of truth and have
`SlackHostBetaOutboundTargetProvider::new` receive its config from that shared
path so future `SlackHostBetaConfig` field changes don’t drift.
🪄 Autofix (Beta)
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: 7af61116-d6ed-47f5-8df4-ff81bcd253c7
⛔ Files ignored due to path filters (8)
crates/ironclaw_first_party_extensions/assets/slack/wasm/slack_user_tool.wasmis excluded by!**/*.wasm,!**/*.wasmtests/snapshots/golden_payload__context_surfacing.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__gated_turn_approve.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__greeting.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__image_attachment.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__multi_turn.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__parallel_tool_calls.snapis excluded by!**/*.snap,!tests/snapshots/**tests/snapshots/golden_payload__tool_call.snapis excluded by!**/*.snap,!tests/snapshots/**
📒 Files selected for processing (42)
crates/ironclaw_conversations/src/inbound.rscrates/ironclaw_first_party_extensions/assets/slack/manifest.tomlcrates/ironclaw_first_party_extensions/assets/slack/prompts/slack/send_message.mdcrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_user_info.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/list_conversations.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rscrates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/lib.rscrates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/types.rscrates/ironclaw_host_runtime/src/first_party_tools/schemas.rscrates/ironclaw_host_runtime/src/first_party_tools/trigger_management.rscrates/ironclaw_host_runtime/tests/first_party_builtin_tools.rscrates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rscrates/ironclaw_reborn_composition/src/automation/facade/tests.rscrates/ironclaw_reborn_composition/src/automation/facade/tests/resolver_tests.rscrates/ironclaw_reborn_composition/src/automation/trigger_poller.rscrates/ironclaw_reborn_composition/src/automation/trigger_poller_trusted_submit.rscrates/ironclaw_reborn_composition/src/extension_host/available_extensions.rscrates/ironclaw_reborn_composition/src/extension_host/bundled_skills.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/outbound/outbound_delivery_capability_surface.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rscrates/ironclaw_reborn_composition/src/slack/slack_delivery.rscrates/ironclaw_reborn_composition/src/slack/slack_host_beta.rscrates/ironclaw_reborn_composition/src/slack/slack_serve/e2e_tests.rscrates/ironclaw_reborn_composition/tests/trigger_poller_e2e.rscrates/ironclaw_reborn_composition/tests/trigger_webui_timeline_e2e.rscrates/ironclaw_reborn_migration/src/convert/automations.rscrates/ironclaw_triggers/src/lib.rscrates/ironclaw_triggers/src/libsql.rscrates/ironclaw_triggers/src/postgres.rscrates/ironclaw_triggers/src/worker/tests.rscrates/ironclaw_triggers/tests/repository_contract.rscrates/ironclaw_turns/src/run_profile/runtime_context.rscrates/ironclaw_turns/tests/agent_loop_host_contract.rsskills/delegation/SKILL.mdskills/routine-advisor/SKILL.mdtests/integration/group_triggers/main.rstests/integration/group_triggers/scenario_delivery_target_fail_closed.rstests/integration/support/triggered_submit.rstests/integration/triggered_delivery_outcome.rs
…Slack name lookups per review Two contract tests still pinned the pre-per-trigger-routing wording (tool_surface_contract's trigger_create description/prompt-schema phrases and loop_driver_host's NoneSet warning line); they now pin the new contract, including the delivery_target_id schema description. Review follow-up (gemini): resolve_user_display_names now caps users.info lookups at 25 distinct ids per read, first-seen order, with over-budget authors keeping raw ids (same degraded shape as a failed lookup). Pinned by a new WASM runtime contract test driving a 30-author history through the rebuilt artifact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 9, 2026 20:03
Destroyed
…l newtype serde, per-trigger routing in routine-advisor - validate_trigger_delivery_target_against_registry and resolve_per_trigger_delivery_route now log the bound error before mapping to sanitized reasons/outcomes (error-handling rule: no discarded error bindings), so provider outages and malformed ids are distinguishable in logs. - TriggerDeliveryTargetId follows the canonical validated-newtype template: serde(try_from = "String") + derived Serialize replace the manual impls; adds into_inner() and From<Id> for String. - routine-advisor teaches delivery_target_id on builtin__trigger_create as the per-routine routing path (set remains the user-wide default), pinned by the embedded-skills gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 9, 2026 20:04
Destroyed
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.08% — 285861 / 336009 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
BenKurrek
added a commit
that referenced
this pull request
Jul 9, 2026
… names-not-ids, per-trigger routing) Four live probe cases for the Slack automation failure classes reported in production, designed to run RED against servers without the #5898 fixes and green after: - qa_9a_slack_connect: standard Slack connect preflight for the shard. - qa_9b_routine_dm_delivery_exactly_once: creates a routine with the historically bug-triggering "send me the result in a Slack DM" phrasing, then requires the delivery marker in the requester's DM EXACTLY once (after a 60s grace re-scan) and only from the bot identity — catching wrong-conversation delivery (timeout), duplicate delivery, and user-identity self-delivery. - qa_9c_slack_digest_names_not_ids: asks for a DM digest and fails if the answer leaks raw U... Slack user ids instead of display names. - qa_9d_routine_per_trigger_delivery_target: requires the routine to be created with its own delivery_target_id persisted on the trigger record and delivered through it (fails on servers without per-trigger routing). _slack_history_contains_marker now scans the full window and reports marker match counts split by bot/human authorship (existing callers keep their semantics). The QA 9 shard is dispatch_only so the 3-hourly cron does not page while the probes are expected-red on main; #5898 flips it into the cron rotation together with the fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek
added a commit
that referenced
this pull request
Jul 9, 2026
… names-not-ids, per-trigger routing) Four live probe cases for the Slack automation failure classes reported in production, designed to run RED against servers without the #5898 fixes and green after: - qa_9a_slack_connect: standard Slack connect preflight for the shard. - qa_9b_routine_dm_delivery_exactly_once: creates a routine with the historically bug-triggering "send me the result in a Slack DM" phrasing, then requires the delivery marker in the requester's DM EXACTLY once (after a 60s grace re-scan) and only from the bot identity — catching wrong-conversation delivery (timeout), duplicate delivery, and user-identity self-delivery. - qa_9c_slack_digest_names_not_ids: asks for a DM digest and fails if the answer leaks raw U... Slack user ids instead of display names. - qa_9d_routine_per_trigger_delivery_target: requires the routine to be created with its own delivery_target_id persisted on the trigger record and delivered through it (fails on servers without per-trigger routing). _slack_history_contains_marker now scans the full window and reports marker match counts split by bot/human authorship (existing callers keep their semantics). The QA 9 shard is dispatch_only so the 3-hourly cron does not page while the probes are expected-red on main; #5898 flips it into the cron rotation together with the fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek
added a commit
that referenced
this pull request
Jul 9, 2026
… names-not-ids, per-trigger routing) Four live probe cases for the Slack automation failure classes reported in production, designed to run RED against servers without the #5898 fixes and green after: - qa_9a_slack_connect: standard Slack connect preflight for the shard. - qa_9b_routine_dm_delivery_exactly_once: creates a routine with the historically bug-triggering "send me the result in a Slack DM" phrasing, then requires the delivery marker in the requester's DM EXACTLY once (after a 60s grace re-scan) and only from the bot identity — catching wrong-conversation delivery (timeout), duplicate delivery, and user-identity self-delivery. - qa_9c_slack_digest_names_not_ids: asks for a DM digest and fails if the answer leaks raw U... Slack user ids instead of display names. - qa_9d_routine_per_trigger_delivery_target: requires the routine to be created with its own delivery_target_id persisted on the trigger record and delivered through it (fails on servers without per-trigger routing). _slack_history_contains_marker now scans the full window and reports marker match counts split by bot/human authorship (existing callers keep their semantics). The QA 9 shard is dispatch_only so the 3-hourly cron does not page while the probes are expected-red on main; #5898 flips it into the cron rotation together with the fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n ids A live QA fire produced a duplicate user-identity delivery: the creating model set delivery_target_id correctly AND pinned the requester's own DM into the trigger prompt as if it were a third-party recipient, so the fire executed slack.send_message under the user's identity on top of the host's bot delivery. Close the laundering path at both layers: - trigger_create description + prompt schema now state that receiving results is delivery routing (delivery_target_id), never a prompt step, even phrased as 'send me the result' with a pinned conversation id. - ScheduledTrigger origin line now tells the fire that a task step sending the result to the trigger creator's own conversation is already covered by automatic delivery and must be skipped. - routine-advisor skill teaches the same framing. Regression tests: trigger_create_description_teaches_task_only_prompt_and_host_owned_delivery, tool_surface_contract trigger_create pins, renders_origin_scheduled_trigger, bundled_reborn_skills routine-advisor pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 9, 2026 22:10
Destroyed
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_host_runtime/src/first_party_tools/schemas.rs (1)
487-494: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuardrail wording duplicated across 3 independently-tested artifacts — drift risk.
The "receiving results is routing, never a prompt step" guardrail here is a near-duplicate (not identical) of
TRIGGER_CREATE_DESCRIPTIONintrigger_management.rsand theroutine-advisorSKILL.mdprompt-parameter guidance. Each copy is validated by a different hardcoded substring assertion, so a future edit to one copy can pass its own test while silently drifting from the other two — the exact failure mode this PR's "laundering guard" follow-up commit had to patch after a live QA incident.Given the repo's stance on single-source-of-truth for shared contracts (owner-type rule for domain values), consider extracting the shared guardrail sentence into one
constreused by both Rust call sites (schemas.rs+trigger_management.rsare in the same crate), with a shared test asserting byte-identical inclusion, rather than three copies each independently worded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_host_runtime/src/first_party_tools/schemas.rs` around lines 487 - 494, The prompt-routing guardrail text is duplicated across multiple independently tested locations, so edits can drift unnoticed. Extract the shared “receiving results is routing, never a prompt step” wording into a single constant or shared helper used by both `schemas.rs` and `trigger_management.rs`, and align the `routine-advisor` `SKILL.md` copy to the same source of truth. Add or update a shared test around the relevant symbols (`prompt`, `delivery_target_id`, and `TRIGGER_CREATE_DESCRIPTION`) to assert the exact byte-identical guardrail text is reused everywhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_host_runtime/src/first_party_tools/schemas.rs`:
- Around line 487-494: The prompt-routing guardrail text is duplicated across
multiple independently tested locations, so edits can drift unnoticed. Extract
the shared “receiving results is routing, never a prompt step” wording into a
single constant or shared helper used by both `schemas.rs` and
`trigger_management.rs`, and align the `routine-advisor` `SKILL.md` copy to the
same source of truth. Add or update a shared test around the relevant symbols
(`prompt`, `delivery_target_id`, and `TRIGGER_CREATE_DESCRIPTION`) to assert the
exact byte-identical guardrail text is reused everywhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b3cdf41-3f31-4d57-8a8d-5e22ef80ecc0
📒 Files selected for processing (6)
crates/ironclaw_host_runtime/src/first_party_tools/schemas.rscrates/ironclaw_host_runtime/src/first_party_tools/trigger_management.rscrates/ironclaw_host_runtime/tests/tool_surface_contract.rscrates/ironclaw_reborn_composition/src/extension_host/bundled_skills.rscrates/ironclaw_turns/src/run_profile/runtime_context.rsskills/routine-advisor/SKILL.md
BenKurrek
added a commit
that referenced
this pull request
Jul 9, 2026
… names-not-ids, per-trigger routing) Four live probe cases for the Slack automation failure classes reported in production, designed to run RED against servers without the #5898 fixes and green after: - qa_9a_slack_connect: standard Slack connect preflight for the shard. - qa_9b_routine_dm_delivery_exactly_once: creates a routine with the historically bug-triggering "send me the result in a Slack DM" phrasing, then requires the delivery marker in the requester's DM EXACTLY once (after a 60s grace re-scan) and only from the bot identity — catching wrong-conversation delivery (timeout), duplicate delivery, and user-identity self-delivery. - qa_9c_slack_digest_names_not_ids: asks for a DM digest and fails if the answer leaks raw U... Slack user ids instead of display names. - qa_9d_routine_per_trigger_delivery_target: requires the routine to be created with its own delivery_target_id persisted on the trigger record and delivered through it (fails on servers without per-trigger routing). _slack_history_contains_marker now scans the full window and reports marker match counts split by bot/human authorship (existing callers keep their semantics). The QA 9 shard is dispatch_only so the 3-hourly cron does not page while the probes are expected-red on main; #5898 flips it into the cron rotation together with the fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🚅 Deployed to the ironclaw-pr-5898 environment in ironclaw-ci-preview
|
Collaborator
Author
Live canary green: run 29055786290 — 4/4 QA-9 automation probes pass on this branchSame probe code (#5899 @ f1db72d, cherry-picked) that is red against main (29051121450: 9b caught a live user-identity wrong-DM delivery; 9d deterministic column-missing red — evidence on #5899).
The first AFTER attempt (pre-hardening probes, run 29050258818) surfaced a real residual gap via home-DB forensics: the creating model set Follow-up after #5899 merges: drop |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 10, 2026 00:00
Destroyed
BenKurrek
added a commit
that referenced
this pull request
Jul 10, 2026
… names-not-ids, per-trigger routing) Four live probe cases for the Slack automation failure classes reported in production, designed to run RED against servers without the #5898 fixes and green after: - qa_9a_slack_connect: standard Slack connect preflight for the shard. - qa_9b_routine_dm_delivery_exactly_once: creates a routine with the historically bug-triggering "send me the result in a Slack DM" phrasing, then requires the delivery marker in the requester's DM EXACTLY once (after a 60s grace re-scan) and only from the bot identity — catching wrong-conversation delivery (timeout), duplicate delivery, and user-identity self-delivery. - qa_9c_slack_digest_names_not_ids: asks for a DM digest and fails if the answer leaks raw U... Slack user ids instead of display names. - qa_9d_routine_per_trigger_delivery_target: requires the routine to be created with its own delivery_target_id persisted on the trigger record and delivered through it (fails on servers without per-trigger routing). _slack_history_contains_marker now scans the full window and reports marker match counts split by bot/human authorship (existing callers keep their semantics). The QA 9 shard is dispatch_only so the 3-hourly cron does not page while the probes are expected-red on main; #5898 flips it into the cron rotation together with the fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ads, membership, entity resolution (#5904) * test(slack): RED contract tests for connected-identity marking and whoami Failing-first tests from the four-lens Slack tool audit: history messages must carry is_current_user + result-level current_user_id (auth.test derived, best-effort absent on failure), and slack.whoami must resolve the connected account. Implementation follows in this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(slack): mark connected-account messages and add slack.whoami Identity-attribution fix for the Slack personal tool ("says George is off but it's actually me"): get_conversation_history now resolves the connected account once via auth.test (best-effort — a failing auth.test never breaks the read) and marks each message with is_current_user: Some(user == self) plus a result-level current_user_id, so the model attributes the requester's own words to the requester. New slack.whoami capability (auth.test + best-effort users.info display name) lets the model ask "who am I on Slack?" directly; its description tells the model to call it before answering anything that depends on which messages are the requester's own. The auth.test identity call does not count against the 25-lookup users.info budget. Composition now also packages the slack output schemas that manifest output_schema_ref already pointed at (previously unpackaged), and the asset-refs pin covers slack under slack-v2-host-beta. Turns green the RED contract pins from the previous commit: slack_whoami_resolves_connected_identity and the identity assertions in slack_history_output_carries_display_names_alongside_raw_user_ids / slack_history_read_survives_users_info_failure_without_names (cargo test -p ironclaw_host_runtime --test github_wasm_runtime_contract). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(slack): surface status, timezone, and title in get_user_info "Is George around?" needs Slack presence signals, not just names. get_user_info now returns tz, tz_label, title, status_text, status_emoji, and status_expiration from users.info (absent when missing; a 0 status_expiration means "does not expire" and is omitted). Manifest description advertises the presence-relevant fields so the model reaches for them instead of guessing. Regression pin: slack_get_user_info_surfaces_status_and_timezone (users.info fixture with "On vacation until July 20" must surface status_text, tz, and title through the full invoke_capability path). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(slack,host): carry structured guest error codes to the model The slack guest returned Err("Slack API error: {code}") strings, which the host's WASM error path erased to the kind's generic sentence ("the tool operation failed") — channel_not_found, missing_scope, and ratelimited all looked identical and unactionable to the model. Guest side: slack_api_call now emits the host's structured guest-error contract ({code, kind} JSON, the shape wasm_execution.rs already parses) for Slack ok:false codes and HTTP-layer failures: - missing_scope / not_authed / invalid_auth / account_inactive / token_revoked -> auth_required (gates on re-auth instead of failing) - channel_not_found / user_not_found / invalid_* -> input (model-fixable, classified InvalidInput, no retry burn) - ratelimited / HTTP 429 -> client (the host's {code, kind} shape has no retry-after channel, so Retry-After cannot ride along) - everything else -> operation_failed Codes are reduced to snake_case identifiers before emission. Enrichment stays best-effort: users.info/auth.test failures are still swallowed inside the guest, so no read fails because of them. Host side: complete the half-built pipe — the parser deserialized StructuredWasmGuestError but dropped `code` (#[allow(dead_code)]). DispatchError::Wasm now carries safe_summary (mirroring the FirstParty variant's existing channel), wasm_guest_dispatch_error fills it with the sanitized code ("provider error code: {code}"), and the capabilities-layer conversion forwards it, so the model-visible failure message keeps the actionable cause. The summary is still re-validated by LoopSafeSummary before rendering. Construction sites gain safe_summary: None; Debug output prints only the kind, matching FirstParty. Regression pins: - slack_channel_not_found_surfaces_code_in_model_visible_failure (full invoke_capability drive: Failed outcome, InvalidInput kind, message contains "channel_not_found") - wasm_guest_dispatch_error_carries_sanitized_structured_code (code extraction, hostile-code sanitation, legacy strings stay summary-less) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(slack): add slack.get_thread_replies and surface reply_count Slack conversation history returns only thread PARENTS — the replies live behind conversations.replies, so thread content was invisible to the model and it summarized channels while missing every discussion. New slack.get_thread_replies capability (channel + parent thread_ts + limit <= 999) with the same enrichment contract as history: resolved user_display_name per message, connected-account is_current_user marking, and result-level current_user_id (shared enriched_history_result post-processing). History messages now carry reply_count, and both the history description and output schema state that replies are NOT in history — fetch them with slack.get_thread_replies. Regression pins: slack_thread_replies_resolve_names_and_mark_connected_account (scripted conversations.replies fixture through invoke_capability, asserting the channel+ts egress and enrichment) and reply_count assertions extended into slack_history_output_carries_display_names_alongside_raw_user_ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(slack): membership marking, pagination, and search enrichment Three read-surface accuracy fixes: - list_conversations: Slack lists channels you can SEE, not only ones you belong to, and the old description claimed otherwise. Each channel now carries is_member (absent for DMs — no membership axis), the result carries next_cursor, an optional cursor input pages through, and the description/schema wording is accurate ("visible to you; is_member marks membership"). - get_conversation_history: limit is clamped to Slack's real maximum of 999 (Slack rejects 1000; the input schema previously advertised max 1000) and the model-visible description now documents newest-first ordering and has_more paging (latest = oldest returned ts) — descriptions and input schemas are the only guidance that reaches the model. - search_messages: matches now resolve author display names exactly like history (best-effort users.info, shared 25-lookup budget), surface thread_ts on threaded hits for get_thread_replies follow-up, and accept a page input passed through to Slack paging. Regression pins (all driven through invoke_capability): slack_list_conversations_surfaces_membership_and_pagination, slack_history_limit_is_clamped_to_slack_maximum, slack_search_matches_carry_display_names_thread_ts_and_page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(slack): honest model-visible descriptions for send_message and get_user_info Three honesty fixes to the only guidance the model actually sees: - get_user_info claimed "email when visible", but the slack_personal OAuth grant has no users:read.email scope, so Slack never returns an email through this tool. Removed the claim (and the dead email field/output-schema property) rather than adding the scope: the setup scopes are pinned as the union shared by every Slack tool (SLACK_PERSONAL_OAUTH_SETUP_SCOPES), and widening them forces every existing account through a scope-upgrade re-consent flow that does not exist yet (tracked in #5669) — not a trivial manifest extension. - send_message promised the run's final reply is "delivered automatically to the requesting user", but a per-trigger delivery_target_id can route it elsewhere; it now says "to the configured outbound delivery target". - Outbound mentions: to notify someone the text must contain <@U…> with a real user id — a plain @name notifies no one. Documented in the send_message description and the text input-schema field. Regression pins (composition, slack-v2-host-beta): slack_get_user_info_description_matches_grantable_scopes and the delivery-target + mention-encoding assertions extended into slack_send_message_description_states_host_owned_final_reply_delivery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(slack): resolve in-text mentions/entities and post sends as the user Two live-canary reds (run 29065062350, canary/automation-probes-on-5904): qa_10i (inbound entity hygiene): message text reached the model with raw Slack control tokens, so replies leaked raw user ids while explaining who <@U…> was. History, thread replies, and search match text now resolve <@U…> / <@U…|label> mentions to @display Name through the SAME users.info cache and 25-lookup budget as author enrichment (in-text ids count against the budget; unresolved tokens stay as-is — never fabricated), rewrite <#C…|name> channel refs to #name, leave links and other tokens untouched, and decode &/</> AFTER token rewriting so literal <@U…> text never becomes a live token. History/search descriptions now state mentions arrive pre-resolved. qa_10f (mention posted from the wrong identity): the probe's forensics show the model did everything right — installed slack, called slack.whoami, then slack.send_message with a correctly encoded <@U0BDJFDEJRY> mention — yet the posted message (ts 1783651853.274969) carried bot_id B0BFW0DKNQY / bot_profile "IronClaw Reborn PR5362": chat.postMessage with a CLASSIC Slack app user token defaults to as_user=false, attributing the post to the APP instead of the connected user (Slack legacy authorship; the probe's own personal-token seeds carry the same bot_id, it just never checks them). send_message now pins as_user=true; granular apps reject the legacy flag with as_user_not_supported — their user-token posts are always user-authored — so the send retries exactly once without it. Regression pins (invoke_capability tier): - slack_history_text_resolves_in_text_entities_to_display_names (+ in-text assertions in the replies and search tests; shared-budget dedup pinned at 3 lookups) - slack_send_message_posts_as_the_connected_user (as_user=true in the chat.postMessage body) - slack_send_message_retries_without_as_user_for_granular_apps (SequencedSlackEgress: as_user_not_supported then ok; retry drops the flag, exactly two calls) The services fixture macro now takes a scope-set parameter so send_message tests resolve the chat:write credential scopes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(slack): forbid raw ids in replies across all read descriptions Live canary qa_10i iteration: with in-text mentions now pre-resolved, the residual leak was the model volunteering an id it got from the structured fields ("Benji (the current user, display name Benji, user id U0…)"). Descriptions are the only model-visible guidance, so every slack read surface (search, list, history, thread replies, user info, whoami) now carries the imperative rule: raw Slack ids (U…/W…/C…/D…) are for tool calls only — never include one in a reply, not even in parentheses; refer to people and channels by name. Regression pin: slack_read_descriptions_forbid_raw_ids_in_replies (composition, slack-v2-host-beta) requires the rule on all six read capabilities. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 10, 2026 12:45
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5898
July 10, 2026 12:49
Destroyed
BenKurrek
changed the title
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 `@crates/ironclaw_capabilities/src/error.rs`:
- Around line 152-155: Extend the conversion-boundary test that exercises
dispatch errors through the caller of dispatch_error_safe_summary to use a WASM
error with a populated sanitized safe_summary, then assert
CapabilityInvocationError::Dispatch preserves and exposes that exact summary.
Keep the existing None case as appropriate and verify the model-visible error
contract rather than testing the helper in isolation.
In
`@crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.input.v1.json`:
- Around line 6-12: Add cursor-based pagination to the get_thread_replies schema
and its WASM wrapper: accept an optional cursor input, expose the returned
next_cursor token alongside has_more, and pass the token through on subsequent
requests. Follow the existing pagination shape used by Slack history/list
implementations, updating the relevant input/output schemas and wrapper symbols
while preserving additionalProperties: false.
In `@crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rs`:
- Around line 99-122: Remove the raw response body from the Debug log in the
Slack API error-handling block, retaining only the resource and HTTP status.
Update the serde_json parsing in the response handling to preserve safe parser
context in the mapped structured error, without exposing response content; use
the existing structured_error helper and ensure no unredacted body data reaches
logs or public errors.
In `@crates/ironclaw_host_runtime/src/services/wasm_execution.rs`:
- Around line 427-438: Update wasm_guest_dispatch_error to deserialize
StructuredWasmGuestError once and retain the parse result or error details
instead of silently discarding failures through wasm_guest_error_code’s .ok()?;
keep the legacy plain-string fallback explicit. Refactor wasm_guest_error_code
to accept the already-parsed payload or otherwise remove its redundant parsing
while preserving the existing code sanitization and length limit.
- Around line 411-415: Update the Runtime branch handling in the WasmGuestError
conversion to avoid interpolating the guest-controlled value from
wasm_guest_error_code into safe_summary; replace it with an allowlisted mapping
to host-authored summaries for recognized codes and return None for unknown
codes.
- Around line 1462-1495: Add caller-level coverage in the existing WASM dispatch
tests by invoking execute_prepared_wasm with a guest error containing a
structured code, then assert the returned DispatchError::Wasm.safe_summary
equals the sanitized provider error summary. Retain the existing direct
wasm_guest_dispatch_error assertions, and use the test’s available
capability/runtime setup to exercise the actual dispatcher boundary.
🪄 Autofix (Beta)
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: 006ea6e6-83a7-4728-b046-8654dcf8e5a1
⛔ Files ignored due to path filters (1)
crates/ironclaw_first_party_extensions/assets/slack/wasm/slack_user_tool.wasmis excluded by!**/*.wasm,!**/*.wasm
📒 Files selected for processing (42)
crates/ironclaw_capabilities/src/error.rscrates/ironclaw_capabilities/tests/capability_host_dispatcher_integration.rscrates/ironclaw_capabilities/tests/capability_host_run_state_contract.rscrates/ironclaw_conversations/src/inbound.rscrates/ironclaw_dispatcher/tests/dispatch_contract.rscrates/ironclaw_dispatcher/tests/event_dispatch_contract.rscrates/ironclaw_dispatcher/tests/runtime_dispatcher_integration.rscrates/ironclaw_dispatcher/tests/vertical_slice_contract.rscrates/ironclaw_first_party_extensions/assets/slack/manifest.tomlcrates/ironclaw_first_party_extensions/assets/slack/prompts/slack/get_thread_replies.mdcrates/ironclaw_first_party_extensions/assets/slack/prompts/slack/whoami.mdcrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_user_info.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/list_conversations.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/list_conversations.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/search_messages.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/send_message.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/whoami.input.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/schemas/slack/whoami.output.v1.jsoncrates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rscrates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/lib.rscrates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/types.rscrates/ironclaw_host_api/src/dispatch.rscrates/ironclaw_host_api/tests/host_api_contract.rscrates/ironclaw_host_runtime/src/services/process_executor.rscrates/ironclaw_host_runtime/src/services/runtime_adapters.rscrates/ironclaw_host_runtime/src/services/tests.rscrates/ironclaw_host_runtime/src/services/wasm_execution.rscrates/ironclaw_host_runtime/tests/extension_v2_lifecycle_e2e.rscrates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rscrates/ironclaw_host_runtime/tests/obligation_services_composition_contract.rscrates/ironclaw_host_runtime/tests/reborn_invoke_vertical_slice.rscrates/ironclaw_reborn_composition/src/extension_host/available_extensions.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/tests/trigger_poller_e2e.rscrates/ironclaw_runner/tests/loop_driver_host.rscrates/ironclaw_wasm/tests/wasm_dispatch_integration.rstests/integration/support/triggered_submit.rs
Comment on lines
152
to
+155
| fn dispatch_error_safe_summary(error: &DispatchError) -> Option<String> { | ||
| match error { | ||
| DispatchError::FirstParty { safe_summary, .. } => safe_summary.clone(), | ||
| DispatchError::FirstParty { safe_summary, .. } | ||
| | DispatchError::Wasm { safe_summary, .. } => safe_summary.clone(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover populated WASM summaries at the conversion boundary.
Line 155 is the new forwarding path, but the tests only exercise safe_summary: None. Assert that a populated sanitized WASM summary reaches CapabilityInvocationError::Dispatch; otherwise this model-visible contract can regress unnoticed.
Proposed test
+ #[test]
+ fn from_dispatch_error_preserves_wasm_safe_summary() {
+ let err = CapabilityInvocationError::from(DispatchError::Wasm {
+ kind: RuntimeDispatchErrorKind::InputEncode,
+ safe_summary: Some("provider error code: channel_not_found".to_string()),
+ });
+
+ match err {
+ CapabilityInvocationError::Dispatch { safe_summary, .. } => {
+ assert_eq!(
+ safe_summary.as_deref(),
+ Some("provider error code: channel_not_found")
+ );
+ }
+ other => panic!("expected Dispatch variant, got {other:?}"),
+ }
+ }As per path instructions, “Test through the caller,” so cover the populated value at this conversion boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_capabilities/src/error.rs` around lines 152 - 155, Extend the
conversion-boundary test that exercises dispatch errors through the caller of
dispatch_error_safe_summary to use a WASM error with a populated sanitized
safe_summary, then assert CapabilityInvocationError::Dispatch preserves and
exposes that exact summary. Keep the existing None case as appropriate and
verify the model-visible error contract rather than testing the helper in
isolation.
Source: Path instructions
Comment on lines
+6
to
+12
| "required": ["channel", "thread_ts"], | ||
| "properties": { | ||
| "channel": { "type": "string", "description": "Conversation ID the thread lives in (C... for a channel, D... for a DM)." }, | ||
| "thread_ts": { "type": "string", "description": "The thread parent message's ts (also present as thread_ts on replies)." }, | ||
| "limit": { "type": "integer", "minimum": 1, "maximum": 999, "description": "Maximum messages to return (default 50, max 999)." } | ||
| }, | ||
| "additionalProperties": false |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the WASM implementation supports cursor-based paging for conversations.replies.
rg -nC3 'conversations\.replies|thread_ts|cursor' crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rs --type rust | head -80Repository: nearai/ironclaw
Length of output: 3035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the thread-replies input/output schemas and the analogous history schema.
for f in \
crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.input.v1.json \
crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.output.v1.json \
crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.input.v1.json \
crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.output.v1.json
do
echo "===== $f ====="
wc -l "$f"
done
echo
echo "===== get_thread_replies.output.v1.json ====="
cat -n crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.output.v1.json
echo
echo "===== get_conversation_history.input.v1.json ====="
cat -n crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.input.v1.json
echo
echo "===== get_conversation_history.output.v1.json ====="
cat -n crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.output.v1.json
echo
echo "===== pager-related references ====="
rg -n 'has_more|next_cursor|cursor|conversations\.replies|conversations\.history' crates/ironclaw_first_party_extensions/assets/slack/schemas/slack crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rsRepository: nearai/ironclaw
Length of output: 9287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Follow the pager path in the wasm implementation and schemas.
ast-grep outline crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rs --view expanded
echo
echo "===== focused pager excerpts ====="
sed -n '300,430p' crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rs
echo
echo "===== schema excerpts ====="
sed -n '1,220p' crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.output.v1.json
echo
sed -n '1,220p' crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.input.v1.json
echo
sed -n '1,260p' crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_conversation_history.output.v1.jsonRepository: nearai/ironclaw
Length of output: 10543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.output.v1.json
printf '\n---\n'
cat -n crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.input.v1.json
printf '\n---\n'
rg -n 'has_more|next_cursor|cursor|conversations\.replies' crates/ironclaw_first_party_extensions/assets/slack/schemas/slack crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rsRepository: nearai/ironclaw
Length of output: 5818
Add a paging token round-trip for thread replies
crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.input.v1.json only accepts channel, thread_ts, and limit, while the output only exposes has_more. With additionalProperties: false and no cursor/next_cursor path in the wasm wrapper, callers can’t fetch the next page once a thread exceeds one response. Mirror the history/list pagination shape here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/ironclaw_first_party_extensions/assets/slack/schemas/slack/get_thread_replies.input.v1.json`
around lines 6 - 12, Add cursor-based pagination to the get_thread_replies
schema and its WASM wrapper: accept an optional cursor input, expose the
returned next_cursor token alongside has_more, and pass the token through on
subsequent requests. Follow the existing pagination shape used by Slack
history/list implementations, updating the relevant input/output schemas and
wrapper symbols while preserving additionalProperties: false.
Comment on lines
+99
to
+122
| // The raw body may carry private data; keep it in debug logs only. | ||
| host::log( | ||
| host::LogLevel::Debug, | ||
| &format!( | ||
| "Slack API {} returned status {}: {}", | ||
| resource, | ||
| response.status, | ||
| String::from_utf8_lossy(&response.body) | ||
| ), | ||
| ); | ||
| // Slack signals rate limiting at the HTTP layer (429 + Retry-After); | ||
| // the host's structured error shape carries only {code, kind}, so the | ||
| // Retry-After value cannot ride along. | ||
| if response.status == 429 { | ||
| return Err(structured_error("ratelimited", "client")); | ||
| } | ||
| return Err(structured_error( | ||
| &format!("http_status_{}", response.status), | ||
| "operation_failed", | ||
| )); | ||
| } | ||
|
|
||
| let parsed: serde_json::Value = serde_json::from_slice(&response.body) | ||
| .map_err(|e| format!("Failed to parse response: {}", e))?; | ||
| .map_err(|_| structured_error("invalid_json_response", "operation_failed"))?; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log raw Slack error bodies; retain only safe parse context.
The response body is explicitly acknowledged as potentially private, but is emitted verbatim to host logs. Meanwhile, the JSON parse error is discarded. Log the status/resource and parser location instead—never the body.
Proposed fix
- "Slack API {} returned status {}: {}",
+ "Slack API {} returned status {}",
resource,
response.status,
- String::from_utf8_lossy(&response.body)
),
);
@@
- let parsed: serde_json::Value = serde_json::from_slice(&response.body)
- .map_err(|_| structured_error("invalid_json_response", "operation_failed"))?;
+ let parsed: serde_json::Value = serde_json::from_slice(&response.body).map_err(|error| {
+ host::log(
+ host::LogLevel::Debug,
+ &format!("Slack API {} returned invalid JSON: {}", resource, error),
+ );
+ structured_error("invalid_json_response", "operation_failed")
+ })?;As per coding guidelines, mapped errors must retain context and unredacted user content must not be exposed across public surfaces.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // The raw body may carry private data; keep it in debug logs only. | |
| host::log( | |
| host::LogLevel::Debug, | |
| &format!( | |
| "Slack API {} returned status {}: {}", | |
| resource, | |
| response.status, | |
| String::from_utf8_lossy(&response.body) | |
| ), | |
| ); | |
| // Slack signals rate limiting at the HTTP layer (429 + Retry-After); | |
| // the host's structured error shape carries only {code, kind}, so the | |
| // Retry-After value cannot ride along. | |
| if response.status == 429 { | |
| return Err(structured_error("ratelimited", "client")); | |
| } | |
| return Err(structured_error( | |
| &format!("http_status_{}", response.status), | |
| "operation_failed", | |
| )); | |
| } | |
| let parsed: serde_json::Value = serde_json::from_slice(&response.body) | |
| .map_err(|e| format!("Failed to parse response: {}", e))?; | |
| .map_err(|_| structured_error("invalid_json_response", "operation_failed"))?; | |
| // The raw body may carry private data; keep it in debug logs only. | |
| host::log( | |
| host::LogLevel::Debug, | |
| &format!( | |
| "Slack API {} returned status {}", | |
| resource, | |
| response.status, | |
| ), | |
| ); | |
| // Slack signals rate limiting at the HTTP layer (429 + Retry-After); | |
| // the host's structured error shape carries only {code, kind}, so the | |
| // Retry-After value cannot ride along. | |
| if response.status == 429 { | |
| return Err(structured_error("ratelimited", "client")); | |
| } | |
| return Err(structured_error( | |
| &format!("http_status_{}", response.status), | |
| "operation_failed", | |
| )); | |
| } | |
| let parsed: serde_json::Value = serde_json::from_slice(&response.body).map_err(|error| { | |
| host::log( | |
| host::LogLevel::Debug, | |
| &format!("Slack API {} returned invalid JSON: {}", resource, error), | |
| ); | |
| structured_error("invalid_json_response", "operation_failed") | |
| })?; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_first_party_extensions/assets/slack/wasm-src/src/api.rs`
around lines 99 - 122, Remove the raw response body from the Debug log in the
Slack API error-handling block, retaining only the resource and HTTP status.
Update the serde_json parsing in the response handling to preserve safe parser
context in the mapped structured error, without exposing response content; use
the existing structured_error helper and ensure no unredacted body data reaches
logs or public errors.
Source: Coding guidelines
Comment on lines
+411
to
+415
| WasmGuestErrorKind::Runtime(kind) => DispatchError::Wasm { | ||
| kind, | ||
| safe_summary: wasm_guest_error_code(error) | ||
| .map(|code| format!("provider error code: {code}")), | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not interpolate guest-controlled text into safe_summary.
code comes from the untrusted WASM payload and reaches the model-visible summary. Character filtering does not make it host-authored; use an allowlisted host-owned mapping or leave unknown codes unset.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_host_runtime/src/services/wasm_execution.rs` around lines 411
- 415, Update the Runtime branch handling in the WasmGuestError conversion to
avoid interpolating the guest-controlled value from wasm_guest_error_code into
safe_summary; replace it with an allowlisted mapping to host-authored summaries
for recognized codes and return None for unknown codes.
Source: Path instructions
Comment on lines
+427
to
+438
| fn wasm_guest_error_code(error: &str) -> Option<String> { | ||
| let payload = serde_json::from_str::<StructuredWasmGuestError>(error).ok()?; | ||
| let code: String = payload | ||
| .code | ||
| .chars() | ||
| .filter(|character| { | ||
| character.is_ascii_alphanumeric() || matches!(character, '_' | '-' | '.') | ||
| }) | ||
| .take(64) | ||
| .collect(); | ||
| (!code.is_empty()).then_some(code) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid dropping structured-payload parse errors.
.ok()? silently collapses deserialization failures into None. Parse the structured payload once in wasm_guest_dispatch_error and keep the legacy plain-string fallback explicit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_host_runtime/src/services/wasm_execution.rs` around lines 427
- 438, Update wasm_guest_dispatch_error to deserialize StructuredWasmGuestError
once and retain the parse result or error details instead of silently discarding
failures through wasm_guest_error_code’s .ok()?; keep the legacy plain-string
fallback explicit. Refactor wasm_guest_error_code to accept the already-parsed
payload or otherwise remove its redundant parsing while preserving the existing
code sanitization and length limit.
Source: Path instructions
Comment on lines
+1462
to
+1495
|
|
||
| /// A structured guest error's `code` must ride the dispatch error as a | ||
| /// sanitized safe summary so the model sees the actionable cause (e.g. | ||
| /// `channel_not_found`), while hostile codes are reduced to identifier | ||
| /// characters and legacy plain-string errors stay summary-less. | ||
| #[test] | ||
| fn wasm_guest_dispatch_error_carries_sanitized_structured_code() { | ||
| let capability = CapabilityId::new("slack.get_conversation_history").unwrap(); | ||
| match wasm_guest_dispatch_error( | ||
| r#"{"code":"channel_not_found","kind":"input"}"#, | ||
| &capability, | ||
| ) { | ||
| DispatchError::Wasm { kind, safe_summary } => { | ||
| assert_eq!(kind, RuntimeDispatchErrorKind::InputEncode); | ||
| assert_eq!( | ||
| safe_summary.as_deref(), | ||
| Some("provider error code: channel_not_found") | ||
| ); | ||
| } | ||
| other => panic!("expected Wasm dispatch error, got {other:?}"), | ||
| } | ||
|
|
||
| // Hostile codes are reduced to identifier characters, never free text. | ||
| assert_eq!( | ||
| wasm_guest_error_code(r#"{"code":"bad {} `code` <script>","kind":"input"}"#).as_deref(), | ||
| Some("badcodescript") | ||
| ); | ||
| // Legacy plain-string guest errors carry no structured code. | ||
| assert_eq!(wasm_guest_error_code("invalid_parameters"), None); | ||
| match wasm_guest_dispatch_error("invalid_parameters", &capability) { | ||
| DispatchError::Wasm { safe_summary, .. } => assert_eq!(safe_summary, None), | ||
| other => panic!("expected Wasm dispatch error, got {other:?}"), | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the new summary through the dispatch caller.
This test calls wasm_guest_dispatch_error directly; it does not verify that execute_prepared_wasm returns the populated DispatchError::Wasm.safe_summary. Add caller-level coverage at the dispatcher boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_host_runtime/src/services/wasm_execution.rs` around lines
1462 - 1495, Add caller-level coverage in the existing WASM dispatch tests by
invoking execute_prepared_wasm with a guest error containing a structured code,
then assert the returned DispatchError::Wasm.safe_summary equals the sanitized
provider error summary. Retain the existing direct wasm_guest_dispatch_error
assertions, and use the test’s available capability/runtime setup to exercise
the actual dispatcher boundary.
Source: Path instructions
pull Bot
pushed a commit
to Stars1233/ironclaw
that referenced
this pull request
Jul 10, 2026
…very + tool correctness (QA 9/10) (nearai#5899) * test(live-canary): add QA 9 automation delivery probes (exactly-once, names-not-ids, per-trigger routing) Four live probe cases for the Slack automation failure classes reported in production, designed to run RED against servers without the nearai#5898 fixes and green after: - qa_9a_slack_connect: standard Slack connect preflight for the shard. - qa_9b_routine_dm_delivery_exactly_once: creates a routine with the historically bug-triggering "send me the result in a Slack DM" phrasing, then requires the delivery marker in the requester's DM EXACTLY once (after a 60s grace re-scan) and only from the bot identity — catching wrong-conversation delivery (timeout), duplicate delivery, and user-identity self-delivery. - qa_9c_slack_digest_names_not_ids: asks for a DM digest and fails if the answer leaks raw U... Slack user ids instead of display names. - qa_9d_routine_per_trigger_delivery_target: requires the routine to be created with its own delivery_target_id persisted on the trigger record and delivered through it (fails on servers without per-trigger routing). _slack_history_contains_marker now scans the full window and reports marker match counts split by bot/human authorship (existing callers keep their semantics). The QA 9 shard is dispatch_only so the 3-hourly cron does not page while the probes are expected-red on main; nearai#5898 flips it into the cron rotation together with the fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): QA 9 fixes + probe hardening from self-review - Add the missing QA_SHEET_PROMPTS entry for qa_9a_slack_connect (the connect helper reads the sheet table; first dispatch failed on the KeyError). - Workspace-wide marker sweep via the personal token inside the exactly-once check: a stray delivery-marker copy in ANY conversation other than the expected DM is a hard failure (positive wrong-channel detection + user-identity strays outside the DM). Search-index lag can only under-report, so absence stays inconclusive. - Digest probe gains a positive arm: ground-truth DM counterpart names are read from the Slack API with the personal token and at least one must appear in the answer, so a cop-out reply cannot pass vacuously; the raw-id negative check is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): install Slack tools extension in 9b/9d for production parity Trace analysis of the first main-branch run showed the duplicate arm of qa_9b was structurally inert: the fired model's stored prompt DID carry the bug-triggering "deliver a Slack DM" phrasing, but the per-case home had no Slack tools extension installed, so slack__send_message did not exist on the fire-time surface and a user-identity duplicate was impossible regardless of model behavior. Production users who hit the bug had the extension installed. The creation prompts for 9b/9d now instruct the model to install/activate the (already connected) Slack extension first — 9c's trace proves live models do this reliably via the extension lifecycle tools — so the fire surface matches production. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): one-shot schedules for exactly-once probes; pure marker-stats fn with unit tests The recurring every-minute schedule re-posts the same delivery marker on the next fire, which the grace re-scan counted as a duplicate — observed live as "expected exactly once, found 2" on both sides. Exactly-once is only well-defined for a single fire, so qa_9b/qa_9d now create one-time routines (~90s out, UTC) via a schedule_instruction parameter on the shared helper; existing recurring cases are unchanged. The history scan is extracted into a pure _marker_match_stats function (per-match authorship + timestamps) with direct unit tests, plus a guard test pinning that the exactly-once probes stay one-shot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): assert the Slack-tools-surface precondition in exactly-once probes A pass on the duplicate arm is only meaningful when the fired model HAS a user-token send capability — the first main run passed vacuously because the per-case home had no Slack extension and nothing could self-send. The creation prompt already instructs installation; the probe now VERIFIES it via the server's own extensions API (GET /api/webchat/v2/extensions, package_ref.id "slack" active) and fails with a "probe precondition" error otherwise, so the vacuous-pass shape can never silently return. Guard test pins the flag on both exactly-once cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): harden QA 9 probes per adversarial audits Two independent adversarial audits (falsifiability + environment/contract) plus artifact forensics from the first before/after runs surfaced probe defects that could fabricate or mask verdicts. All fixed here: - Verify the PERSISTED schedule outcome (schedule_kind == 'once', exactly one record, fire time inside the delivery wait window) instead of trusting prompt wording — a cron trigger fabricates a duplicate red. - Answer timezone clarifying questions with UTC for UTC-pinned probes (the canned London follow-up could shift a one-shot fire ~1h out). - requires_slack_personal_auth on 9b/9d: the workspace sweep runs on the personal token; the mismatch gate must apply or the sweep searches the wrong workspace and passes blind. - Retry the exactly-once re-scan and the workspace sweep; a transient Slack API error must not read as 'found 0' or pass silently. Permanent token problems (missing_scope, invalid_auth) fail with a distinct probe-environment error instead of fail-opening forever. - Move all preconditions inside the case try (a transient HTTP failure fails the case, never crashes the shard) and guard every helper HTTP call; capability checks (delivery_target column) now run BEFORE the extension-surface check so pre-fix servers red deterministically. - _exc_text(): report repr() for empty-str exceptions (httpx/asyncio timeouts) — the observed empty-error failure mode in run 29050258818. - Drop the 'and Slack message' clause from delivery-probe creation prompts: it manufactured self-send pressure that punishes a correct server; 'send me the result in a Slack DM' (the production phrasing) is the discriminating ask. - qa_9d also asserts the user-wide default target was NOT rewritten (before/after preferences snapshot) and 16k reply excerpts keep the qa_9c raw-id scan from being truncation-blind; ground truth filters Slackbot/self/bots/deleted and matches name tokens. - dispatch_only shards skip on cases=all/default dispatches (bare /canary on unrelated PRs stays green) and 9b/9c/9d leave bare local default runs until promoted. Unit tests pin each behavior (schedule-outcome verification, spec gates, _exc_text, snapshot reader incl. column-missing, epoch parsing, raw-id shape, follow-up timezone override, default-target reader). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): address review — full-reply raw-id scan, paginated DM ground truth, word-boundary names - qa_9c scans the FULL in-memory assistant reply for raw user ids (an id early in a long digest escaped the truncated excerpt) and redacts raw ids from the persisted text_excerpt diagnostic; excerpts stay bounded at 2000 chars for artifacts. - _slack_personal_dm_counterpart_names follows response_metadata .next_cursor to exhaustion (page cap 10) before reporting checked=True; an incomplete scan reports checked=False instead of a verdict from one page. - Ground-truth name matching uses word boundaries per token so unrelated substrings cannot satisfy the positive arm. Review threads: gemini high (distinct trigger-record read diagnosis) and coderabbit helper-tests were already addressed in f1db72d. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): QA 10 Slack tool-correctness suite from four-lens audit Nine live probes pinning the audited failure classes, each with API-computed ground truth, nonce-exact assertions, and distinct precondition reds (no vacuous passes, no fuzzy matching): - 10a self-attribution: seeded own+bot messages; 'which are mine' (no-self-identity gap — the misattribution users reported) - 10b OOO status: users.profile.set fixture; status fields are currently dropped by get_user_info - 10c thread replies: seeded root+3 replies; no replies capability - 10d channel membership vs users.conversations ground truth - 10e error honesty: model must be able to quote channel_not_found (host currently erases Slack error codes) - 10f outbound mention encoding (<@U…>, not dead literal @name) - 10g last-message-I-sent (search-lag/self-identity class) - 10h email hallucination guard (users:read.email absent from grant) - 10i inbound raw-entity hygiene (<@U…> resolved to display names) Plus: qa_9c gains the raw conversation-id arm + excerpt redaction; optional AUTH_LIVE_SLACK_SECOND_USER_TOKEN plumbing for future second-human arms (strict precondition message, never silent); dispatch_only qa-10 shard; count pins 37→46, 8→9 shards; unit tests for every new pure helper and per-case source pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): re-anchor QA 10 seeding on the personal↔bot DM; address CodeRabbit review Live run 29062917993 proved the QA 10 seeding anchor wrong: qa_10a/c/f/g/h/i seeded and read via _slack_delivery_channel_id, which is the DM between the bot and the BOUND WEBCHAT user — the personal (xoxp) token's human is a different user who is not in that conversation, so every personal-token chat.postMessage/read there failed with channel_not_found. The six cases now anchor on the personal↔bot DM via _slack_personal_bot_dm_channel (bot-token auth.test → personal-token conversations.list types=im scan with guarded pagination → conversations.open fallback; success cached per ctx), failing red with the distinct "probe precondition failed: could not resolve the personal↔bot DM: …" message. qa_9* keep the delivery DM; qa_10b is self-scoped and unchanged. CodeRabbit review dispositions (PR nearai#5899): - run_live_qa.py:4885 raw conversation ids persisted in probe details — fixed: qa_9c/qa_10i persist leak COUNTS only (leaked_raw_user_id_count / leaked_raw_conversation_id_count); error messages carry counts, not ids. - run_live_qa.py:5586 failed status restore left qa_10b green — fixed: a failed users.profile.set restore now sets success=False with a distinct "canary status restore failed" error and records status_restore_error. - run_live_qa.py:5854 any <@U…> mention passed qa_10f — fixed: the raw message must carry an encoded mention of the DM counterpart's user id (_encoded_mention_targets_user), not just any encoded mention. - test_run_live_qa.py:3763 pin excerpt conversation-id redaction — fixed: the digest-scan source test now also asserts RAW_SLACK_CONVERSATION_ID_PATTERN.sub. - test_run_live_qa.py:3934 Ruff PT027 assertRaisesRegex — not fixed: the suite is uniformly unittest.TestCase-style (9 pre-existing assertRaises* usages) and this repo runs no ruff gate; converting one call site would be inconsistent. - test_run_live_qa.py:4075 fail-closed coverage for all personal-token QA 10 entrypoints — fixed: the no-token test now drives qa_10a/b/c/d/f/g/h/i (qa_10e acquires no token) and scopes the status_restored assertion to qa_10b. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): qa_10b reads the manually-set OOO status fixture (read-verify mode) The QA account now carries a permanent manually-set OOO status, so qa_10b_slack_ooo_status no longer writes: the users.profile.set seeding and finally-restore logic are gone (with the now-dead _slack_status_profile / _slack_set_own_status helpers). The probe resolves the connected user via personal-token auth.test, reads profile.status_text via users.info (_slack_user_status_text), and fails the precondition "probe precondition failed: no status is set on the QA account — restore the OOO canary status fixture" when the fixture is missing — before burning a chat turn. The reply must contain a word-boundary token (>=3 chars, alnum) of the ground-truth status text, and any hyphenated code token in the status (e.g. OOO-CANARY-FIXTURE, via _status_code_tokens) must be quoted verbatim. Ground truth is recorded in details. Tests: test_qa_10b_fails_when_status_restore_fails and the write-mode status-profile shape test are replaced by test_qa_10b_reads_manually_set_status_fixture (empty-status precondition without a chat turn, green echo, code-dropping paraphrase red) and a _status_code_tokens unit test; the 10b source pins now require users.info + the new precondition string instead of users.profile.set/restore; the fail-closed subTest drops its status_restored scoping; the case-matrix gate text documents the manual fixture. All other cases untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-canary): judge qa_10f authorship on Slack's user field, not bot_id Live forensics on runs 29065062350/29089609863: the model posted the encoded mention correctly via slack.send_message with the connected user's xoxp, and Slack recorded user = the connected account — but stamped bot_id/bot_profile ("via app") on the message, as granular apps do on EVERY user-token post (the probe's own personal-token seeds all carry the same stamp; verified via conversations.history). The old `user == author AND not bot_id` conjunct is therefore unsatisfiable for any new Slack app (classic as_user is not available to them), so the probe could never go green regardless of product behavior. Reformulated without weakening: - Target selection is now encoded-mention-first: only marker messages whose raw text carries <@U…> are authorship candidates, so a marker echo (e.g. a delivered assistant reply) can neither pass nor mask the real post — the sharpening this probe needed anyway. - Authorship is judged on Slack's authoritative, unforgeable `user` field (chat:write.customize can change display name/icon, never `user`); a true bot-identity post still fails because its `user` is the bot user id, and the via-app stamp is surfaced as mention_via_app for forensics instead of failing the case. - The literal-@ pin is preserved: a marker post by the connected user WITHOUT an encoded mention fails with the original "notifies nobody" assert; wrong-identity and never-posted failures keep their messages. New pure classifier _classify_encoded_mention_messages with unit tests covering: via-app user post accepted, unencoded echo ignored, bot-user encoded post rejected as wrong identity, literal-@ flagged. The audited-assert meta-pin now also pins the classifier's gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 12, 2026
This was referenced Jul 14, 2026
This was referenced Jul 17, 2026
This was referenced Jul 22, 2026
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
1 participant
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.
Slack automation delivery + tool-surface overhaul
Fixes the three reported automation failures (wrong-channel delivery, raw user IDs in digests, duplicate bot+user delivery) and — via the squash-merged tool overhaul #5904 — every model-visible defect from the four-lens Slack tool audit. All fixes landed test-first and are live-verified by the canary suite in #5899.
Part 1 — automation delivery routing
delivery_targetcolumn (PostgreSQL + libSQL),delivery_target_idontrigger_create(validated fail-closed against the outbound target registry), host-side routing inTriggeredRunDeliveryDriver— a routine can say where ITS results go without touching the user-wide defaultuser_display_name(oneusers.infoper distinct id, capped at 25, best-effort)Part 2 — tool overhaul (squashed from #5904, commit 0151f2e)
slack.whoami; history marksis_current_user+current_user_id(best-effortauth.test) — fixes the 'says George is off but it's actually me' misattribution classget_user_inforeturnsstatus_text/status_emoji/status_expiration/tz/tz_label/title{code, kind};DispatchError::Wasmgainedsafe_summaryso failures read 'provider error code: channel_not_found' instead of 'the tool operation failed'slack.get_thread_replies; history surfacesreply_countand documents that replies are not in historyis_member,next_cursor/cursor, accurate 'visible to you' wording; history limit clamped to 999, newest-first documented; search matches gainuser_display_name/thread_ts/page<@U…>/<#C…|name>resolved to display names, HTML entities decoded; sends post as the userusers:read.email; widening needs the scope-upgrade re-consent flow, Slack least-privilege: reduce OAuth grant for read-only users (write opt-in) #5669);send_messagedelivery wording corrected to the outbound delivery target;<@U…>mention encoding documentedLive verification (probes from #5899, identical code red vs green)
Merge order: #5899 (canaries) first, then this. Follow-up: promote QA 9+10 into the cron rotation.
🤖 Generated with Claude Code