Automations and webhooks for the AT Protocol airglow.run

fix(form): key the foreign-repo warnings and soften them for strongRef fields #68

Closed tyb.there-hugo.dev
Closed opened by tyb.there-hugo.dev

Low priority. Applies to the #12 branch (loop/3my32fgxkok22, afaa013) before it merges, or as a follow-up after.

Problem #

The foreign-repo warning added in #12 is noisy. A strongRef to someone else's post is the normal case for likes, reposts and replies, yet any field holding a foreign at:// URI gets a warning. The warning divs rendered in a .map also have no key.

Evidence #

  • app/islands/AutomationForm.tsx, in the renderActionBody hunk of afaa013: the warning divs have no key.
  • foreign-repo-refs.ts flags any field whose at:// URI points at another repo, including strongRef fields.

Expected #

  • Each warning has a stable key.
  • Only fields that set the write target (repo / uri of patch-record) get the full warning. strongRef fields get a softer note, or none.

Acceptance criteria #

Out of scope #

Server-side validation (the PDS already rejects writes to foreign repos), and the "Success/200" result wording (#13).

Queued: the loop will work on this version of the issue.

loop:queued cid=bafyreidhpca3u2bkftsuwrmvowbexaaybg3wbycbkjs7wy7xvk2cndhyp4

Working on this, attempt 1.

loop:claim run=contribute-20261030-105555-68 branch=loop/68 attempt=1 issue=68

fix(form): key the foreign-repo warnings and drop them for strongRef fields

The foreign-repo warning from #12 no longer fires for strongRef fields. These are objects with both uri and cid, such as a like's or repost's subject, a reply's root/parent, or a quote embed's record. Pointing at someone else's post is the normal case there. Fields that do make a record land in or count toward another repo still get the full warning: patch-record's Base Record URI, and plain at-uri fields such as a listitem's list. Each rendered warning now has a stable key (field + repo).

Verified #

  • pnpm run check passes, and so does pnpm run test (183 files, 2823 tests).
  • foreign-repo-refs.test.ts: a like's strongRef, a reply's strongRefs and a quote's embedded strongRef to another account give no refs. A list next to a strongRef is still found. A {uri} with no cid is still found.
  • AutomationForm.test.tsx "target repo" cases:
    • A like and a reply pointing at another account's posts show no warning.
    • The listitem list still warns.
    • Patch-record's foreign Base Record URI warns.
    • The new key test fails without the key and passes with it. It renders two warnings, clears the first through the input, and checks that the remaining warning keeps its own DOM element.
  • No new controls, so there was nothing new to try with the keyboard.

Acceptance criteria #

Decisions you can reverse #

  • No note for strongRefs. The issue allowed a soft note or none, and I picked none. A soft note would mean adding a kind to ForeignRepoRef and a second message.
  • How a strongRef is spotted. It is an object with both uri and cid, as in the lexicon. A { "uri": ... } with no cid is not a valid strongRef, so it still warns.
  • Warning text. I removed "That is fine for a like or a reply, but ..." from the full warning, because those cases no longer show it.

Review: https://tangled.org/exosphere.site/airglow/compare/main...loop/68

loop:pushed run=contribute-20261030-105555-68 sha=b3bc3fbad9ba10a9bc05814c5528355c71c65571

Changes requested. The branch adds the key and stops warning about a complete strongRef, and pnpm run check and pnpm run test (183 files, 2823 tests) both pass. One gap is left: in the record builder, the form's default editor, a like or reply to another account's post still shows the full warning until the CID is filled in. That is the noise this issue is about.

What I checked and how:

  • Read the diff against each acceptance criterion and the code around it: foreign-repo-refs.ts, draftForeignRepoRefs in registry.ts (the only caller) and renderActionBody in AutomationForm.tsx.
    • The key field\nrepo is unique. Field paths from one template never repeat, and the patch-record base URI is labelled "Base Record URI".
    • The key test does fail if the key is removed.
    • Patch-record's foreign base URI still warns, and so does a listitem's list.
  • I accept the branch's reading of "only fields that set the write target". It keeps the plain at-uri warning (a listitem's list), which was the whole point of #12, and silences strongRefs, as the issue's title asks. A side effect: a plain at-uri that normally points at someone else's record, such as app.bsky.graph.listblock's subject, still gets the full warning. If the owner wants that quiet too, it is a separate issue.
  • I tested the common real flow with a throwaway test in /work, since deleted:
    • Setup: AutomationForm with a record action on app.bsky.feed.like, and the lexicon fetch mocked with the like schema, subject being a com.atproto.repo.strongRef object.
    • I typed at://did:plc:other/app.bsky.feed.post/1 into #field-uri. The builder emits {"subject": {"$type": "com.atproto.repo.strongRef", "uri": "at://did:plc:other/..."}}, with no cid until that field is filled. So isStrongRef returns false, and the full "points at a record in another account's repo ... will have no effect" warning appears.
    • It went away only after I typed into #field-cid.

Changes:

  1. In app/islands/action-editors/foreign-repo-refs.ts, also count an object whose $type is "com.atproto.repo.strongRef" as a strongRef in isStrongRef, for example ("$type" in value && value.$type === "com.atproto.repo.strongRef") || ("uri" in value && "cid" in value).
    • The record builder stamps that $type on every object reached through the strongRef ref, so a like, repost, reply or quote that is still being filled in stays quiet.
    • A raw-JSON { "uri": ... } with neither $type nor cid still warns, as the branch intends.
    • I tried exactly this change against the builder flow above: no warning after typing the URI, none after typing the CID. foreign-repo-refs.test.ts still passes.
  2. Add tests for it:
    • In foreign-repo-refs.test.ts: { subject: { $type: "com.atproto.repo.strongRef", uri: "at://did:plc:other/app.bsky.feed.post/1" } } gives no refs.
    • In the AutomationForm.test.tsx "target repo" cases: a like built through the record builder, with the lexicon fetch mocked to the like schema as undeclared trigger record paths does. Type a foreign URI into #field-uri with the CID still empty, and assert that no warning shows.

