Skip to content

fix(relay): stop asserting the owner p tag we deliberately removed - #43

Merged
mfethe1 merged 1 commit into
product/mainfrom
fix/workflow-injected-owner-ptag
Sep 11, 2026
Merged

mfethe1 merged 1 commit into
product/mainfrom
fix/workflow-injected-owner-ptag

Conversation

@mfethe1

@mfethe1 mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Closes the last red PostgreSQL Tests assertion on product/main.

Problem

workflow_send_message_binds_authority_to_authored_mentions fails on unmodified trunk at workflow_sink.rs:1371:

trigger-rendered output must preserve the legacy owner p tag; got ["<agent>"]

The same test asserts the opposite 30 lines earlier for the explicit arm (author must NOT be p-tagged — that wakes them as a second agent).

Root cause — conflicting intents merged, not a code bug

commit date intent
31e2f5cfc7 (ours) 2026-08-18 move owner attribution off pactor + buzz:workflow-owner, because ACP wakes on any matching p
93237b4a74 (upstream block#6953) 2026-08-31 still p-tags the owner; added this assertion

Our implementation is the intended one. Only the imported assertion is stale.

Fix

Realign the trigger-rendered arm with the explicit arm: owner must not be p-tagged, and assert positively that actor and buzz:workflow-owner still carry attribution — so this can't regress into silently dropping the wake path instead.

Verification (real PostgreSQL, migrations applied to head, one container)

  • unpatched trunk → panics at 1371
  • patched → test result: ok. 6 passed; 0 failed across workflow_sink::postgres_tests
  • cargo fmt --check 0, cargo clippy --all-targets -- -D warnings 0 diagnostics

…moved

`workflow_send_message_binds_authority_to_authored_mentions` has failed on
product/main since the upstream import: the trigger-rendered arm asserts the
owner is p-tagged, while the explicit arm 30 lines above asserts the exact
opposite.

Root cause is a merge of two conflicting intents, not a code bug:

- 31e2f5c (ours, 2026-08-18) moved owner attribution off `p` and onto
  `actor`/`buzz:workflow-owner`, because ACP wakes on any `p` matching an
  agent pubkey — p-tagging the owner woke them as a second agent on their
  own workflow's output.
- 93237b4 (upstream block#6953, 2026-08-31) still p-tags the owner and added
  this assertion to lock that in.

The implementation is ours and correct; only the imported assertion was
stale. Realign it: the owner must NOT be p-tagged on either arm, and assert
positively that `actor` and `buzz:workflow-owner` still carry attribution so
the wake path is not silently dropped instead.

Verified on real PostgreSQL (migrations applied to head):
- trunk: panics at 1371 "must preserve the legacy owner p tag"
- patched: `6 passed; 0 failed` across workflow_sink::postgres_tests
fmt 0 / clippy -D warnings 0 diagnostics.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 689a9896-6d61-4dde-8b77-4ffab7345d6a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

CI verdict: strict improvement over trunk

Baseline = trunk push run 34643873479 @ f60aa794c0. PR run 34645519472.

job trunk f60aa794c0 PR #43
PostgreSQL Domain / PostgreSQL Tests ❌ fail pass
PostgreSQL Tests ❌ fail pass
Clients / Mobile ❌ fail ✅ pass
Desktop / Desktop Domain / Desktop ❌ fail ✅ pass
Desktop Smoke E2E (4) ❌ fail ✅ pass
Mobile ❌ fail ✅ pass
Windows Rust (x2) ❌ fail ❌ fail (pre-existing, recovery_tests.rs:361)
Relay E2E / Backend Integration / Desktop E2E Integration (x6) ❌ fail ❌ fail
Desktop Smoke E2E (3) ❌ fail ❌ fail

#43's failure set is a strict subset of trunk's. Nothing new is red.

The relay/e2e cluster is infrastructure, not code — job 103421381847 log line 336:

minio-init Error pull access denied for minio/mc, repository does not exist or may require 'docker login': denied

Same Docker Hub pull denial that hit #42.

Merging: this closes the last red PostgreSQL assertion. Windows Rust and Clients / Mobile remain open and tracked separately.

@mfethe1
mfethe1 merged commit c644b8d into product/main Sep 11, 2026
57 of 69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant