Skip to content

fix(tlon): keep fragment image URLs as media blocks#104867

Open
arkyu2077 wants to merge 2 commits into
openclaw:mainfrom
arkyu2077:fix/104852-tlon-fragment
Open

fix(tlon): keep fragment image URLs as media blocks#104867
arkyu2077 wants to merge 2 commits into
openclaw:mainfrom
arkyu2077:fix/104852-tlon-fragment

Conversation

@arkyu2077

@arkyu2077 arkyu2077 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix Tlon media URL classification so image URLs with #fragment suffixes still render as block.image verses.
  • Add focused regression coverage for fragment-suffixed image URLs and keep non-image fragment URLs on the existing link path.
  • Keep the change intentionally narrow: only the image-extension matcher changes.

Why does this matter now?

  • The current behavior silently downgrades valid image media into plain links for a reproducible outbound Tlon case.

What is the intended outcome?

  • buildMediaStory() should treat https://cdn.example/image.png#preview the same way it already treats query-suffixed image URLs.

What is intentionally out of scope?

  • Broader URL normalization or changes to non-image link handling.

What does success look like?

  • Fragment-suffixed image URLs produce an image block instead of a link block.

What should reviewers focus on?

  • The image URL matcher change and the new regression coverage around buildMediaStory().

Linked context

Which issue does this close?

Closes #104852

Which issues, PRs, or discussions are related?

Related #104852

Was this requested by a maintainer or owner?

  • No; external bug-fix PR against a small reproducible issue.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: Tlon outbound image URLs with #fragment were emitted as link verses instead of image verses.
  • Real environment tested: local OpenClaw checkout on macOS (Darwin arm64) using the real buildMediaStory() implementation via pnpm tsx.
  • Exact steps or command run after this patch:
    • Before (current main): CI=true pnpm tsx -e "import { buildMediaStory } from './extensions/tlon/src/urbit/send.ts'; console.log(JSON.stringify(buildMediaStory('caption', 'https://cdn.example/image.png#preview'), null, 2));"
    • After (this branch): pnpm tsx -e "import { buildMediaStory } from './extensions/tlon/src/urbit/send.ts'; console.log(JSON.stringify(buildMediaStory('caption', 'https://cdn.example/image.png#preview'), null, 2));"
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):
[
  {
    "inline": ["caption"]
  },
  {
    "block": {
      "image": {
        "src": "https://cdn.example/image.png#preview",
        "height": 0,
        "width": 0,
        "alt": ""
      }
    }
  }
]
  • Observed result after fix: the fragment-suffixed PNG is emitted as a block.image verse.
  • What was not tested: a live Tlon server round-trip.
  • Proof limitations or environment constraints: this proof exercises the exact OpenClaw story-building path locally, but not a full outbound Tlon delivery session.
  • Before evidence (optional but encouraged):
[
  {
    "inline": ["caption"]
  },
  {
    "inline": [
      {
        "link": {
          "href": "https://cdn.example/image.png#preview",
          "content": "https://cdn.example/image.png#preview"
        }
      }
    ]
  }
]

Tests and validation

Which commands did you run?

  • pnpm vitest run extensions/tlon/src/urbit/send.test.ts
  • CI=true pnpm tsx -e "import { buildMediaStory } from './extensions/tlon/src/urbit/send.ts'; console.log(JSON.stringify(buildMediaStory('caption', 'https://cdn.example/image.png#preview'), null, 2));" (on main for before-proof)
  • pnpm tsx -e "import { buildMediaStory } from './extensions/tlon/src/urbit/send.ts'; console.log(JSON.stringify(buildMediaStory('caption', 'https://cdn.example/image.png#preview'), null, 2));" (on this branch for after-proof)

What regression coverage was added or updated?

  • Added buildMediaStory() tests for fragment-suffixed image URLs and non-image fragment URLs.

What failed before this fix, if known?

  • The before-proof command on main returned a link verse for image.png#preview instead of an image verse.

If no test was added, why not?

  • Added focused regression tests.

Risk checklist

Did user-visible behavior change? (Yes/No)

  • Yes

Did config, environment, or migration behavior change? (Yes/No)

  • No

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

  • No

What is the highest-risk area?

  • URL classification around image extensions.

How is that risk mitigated?

  • The matcher change is minimal and regression coverage keeps non-image fragment URLs on the link path.

Current review state

What is the next action?

  • Reviewer confirmation that fragment-suffixed image URLs should follow the same media path as query-suffixed image URLs.

What is still waiting on author, maintainer, CI, or external proof?

  • Waiting on CI and maintainer review.

Which bot or reviewer comments were addressed?

  • None yet.

@openclaw-barnacle openclaw-barnacle Bot added channel: tlon Channel integration: tlon gateway Gateway runtime size: M labels Jul 12, 2026
@arkyu2077
arkyu2077 force-pushed the fix/104852-tlon-fragment branch from b4c3f5a to d3e9f4d Compare July 12, 2026 01:42
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 11, 2026, 9:54 PM ET / July 12, 2026, 01:54 UTC.

Summary
The branch makes Tlon image-extension detection accept URL fragments and adds focused story-builder tests, while also adding hooks-token config validation and expanded gateway error guidance.