loop:verdict run=review-20261102-180252-68 sha=b3bc3fbad9ba10a9bc05814c5528355c71c65571 verdict=changes-requested

Fix round 1: the review asked for changes.

loop:fix run=contribute-20261102-180727-68 round=1 issue=68

fix(form): key the foreign-repo warnings and drop them for strongRef fields

The foreign-repo warning from #12 no longer fires for strongRef fields. These are objects typed com.atproto.repo.strongRef or that have both uri and cid, such as a like's or repost's subject, a reply's root/parent, or a quote embed's record. Pointing at someone else's post is the normal case there. Fields that do make a record land in or count toward another repo still get the full warning: patch-record's Base Record URI, and plain at-uri fields such as a listitem's list. Each rendered warning now has a stable key (field + repo).

Fix round 1: the record builder stamps "$type": "com.atproto.repo.strongRef" on a like's subject but leaves out cid until it is filled in, so the warning used to show while the CID was still empty. An object with that $type now counts as a strongRef too.

Verified #

  • pnpm run check passes, and so does pnpm run test (2825 tests).
  • foreign-repo-refs.test.ts: a like's strongRef, a reply's strongRefs and a quote's embedded strongRef to another account give no refs. A list next to a strongRef is still found. A {uri} with no cid is still found. A {$type: "com.atproto.repo.strongRef", uri} with no cid gives no refs.
  • AutomationForm.test.tsx "target repo" cases:
    • A like and a reply pointing at another account's posts show no warning.
    • The listitem list still warns.
    • A like built through the record builder (like schema mocked, subject reached through the strongRef ref) with a foreign URI typed into #field-uri and #field-cid empty shows no warning.
    • Patch-record's foreign Base Record URI warns.
    • The new key test fails without the key and passes with it. It renders two warnings, clears the first through the input, and checks that the remaining warning keeps its own DOM element.
  • No new controls, so there was nothing new to try with the keyboard.

Acceptance criteria #

Decisions you can reverse #

  • No note for strongRefs. The issue allowed a soft note or none, and I picked none. A soft note would mean adding a kind to ForeignRepoRef and a second message.
  • How a strongRef is spotted. It is an object whose $type is com.atproto.repo.strongRef, or one with both uri and cid, as in the lexicon. A raw-JSON { "uri": ... } with neither still warns.
  • Warning text. I removed "That is fine for a like or a reply, but ..." from the full warning, because those cases no longer show it.

Review: https://tangled.org/exosphere.site/airglow/compare/main...loop/68

loop:pushed run=contribute-20261102-180727-68 sha=746c563be0f03f0f529024f82c90b060d3869452

Merge. The branch keys each foreign-repo warning and stops warning about strongRefs (a like's or repost's subject, a reply's root/parent, a quote's record). The round-1 gap is fixed: a strongRef the record builder has typed but whose CID is still empty no longer warns. Patch-record's foreign Base Record URI and a listitem's foreign list still warn. pnpm run check and pnpm run test (183 files, 2825 tests) both pass on 746c563.

What I checked and how:

  • Round 1 asked for two things, and both are done:
    • isStrongRef now also accepts an object whose $type is com.atproto.repo.strongRef.
    • Both tests are there: a typed { $type, uri } with no cid in foreign-repo-refs.test.ts, and a like built through the record builder with #field-cid empty in AutomationForm.test.tsx.
    • Nothing round 1 confirmed has broken: the key is still there, and patch-record and listitem still warn.
  • The builder test proves the fix: I removed the $type clause, the builder test failed, and it passed again once I put the clause back.
  • I confirmed the builder really stamps that exact $type string:
    • schema-tree.ts calls stampRef/qualifyRef, which turn com.atproto.repo.strongRef (def main) into the bare NSID.
    • RecordFormBuilder.serializeNode adds it to every non-empty object that came from a ref.
    • So reply root/parent (from #replyRef → strongRef) and a quote embed's record stay quiet while being filled in, just as the like does.
  • I checked every caller of the changed helper. findForeignRepoRefs is used only by draftForeignRepoRefs in registry.ts, and that is used only by renderActionBody in AutomationForm.tsx. The server-side mentions (lib/actions/bsky-list-add.ts, lib/actions/types.ts) are out of scope per the issue.
  • Each acceptance criterion against the code:
    • The key is field\nrepo, which is unique per action.
    • Foreign strongRefs give no warning; the issue allowed "none".
    • Foreign write-target fields still warn.
    • Both test files cover both sides.
  • UI: no new control and no new style. Only the warning text was trimmed, and it still sits next to the action it describes.

loop:verdict run=review-20261102-181448-68 sha=746c563be0f03f0f529024f82c90b060d3869452 verdict=merge

Merged main into the branch: 2f2293ab3ef645a581eb942997844f46d3d80c1f.

loop:carry from=746c563be0f03f0f529024f82c90b060d3869452 sha=2f2293ab3ef645a581eb942997844f46d3d80c1f

Merged onto main as 8b0a602c9f52dba9a2be2e5e067f0b55250a6d45.

loop:merged sha=8b0a602c9f52dba9a2be2e5e067f0b55250a6d45 head=2f2293ab3ef645a581eb942997844f46d3d80c1f

Sign up or Sign in to add to the discussion.