Skip to content

fix(slack): restore personal delivery and standardized canaries - #7300

Merged
BenKurrek merged 5 commits into
mainfrom
agent/fix-slack-canary-identity
Aug 7, 2026
Merged

BenKurrek merged 5 commits into
mainfrom
agent/fix-slack-canary-identity

Conversation

@BenKurrek

@BenKurrek BenKurrek commented Aug 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Restore the provisioned Slack personal-DM target when OAuth post-bind state contains a conversation ID but no workspace ID, using the active connection's validated workspace scope.
  • Fail closed when a stored DM target explicitly names a workspace different from the active Slack connection, and verify listing/resolution through the production provider path.
  • Restore provider-neutral newest-first message search and map it to Slack's sort=timestamp&sort_dir=desc semantics.
  • Update Slack QA canaries for standardized messaging's canonical conversation, message_ref, and messaging.unknown_conversation shapes, and rebuild/freshness-pin the bundled Slack WASM artifact.

Root cause

The QA account identity was correct throughout: the WebUI run belonged to the requesting IronClaw user, and slack.whoami resolved that user's connected Slack account. There was no internal thread-owner substitution.

The failure was in delivery-target projection. Slack OAuth post-bind provisioning opened the requesting user's self-DM and persisted its conversation ID, but the Slack adapter did not have a workspace ID in that response. The generic outbound-target provider required the missing workspace ID to encode a personal Slack destination, so it silently omitted the otherwise valid target. In the observed QA run, outbound_delivery_targets_list consequently returned no Slack target, and the routine was created with delivery_target_id: null. At fire time the host logged that delivery was skipped because routing was ambiguous.

That left the scheduled agent with the prompt's free-form "send to Slack" instruction and ordinary user-token Slack tools. It enumerated the requester's DM catalog and selected another person's DM. Slack therefore displayed the requester as the sender in the other person's conversation—the credential/actor was correct, but the recipient was wrong.

This routing defect predates standardized messaging. The standardized-messaging merge did independently expose deterministic canary regressions: QA10E/F still expected legacy result shapes, and removing Slack's sort input made QA10G-global enumerate conversations until timeout. This PR fixes both issue families.

Change Type

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • CI/Infrastructure
  • Security
  • Dependencies

Linked Issue

Related #7249; Related #7295

Validation

  • cargo fmt --all -- --check
  • cargo clippy --all --benches --tests --examples --all-features -- -D warnings (scoped warnings-denied clippy run for every changed Rust owner; see commands below)
  • cargo build (covered by the scoped test/clippy builds and CI)
  • Relevant tests pass: extension host, Slack adapter, host API, host runtime WASM contract, extension registry, architecture, and QA harness suites listed below
  • cargo test -p <owning-crate> --features integration if database-backed or runtime-integration behavior changed (Not applicable: no database backend or schema behavior changed)
  • Manual testing: correlated the supplied screenshots with Railway QA logs and the exact WebUI/scheduled-run timeline; no live Slack write was performed
  • If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review (Not applicable: repository review automation runs on the ready PR; all actionable findings will be handled)

Scoped clippy command:

cargo clippy -p ironclaw_extension_host -p ironclaw_slack_extension -p ironclaw_host_api --all-targets --all-features -- -D warnings

Test Strategy

User behavior: A WebUI routine asking to send results to Slack binds the creator's provisioned personal Slack DM and is delivered by the host; "latest/most recent message anywhere in Slack" uses newest-first provider search without enumerating every DM.

Risk areas:

  • Model behavior
  • Browser
  • Side effect
  • Persistence
  • Security or permissions
  • External provider
  • Cross-component behavior