PR surface: Source +12, Tests +44. Total +56 across 5 files.

Reproducibility: yes. from source with high confidence: current main's matcher excludes fragment suffixes and its caller falls back to a link verse. This review did not independently execute the contributor branch, so the structured status is source-reproducible rather than reproduced.

Review metrics: 1 noteworthy metric.

  • Config validation surfaces: 1 fail-closed rule reintroduced. The branch adds a general rejection for enabled hooks without a token, outside the stated Tlon fix and prior to runtime resolution.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #104852
Summary: This PR is the candidate fix for the linked Tlon issue; the embedded hooks proposal is independent branch ancestry rather than part of the same root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase or cherry-pick the Tlon-only commit onto current main and rerun the focused story-builder test on the updated head.

Risk before merge

  • [P1] Merging the current head would make hooks.enabled: true without a token fail general config validation, an unrelated compatibility-sensitive change that could stop previously loadable configurations earlier in startup.

Maintainer options:

  1. Rebase to the Tlon-only fix (recommended)
    Place the Tlon commit directly on current main and remove all three hooks/config files from this PR.
  2. Reconsider hooks separately
    If config-time hooks validation remains desirable, restore it as an independent proposal with current maintainer direction and fresh-install and upgrade evidence.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Rebase or cherry-pick the Tlon-only commit onto current main, preserve only the two Tlon file changes, remove all hooks/config changes, and rerun focused Tlon validation.

Next step before merge

  • [P2] The blocking defect has a mechanical repair: preserve the Tlon commit while removing its unrelated hooks-validation parent.

Security
Cleared: The diff adds no dependency, workflow, permission, downloaded-code, package-resolution, or secret-access surface; the blocker is unrelated config compatibility rather than a security regression.

Review findings

  • [P1] Remove the unrelated hooks validation commit — src/config/validation.ts:1601
Review details

Best possible solution:

Rebuild the branch from current main with only the fragment-aware Tlon matcher and its regression tests; reconsider hooks validation only in a separate proposal with explicit compatibility and upgrade review.

Do we have a high-confidence way to reproduce the issue?

Yes, from source with high confidence: current main's matcher excludes fragment suffixes and its caller falls back to a link verse. This review did not independently execute the contributor branch, so the structured status is source-reproducible rather than reproduced.

Is this the best way to solve the issue?

The two-file Tlon matcher change is the narrowest maintainable fix. The submitted branch is not the best merge unit until the unrelated closed hooks proposal is removed.

Full review comments:

  • [P1] Remove the unrelated hooks validation commit — src/config/validation.ts:1601
    This line reintroduces the config-time behavior from the separately closed fix: validate hooks.token requirement at config.patch time #54831, so merging this Tlon fix would also make hooks.enabled: true without a token fail general config validation. That compatibility-sensitive change is unrelated to the linked bug and contradicts the PR body's claim that only the image matcher changes; rebase or cherry-pick the Tlon commit onto current main so the three hooks/config files leave the diff.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7a456e362d0b.

Label changes

Label justifications:

  • P2: The intended Tlon regression is channel-specific and has a clear bounded fix, but the current branch requires cleanup before merge.
  • merge-risk: 🚨 compatibility: The unrelated parent changes config acceptance and can prevent existing hooks configurations from loading.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): Copied before/after output from the real buildMediaStory() implementation directly demonstrates the changed verse classification; a live Tlon round-trip would be stronger but is not required for this pure story-construction change.
  • proof: sufficient: Contributor real behavior proof is sufficient. Copied before/after output from the real buildMediaStory() implementation directly demonstrates the changed verse classification; a live Tlon round-trip would be stronger but is not required for this pure story-construction change.
Evidence reviewed

PR surface:

Source +12, Tests +44. Total +56 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 14 2 +12
Tests 2 44 0 +44
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 58 2 +56

Acceptance criteria:

  • [P1] pnpm test extensions/tlon/src/urbit/send.test.ts.
  • [P1] git diff --check.
  • [P1] git diff --name-only 7a456e3...HEAD.

What I checked:

Likely related people:

  • arthyn: Authored the major merged Tlon expansion that introduced and maintained the central story-building and media paths. (role: feature owner; confidence: high; commits: f4682742d9d1; files: extensions/tlon/src/urbit/story.ts, extensions/tlon/src/urbit/send.ts)
  • xingzhou: Most recently refactored the Tlon send path that calls the affected story-building behavior. (role: recent area contributor; confidence: medium; commits: 57c2d65ad532; files: extensions/tlon/src/urbit/send.ts, extensions/tlon/src/urbit/send.test.ts)
  • steipete: Introduced the Tlon plugin and authored the original gateway hooks resolution path implicated by the unrelated parent commit. (role: original introducer and adjacent owner; confidence: high; commits: 791b568f7825, 6ae51ae3debf; files: extensions/tlon/src/urbit/story.ts, src/gateway/hooks.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: tlon Channel integration: tlon gateway Gateway runtime merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tlon media URLs with fragments are sent as links instead of image blocks

2 participants

@arkyu2077 @JasmineZhangHM