Tests added or updated:

  • Unit or contract: canonical search sort values/default rejection; QA evidence redaction and canonical argument/result/error assertions; the QA harness passes 204 tests with 5 skipped and 4 expected failures.
  • Reborn integration: caller-path extension-host tests reproduce the real workspace-less provisioned record, prove the personal target is listed/resolved with the active workspace, and prove an explicit workspace mismatch fails closed. The complete extension-host and architecture suites pass.
  • Recorded fixture: Not applicable: model output is not relied on for the routing fix; Slack request mapping is asserted by the runtime contract.
  • Browser E2E: Not applicable: no browser presentation or interaction changed.
  • Backend or runtime: the host runtime executes the rebuilt Slack artifact and asserts timestamp-sort query parameters; native Slack conformance and artifact freshness pass.
  • Live canary: Not run locally because it writes to the shared QA Slack workspace. The incident was diagnosed read-only from Railway logs; the next credentialed canary will validate the deployed provider boundary.

What the tests prove: an OAuth-shaped Slack DM record can no longer disappear because it lacks redundant workspace metadata; stored state cannot be rebound across workspaces; the resulting target is creator-scoped and round-trips through the production outbound registry; Slack search ordering and standardized canary evidence use their canonical contracts.

Commands run:

  • cargo test -p ironclaw_extension_host (371 unit/E2E + 17 contract tests)
  • cargo test -p ironclaw_slack_extension (91 unit + 1 conformance test)
  • cargo test -p ironclaw_host_api (202 unit + 70 contract/seal tests)
  • cargo test -p ironclaw_host_runtime --test github_wasm_runtime_contract (55 tests)
  • cargo test -p ironclaw_extension_registry -p ironclaw_architecture_tests
  • cargo clippy -p ironclaw_extension_host -p ironclaw_slack_extension -p ironclaw_host_api --all-targets --all-features -- -D warnings
  • python3 scripts/reborn_webui_v2_live_qa/test_run_live_qa.py (204 tests; 5 skipped; 4 expected failures)
  • python3 scripts/ci/check-wasm-artifact-freshness.py
  • cargo fmt --all -- --check
  • git diff --check origin/main...HEAD

Security Impact

This strengthens recipient scoping. A missing redundant workspace field is completed only from the active extension connection's validated workspace claim. An explicitly stored, stale, or tampered workspace mismatch fails closed. Tenant/user ownership checks and the provisioned external actor check remain mandatory; no credential, authorization, ingress, filesystem, sandbox, or tool-write permission is weakened.

Reborn Trust-Boundary Checklist

  • Public policy/evidence/trust-bearing types: no new public authority type; the effective DM scope is derived from active extension configuration plus caller-owned provisioned state.
  • Untrusted content enters prompts only through an envelope/escaping primitive. No prompt-content ingress path changed.
  • Hashes declare purpose; trust/binding/authenticity uses SHA-256/BLAKE3 or separate authenticity check. The existing WASM source-freshness digest was regenerated and verified.
  • New/changed status, exit, policy, runtime, or error variants: downstream match sites audited. Command/output: not applicable; no variant changed.
  • Security/durability serde(default) fields fail closed or have migration tests. A missing DM workspace is covered as a compatibility case; an explicit mismatch is covered as fail-closed.
  • Queues/maps/buffers/counters have bounds and overflow-safe arithmetic. Not applicable: none changed.
  • Driver/operator-visible errors have stable class semantics (Transient, Permanent, Misconfigured, PolicyDenied or equivalent). Not applicable: error classification is unchanged.
  • Sandbox/native/host names accurately describe trust boundary. Provider-specific Slack encoding remains in the Slack codec; the generic host consumes only connection scope and typed target state.

Database Impact

None. No schema or migration changes. Existing workspace-less DM records become usable at read time, so no connection-data backfill or Slack reconnect is required; PostgreSQL/libSQL behavior is unchanged. Triggers already created during the defect window with delivery_target_id: null are deliberately not rewritten because their intended recipient cannot be recovered authoritatively; users must recreate those routines or explicitly select a destination after deployment.

Blast Radius

Generic outbound-target listing/resolution for channel extensions, the canonical message-search input, the bundled Slack personal-account search guest, runtime contract fixture, standardized messaging design documentation, and QA10 Slack harness. For non-Slack channels, a workspace-less personal target can inherit only that same active extension's configured space; an explicit mismatch is omitted.

Rollback Plan

Revert this PR. No persisted data needs rollback. The known consequence is that existing workspace-less Slack DM targets disappear again, routines may be created without a delivery target, Slack canaries regress to stale canonical shapes, and global latest-message search returns to conversation enumeration/timeouts.

Review Follow-Through

After deployment, inspect the next credentialed Slack routine/canary for a non-null creator-owned delivery target and host-delivered final reply. No manual live send should target a real QA user's DM merely to validate routing.


Review track: C (recipient-routing/security boundary and cross-component runtime bug fix)

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added optional message search sorting by relevance or timestamp.
    • Timestamp sorting returns the newest matching messages first; relevance remains the default.
  • Bug Fixes
    • Improved Slack direct-message resolution across workspace contexts.
    • Prevented messages from unrelated workspaces from being resolved or used for replies.
    • Standardized unknown-conversation error reporting.
  • Documentation
    • Updated messaging and Slack guidance to describe sorting options and newest-first searches.
  • Tests
    • Added coverage for search sorting and workspace-aware direct-message behavior.

Walkthrough

The PR adds optional relevance or timestamp sorting to messaging search and maps these modes to Slack query parameters. It also scopes Slack DM resolution by workspace and updates runtime, contract, end-to-end, and live-QA coverage.

Changes

Messaging search sorting

Layer / File(s) Summary
Canonical search sorting contract
crates/contracts/ironclaw_host_api/schemas/messaging/..., crates/contracts/ironclaw_host_api/prompts/messaging/..., docs/internal/superpowers/specs/..., crates/contracts/ironclaw_host_api/tests/...
The search input accepts optional relevance or timestamp sorting. Contract tests validate both values, omission, and rejection of newest.
Slack search sort forwarding
crates/extensions/packages/slack/wasm-src/src/..., crates/extensions/packages/slack/prompts/slack/...
Slack actions accept the sort enum and forward it. The adapter maps timestamp sorting to descending Slack timestamps and relevance sorting to Slack score ordering.
Search runtime and QA validation
crates/kernel/ironclaw_host_runtime/tests/..., scripts/ci/..., scripts/reborn_webui_v2_live_qa/...
Runtime and live-QA checks use timestamp sorting, conversation arguments, structured message references, and the normalized messaging.unknown_conversation error.

Scoped DM target resolution

Layer / File(s) Summary
Workspace-scoped DM resolution
crates/extensions/ironclaw_extension_host/src/channel_outbound_targets.rs
DM resolution fills missing workspace IDs from the active scope and rejects blank or mismatched workspace data. Reply bindings must match both workspace and conversation IDs.
DM scope regression coverage
crates/extensions/ironclaw_extension_host/src/channel_host/e2e_tests.rs
End-to-end tests cover workspace inheritance and fail-closed behavior for records from another workspace.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant SlackUserAction
  participant SlackSearchAPI
  Caller->>SlackUserAction: submit query with optional sort
  SlackUserAction->>SlackSearchAPI: forward SearchMessagesSort
  SlackSearchAPI->>SlackSearchAPI: add Slack sorting parameters
  SlackSearchAPI-->>Caller: return ordered search results
Loading

Possibly related PRs

  • nearai/ironclaw#6831: This PR extends the standardized search_messages contract and Slack implementation introduced there.

Suggested reviewers: henrypark133

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses Conventional Commits style and accurately describes the Slack delivery and canary fixes.
Description check ✅ Passed The description follows the template and documents scope, risks, validation, security, database impact, rollback, and follow-through.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@railway-app

railway-app Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

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

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Aug 7, 2026 at 11:33 am

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7300 August 6, 2026 15:22 Destroyed
@github-actions github-actions Bot added size: M 50-199 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs scope: docs Documentation labels Aug 6, 2026
@ironloopai

ironloopai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Review · PR #7300

🟢 Completed · Review submitted

Submitted review →

Reviewed the complete trusted base-to-head comparison across the canonical messaging contract, Slack WASM implementation and artifact, runtime contract coverage, documentation, and live-QA evidence handling. No concrete correctness, security, architecture, maintainability, or test-coverage defects were found.

Automatic · PR opened · attempt 1 of 3 · completed in 1m 40s

Run details
  • Repository: nearai/ironclaw
  • Base: main at 9c85269
  • Head: agent/fix-slack-canary-identity at 9e7dcb3
  • Created: Aug 6, 2026, 3:27 PM UTC
  • Updated: Aug 6, 2026, 3:29 PM UTC
  • Run: a9565912-9fb5-4d14-9095-18283d7537ca
  • Latest attempt: 1 · Completed · 74ced6ae-7057-4489-ad87-45582a860f1c

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Review complete · PR #7300

✅ No actionable findings

Reviewed the complete trusted base-to-head comparison across the canonical messaging contract, Slack WASM implementation and artifact, runtime contract coverage, documentation, and live-QA evidence handling. No concrete correctness, security, architecture, maintainability, or test-coverage defects were found.

Validation and technical details
  • Inspected the complete refs/ironloop/base..refs/ironloop/head diff and surrounding call paths for all 14 changed files.
  • Live-QA unit suite passed: 204 tests, 5 skipped, 4 expected failures.
  • WASM artifact freshness check passed for all 6 checked packages.
  • Verified Slack timestamp ordering maps to sort=timestamp&sort_dir=desc and omitted sort preserves vendor-default relevance behavior.
  • Verified QA evidence retains only canonical non-content arguments (conversation and sort), excluding query and message text.
  • git diff --check completed without errors.
  • Targeted Rust tests could not be rerun because cargo is unavailable in the review environment; their implementations and assertions were inspected directly.
  • Base: main
  • Head: agent/fix-slack-canary-identity at 9e7dcb3
  • Run: a9565912-9fb5-4d14-9095-18283d7537ca

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7300 August 6, 2026 16:19 Destroyed
@BenKurrek BenKurrek changed the title fix(slack): repair standardized canaries and latest search fix(slack): restore personal delivery and standardized canaries Aug 6, 2026
@BenKurrek
BenKurrek marked this pull request as ready for review August 6, 2026 16:20
@ironloopai

ironloopai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Review · PR #7300

🟢 Completed · Review submitted

Submitted review →

Reviewed the complete trusted base-to-head comparison. No concrete correctness, security, architecture, maintainability, or test-coverage defects were found. The workspace-less DM fallback is constrained to the active connection scope, explicit workspace mismatches fail closed, target listing and resolution remain caller-scoped, Slack timestamp sorting maps to newest-first provider semantics, QA evidence retains only approved non-content fields, and the bundled WASM matches its source digest.

Automatic · PR opened · attempt 1 of 3 · completed in 1m 34s

Run details
  • Repository: nearai/ironclaw
  • Base: main at 9c85269
  • Head: agent/fix-slack-canary-identity at b66fb9d
  • Created: Aug 6, 2026, 4:25 PM UTC
  • Updated: Aug 6, 2026, 4:27 PM UTC
  • Run: 7b0a9fba-835c-4a33-8efd-054d9566563e
  • Latest attempt: 1 · Completed · b8b138f3-0849-4b90-bb56-f6989320f1bf

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Review complete · PR #7300

✅ No actionable findings

Reviewed the complete trusted base-to-head comparison. No concrete correctness, security, architecture, maintainability, or test-coverage defects were found. The workspace-less DM fallback is constrained to the active connection scope, explicit workspace mismatches fail closed, target listing and resolution remain caller-scoped, Slack timestamp sorting maps to newest-first provider semantics, QA evidence retains only approved non-content fields, and the bundled WASM matches its source digest.

Validation and technical details
  • Inspected all 16 changed files and surrounding recipient-routing, target-resolution, messaging-contract, Slack adapter, runtime-contract, QA harness, documentation, and artifact-provenance code.
  • Codebase graph status was MISSING; followed repository guidance and used crate-local instructions plus targeted live-code searches.
  • python3 scripts/ci/check-wasm-artifact-freshness.py: passed (6 packages checked).
  • python3 scripts/reborn_webui_v2_live_qa/test_run_live_qa.py: passed 204 tests (5 skipped, 4 expected failures).
  • git diff --check refs/ironloop/base..refs/ironloop/head: passed.
  • Focused Rust tests could not be rerun because cargo is unavailable in the review environment (cargo: command not found); their implementations and assertions were inspected directly.
  • Base: main
  • Head: agent/fix-slack-canary-identity at b66fb9d
  • Run: 7b0a9fba-835c-4a33-8efd-054d9566563e

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/contracts/ironclaw_host_api/tests/host_api_contract.rs`:
- Around line 47-67: Extend search_messages_accepts_portable_sort_modes to
assert that a request containing only the required query field, with sort
omitted, remains valid according to the compiled schema. Keep the existing
canonical enum and obsolete-value assertions unchanged.

In `@crates/extensions/ironclaw_extension_host/src/channel_host/e2e_tests.rs`:
- Around line 4698-4727: Extend
generic_dm_target_rejects_record_from_a_different_workspace to create a
personal-DM binding that decodes to the active TEAM while retaining
T_OTHER_WORKSPACE in the stored record. Invoke resolve_reply_target_binding
through the relevant caller/provider path and assert it returns None, rather
than only asserting list_outbound_delivery_targets excludes the target.
🪄 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: 9c552ba8-47a3-48e9-8ad9-e5cf0966ceae

📥 Commits

Reviewing files that changed from the base of the PR and between 9c85269 and b66fb9d.

⛔ Files ignored due to path filters (1)
  • crates/extensions/packages/slack/wasm/slack_user_tool.wasm is excluded by !**/*.wasm, !**/*.wasm
📒 Files selected for processing (15)
  • crates/contracts/ironclaw_host_api/prompts/messaging/search_messages.core.md
  • crates/contracts/ironclaw_host_api/schemas/messaging/search_messages.input.v1.json
  • crates/contracts/ironclaw_host_api/tests/host_api_contract.rs
  • crates/extensions/ironclaw_extension_host/src/channel_host/e2e_tests.rs
  • crates/extensions/ironclaw_extension_host/src/channel_outbound_targets.rs
  • crates/extensions/packages/slack/prompts/slack/search_messages.md
  • crates/extensions/packages/slack/wasm-src/src/api.rs
  • crates/extensions/packages/slack/wasm-src/src/lib.rs
  • crates/extensions/packages/slack/wasm-src/src/types.rs
  • crates/kernel/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs
  • docs/superpowers/specs/2026-07-27-standardized-messaging-framework-design.md
  • scripts/ci/wasm-src-digests.toml
  • scripts/reborn_webui_v2_live_qa/case_matrix.py
  • scripts/reborn_webui_v2_live_qa/run_live_qa.py
  • scripts/reborn_webui_v2_live_qa/test_run_live_qa.py

Comment thread crates/contracts/ironclaw_host_api/tests/host_api_contract.rs
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7300 August 6, 2026 16:30 Destroyed
Resolves the GitHub-reported conflict: main relocated
docs/superpowers/specs/2026-07-27-standardized-messaging-framework-design.md
to docs/internal/superpowers/specs/ (#7259); this PR's edits to that doc now
land at the new path. The only other overlapping file
(channel_host/e2e_tests.rs) merged trivially (comment path update on main).
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7300 August 7, 2026 11:26 Destroyed
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/extensions/ironclaw_extension_host/src/channel_host/e2e_tests.rs`:
- Around line 4651-4696: Extend
generic_dm_target_inherits_active_workspace_when_record_omits_space to call
resolve_reply_target_binding with external_reply_target(dm) after the existing
resolution assertions, and verify the returned binding has the same target ID as
dm.summary.target_id. Exercise this through the provider caller path and
preserve the existing listing and resolve_outbound_delivery_target checks.

In `@crates/extensions/packages/slack/wasm-src/src/types.rs`:
- Around line 33-40: Use the enum in
crates/extensions/packages/slack/wasm-src/src/types.rs:33-40 as the sole
SearchMessagesSort definition; remove the duplicate API-local enum, import the
canonical type in api.rs, and update the forwarding call in
crates/extensions/packages/slack/wasm-src/src/lib.rs:97-101 to pass
crate::types::SearchMessagesSort.
🪄 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: 29f50300-79b7-451c-9d58-2d6ccf6b9242

📥 Commits

Reviewing files that changed from the base of the PR and between 389aa99 and eba7c41.

⛔ Files ignored due to path filters (1)
  • crates/extensions/packages/slack/wasm/slack_user_tool.wasm is excluded by !**/*.wasm, !**/*.wasm
📒 Files selected for processing (15)
  • crates/contracts/ironclaw_host_api/prompts/messaging/search_messages.core.md
  • crates/contracts/ironclaw_host_api/schemas/messaging/search_messages.input.v1.json
  • crates/contracts/ironclaw_host_api/tests/host_api_contract.rs
  • crates/extensions/ironclaw_extension_host/src/channel_host/e2e_tests.rs
  • crates/extensions/ironclaw_extension_host/src/channel_outbound_targets.rs
  • crates/extensions/packages/slack/prompts/slack/search_messages.md
  • crates/extensions/packages/slack/wasm-src/src/api.rs
  • crates/extensions/packages/slack/wasm-src/src/lib.rs
  • crates/extensions/packages/slack/wasm-src/src/types.rs
  • crates/kernel/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs
  • docs/internal/superpowers/specs/2026-07-27-standardized-messaging-framework-design.md
  • scripts/ci/wasm-src-digests.toml
  • scripts/reborn_webui_v2_live_qa/case_matrix.py
  • scripts/reborn_webui_v2_live_qa/run_live_qa.py
  • scripts/reborn_webui_v2_live_qa/test_run_live_qa.py

Comment on lines +4651 to +4696
/// REGRESSION (OAuth post-bind provisioning): Slack's `conversations.open`
/// response supplies the DM conversation id but not the workspace id. The
/// generic target provider must complete that record with the active,
/// connection-scoped workspace claim or the creator's personal destination
/// disappears and trigger creation cannot bind delivery to their own DM.
#[tokio::test]
async fn generic_dm_target_inherits_active_workspace_when_record_omits_space() {
let harness = build_harness(TurnMode::Running).await;
save_outbound_target_config(&harness).await;
let dm_targets = generic_dm_target_store();
dm_targets
.upsert(
ADAPTER,
&UserId::new(USER).expect("user"), // safety: static test user id is valid.
SLACK_USER.to_string(),
dm_target_payload(None, CHANNEL),
)
.await
.expect("provision DM target without workspace");
let provider = generic_outbound_target_provider(&harness, dm_targets);

let listed = provider
.list_outbound_delivery_targets(&operator_caller())
.await
.expect("target list");
let dm = listed
.iter()
.find(|entry| entry.summary.target_id.as_str().contains("personal-dm"))
.expect("workspace-less provisioned DM should remain available");
assert_eq!(
dm.summary.target_id.as_str(),
format!("slack:personal-dm:{TEAM}:{USER}")
);
let conversation = SlackPreferenceTargetCodec
.conversation_for_target(external_reply_target(dm))
.expect("personal-DM binding ref decodes");
assert_eq!(conversation.space_id(), Some(TEAM));
assert_eq!(conversation.conversation_id(), CHANNEL);

let resolved = provider
.resolve_outbound_delivery_target(&operator_caller(), &dm.summary.target_id)
.await
.expect("resolve succeeds")
.expect("listed personal-DM target resolves");
assert_eq!(resolved.summary.target_id, dm.summary.target_id);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Test successful reply-binding resolution for a workspace-less record.

This test exercises listing and resolve_outbound_delivery_target, but not resolve_reply_target_binding. The changed reply-binding branch must accept the binding after dm_record_conversation completes the missing workspace ID.

Resolve external_reply_target(dm) through resolve_reply_target_binding and assert that it returns the same target ID.

Proposed test addition
     assert_eq!(resolved.summary.target_id, dm.summary.target_id);
+
+    let resolved_binding = provider
+        .resolve_reply_target_binding(&operator_caller(), external_reply_target(dm))
+        .await
+        .expect("reply-target resolution succeeds")
+        .expect("workspace-less DM binding resolves");
+    assert_eq!(resolved_binding.summary.target_id, dm.summary.target_id);
 }

As per coding guidelines, “Every bug fix must add a regression test.” As per path instructions, “Test through the caller.”

📝 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
/// REGRESSION (OAuth post-bind provisioning): Slack's `conversations.open`
/// response supplies the DM conversation id but not the workspace id. The
/// generic target provider must complete that record with the active,
/// connection-scoped workspace claim or the creator's personal destination
/// disappears and trigger creation cannot bind delivery to their own DM.
#[tokio::test]
async fn generic_dm_target_inherits_active_workspace_when_record_omits_space() {
let harness = build_harness(TurnMode::Running).await;
save_outbound_target_config(&harness).await;
let dm_targets = generic_dm_target_store();
dm_targets
.upsert(
ADAPTER,
&UserId::new(USER).expect("user"), // safety: static test user id is valid.
SLACK_USER.to_string(),
dm_target_payload(None, CHANNEL),
)
.await
.expect("provision DM target without workspace");
let provider = generic_outbound_target_provider(&harness, dm_targets);
let listed = provider
.list_outbound_delivery_targets(&operator_caller())
.await
.expect("target list");
let dm = listed
.iter()
.find(|entry| entry.summary.target_id.as_str().contains("personal-dm"))
.expect("workspace-less provisioned DM should remain available");
assert_eq!(
dm.summary.target_id.as_str(),
format!("slack:personal-dm:{TEAM}:{USER}")
);
let conversation = SlackPreferenceTargetCodec
.conversation_for_target(external_reply_target(dm))
.expect("personal-DM binding ref decodes");
assert_eq!(conversation.space_id(), Some(TEAM));
assert_eq!(conversation.conversation_id(), CHANNEL);
let resolved = provider
.resolve_outbound_delivery_target(&operator_caller(), &dm.summary.target_id)
.await
.expect("resolve succeeds")
.expect("listed personal-DM target resolves");
assert_eq!(resolved.summary.target_id, dm.summary.target_id);
}
/// REGRESSION (OAuth post-bind provisioning): Slack's `conversations.open`
/// response supplies the DM conversation id but not the workspace id. The
/// generic target provider must complete that record with the active,
/// connection-scoped workspace claim or the creator's personal destination
/// disappears and trigger creation cannot bind delivery to their own DM.
#[tokio::test]
async fn generic_dm_target_inherits_active_workspace_when_record_omits_space() {
let harness = build_harness(TurnMode::Running).await;
save_outbound_target_config(&harness).await;
let dm_targets = generic_dm_target_store();
dm_targets
.upsert(
ADAPTER,
&UserId::new(USER).expect("user"), // safety: static test user id is valid.
SLACK_USER.to_string(),
dm_target_payload(None, CHANNEL),
)
.await
.expect("provision DM target without workspace");
let provider = generic_outbound_target_provider(&harness, dm_targets);
let listed = provider
.list_outbound_delivery_targets(&operator_caller())
.await
.expect("target list");
let dm = listed
.iter()
.find(|entry| entry.summary.target_id.as_str().contains("personal-dm"))
.expect("workspace-less provisioned DM should remain available");
assert_eq!(
dm.summary.target_id.as_str(),
format!("slack:personal-dm:{TEAM}:{USER}")
);
let conversation = SlackPreferenceTargetCodec
.conversation_for_target(external_reply_target(dm))
.expect("personal-DM binding ref decodes");
assert_eq!(conversation.space_id(), Some(TEAM));
assert_eq!(conversation.conversation_id(), CHANNEL);
let resolved = provider
.resolve_outbound_delivery_target(&operator_caller(), &dm.summary.target_id)
.await
.expect("resolve succeeds")
.expect("listed personal-DM target resolves");
assert_eq!(resolved.summary.target_id, dm.summary.target_id);
let resolved_binding = provider
.resolve_reply_target_binding(&operator_caller(), external_reply_target(dm))
.await
.expect("reply-target resolution succeeds")
.expect("workspace-less DM binding resolves");
assert_eq!(resolved_binding.summary.target_id, dm.summary.target_id);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/extensions/ironclaw_extension_host/src/channel_host/e2e_tests.rs`
around lines 4651 - 4696, Extend
generic_dm_target_inherits_active_workspace_when_record_omits_space to call
resolve_reply_target_binding with external_reply_target(dm) after the existing
resolution assertions, and verify the returned binding has the same target ID as
dm.summary.target_id. Exercise this through the provider caller path and
preserve the existing listing and resolve_outbound_delivery_target checks.

Sources: Coding guidelines, Path instructions

Comment on lines +33 to +40
/// Portable ordering for a cross-conversation message search.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub enum SearchMessagesSort {
Relevance,
Timestamp,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Use one SearchMessagesSort type across the Slack WASM path.

The new enum in types.rs conflicts with the existing enum in api.rs Lines 36-39. Rust treats them as distinct types, so the forwarding call cannot compile.

  • crates/extensions/packages/slack/wasm-src/src/types.rs#L33-L40: Keep SearchMessagesSort as the single definition and remove the API-local enum.
  • crates/extensions/packages/slack/wasm-src/src/lib.rs#L97-L101: Forward the shared crate::types::SearchMessagesSort after api.rs imports the canonical type.

As per coding guidelines, shared Rust types must have exactly one definition in the owning crate.

📍 Affects 2 files
  • crates/extensions/packages/slack/wasm-src/src/types.rs#L33-L40 (this comment)
  • crates/extensions/packages/slack/wasm-src/src/lib.rs#L97-L101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/extensions/packages/slack/wasm-src/src/types.rs` around lines 33 - 40,
Use the enum in crates/extensions/packages/slack/wasm-src/src/types.rs:33-40 as
the sole SearchMessagesSort definition; remove the duplicate API-local enum,
import the canonical type in api.rs, and update the forwarding call in
crates/extensions/packages/slack/wasm-src/src/lib.rs:97-101 to pass
crate::types::SearchMessagesSort.

Source: Coding guidelines

@BenKurrek
BenKurrek enabled auto-merge August 7, 2026 12:13
@BenKurrek
BenKurrek added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit a849373 Aug 7, 2026
48 checks passed
@BenKurrek
BenKurrek deleted the agent/fix-slack-canary-identity branch August 7, 2026 12:26
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
…ai#7300)

* fix(qa): align Slack canaries with messaging standard

* fix(slack): restore recency-ordered message search

* fix(slack): retain provisioned personal DM targets

* test(slack): cover default sort and DM scope resolution

This branch was successfully deployed

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

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules scope: docs Documentation size: M 50-199 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants