Skip to content

feat: add copyable message citation links - #128

Open
PollyBot13 wants to merge 9 commits into
openclaw:mainfrom
PollyBot13:feat/message-citations
Open

feat: add copyable message citation links#128
PollyBot13 wants to merge 9 commits into
openclaw:mainfrom
PollyBot13:feat/message-citations

Conversation

@PollyBot13

@PollyBot13 PollyBot13 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Closes #127

What Problem This Solves

Users and agents can find messages inside ClickClack, but cannot copy a stable, permission-checked link back to a channel message. Decisions and evidence therefore have to be referenced with copied text or vague navigation instructions.

Why This Change Was Made

Channel root messages now receive immutable public M... route IDs at creation or migration, while replies and new DM citation actions remain out of scope. Desktop and touch action surfaces copy one canonical absolute URL using the configured public frontend origin. Before a first reply the route highlights the root in its channel; afterward the same URL opens the thread pane.

Clipboard denial exposes a focused, selected, read-only URL rather than losing the citation. Existing route authorization remains authoritative, so possession of a link grants no access.

Channel and DM roots retain separate persistence paths: CreateMessage is channel-only and assigns the eager citation identity, while CreateDirectMessage preserves the existing lazy DM-thread identity behavior.

The upgrade backfill respects the stores’ actual schema histories: SQLite keys off 0011_public_route_ids.sql, while PostgreSQL keys off its initial 0001_schema.sql. This makes the citation backfill run on existing PostgreSQL installations instead of silently skipping them.

User Impact

People and integrations can cite a durable ClickClack discussion with a copyable link that remains stable as the message becomes a thread. Links use public route IDs only and preserve current workspace, channel, guest, and message permissions. Existing SQLite and PostgreSQL stores receive the same channel-root citation behavior during migration.

Evidence

Exact reviewed head: 98044b55ba89a6fe2f49a928eb3ec8a7ea8ea2f6, based on main at 4843afe2e8eb6bcfeba947496feae3db72e6f659.

Built-server browser proof

The production web bundle was built into the Go server and exercised against a fresh real SQLite store:

CLICKCLACK_E2E_PORT=<loopback-port> \
  pnpm exec playwright test tests/e2e/message-citations.spec.ts \
  --workers=1 --reporter=list --trace=on

1 passed

The exact-head scenario proves desktop and touch copy actions, configured-origin handling, direct-route highlight before replies, the same route opening the thread after its first reply, replies remaining without independent route IDs, and the accessible selected-URL fallback after clipboard denial.

All refreshed screenshots were visually inspected for rendered state, readable controls, stable URL, and layout integrity.

Existing-store upgrade proof

Focused real-store tests passed:

CLICKCLACK_POSTGRES_TEST_DSN=<local PostgreSQL 17 DSN> \
  go test ./apps/api/internal/store/postgres \
  -run 'TestMigrateBackfillsLegacyChannelRootRouteIDs|TestMessageRouteIDCreationRespectsChannelAndDirectBoundaries' \
  -count=1

go test ./apps/api/internal/store/sqlite \
  -run 'TestMigrateBackfillsRouteIDsOnce|TestRouteIDBackfillAllChannelRoots' \
  -count=1

They prove that legacy workspace, channel, direct-conversation, and channel-root rows receive the expected routes; direct-message roots and replies remain uncited; and the one-time completion marker is recorded.

Broader validation

  • pnpm generate:sqlc — passed with no generated drift.
  • umask 0027; pnpm check — passed.
  • git diff --check origin/main...HEAD — passed.
  • Independent exact-diff local Codex review — no correctness, security, or maintainability findings.
  • Hosted checks are rerunning for this head.

AI-assisted development: yes (Codex); all generated changes were reviewed and validated against the exact head above.

@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Exact head 661fe3143a64409a6716927ca97cee987e2397ce has all seven hosted checks green. The PR includes inspectable built-server/browser screenshots and reproduction covering desktop and touch copy, configured public origin, pre-reply highlight, stable first-reply routing, and clipboard-denial fallback. Full local checks, real PostgreSQL coverage, and an independent exact-head Codex review found no actionable regressions.

@clawsweeper

clawsweeper Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 25, 2026
@clawsweeper

clawsweeper Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed August 1, 2026, 1:34 PM ET / 17:34 UTC.

ClawSweeper review

What this changes

This PR gives channel-root messages stable public route IDs, adds copyable citation links and clipboard fallback in the web app, and backfills existing SQLite and PostgreSQL stores.

Merge readiness

⚠️ Needs maintainer review before merge - 4 items remain

Keep this PR open: the visible citation behavior has strong real-browser proof, but the changed legacy backfill now runs synchronously during store migration and can perform unbounded per-row work before the service is ready. The linked request remains open at #127, and this PR is its active implementation candidate rather than work already present on current main.

Priority: P2
Reviewed head: 98044b55ba89a6fe2f49a928eb3ec8a7ea8ea2f6

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Behavior proof is strong, but the source-visible unbounded startup migration is a material merge blocker.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (screenshot): The exact-head PR body supplies built-server browser evidence, real SQLite and PostgreSQL upgrade test evidence, and three prepared screenshots that directly show the copy, routing, and clipboard-fallback states.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (screenshot): The exact-head PR body supplies built-server browser evidence, real SQLite and PostgreSQL upgrade test evidence, and three prepared screenshots that directly show the copy, routing, and clipboard-fallback states.
Evidence reviewed 6 items Changed migration scope: The PR changes the message backfill predicate from roots with existing replies to every unassigned channel root, expanding the upgrade work to all legacy channel-root messages.
Unbounded per-row startup work: Both store implementations select every matching ID into memory and then issue one route-ID update per ID; there is no batch limit, cursor, checkpoint, or interruption-resume path.
Migration blocks readiness: SQLite and PostgreSQL invoke the route-ID backfill synchronously at the end of store migration; PostgreSQL performs migration work while holding its advisory migration lock.
Findings 1 actionable finding [P1] Bound the startup citation backfill
Security None None.

How this fits together

ClickClack assigns public route IDs to workspace content and resolves those IDs through existing permission checks into the web client. This feature makes channel-root messages addressable before replies exist so users can copy one durable URL that later opens the same root or its thread.

flowchart LR
  Root[Channel-root message] --> ID[Public M route ID]
  ID --> Store[SQLite or PostgreSQL store]
  Store --> Resolver[Authorized route resolution]
  Resolver --> Web[ClickClack web client]
  Web --> Copy[Copy-link actions]
  Copy --> URL[Stable citation URL]
Loading

Before merge

  • Bound the startup citation backfill (P1) - The changed predicate now selects every legacy channel root without a route ID. Migration calls this helper synchronously, and it first retains every ID in memory then executes one update per row, so a large existing store can delay readiness for an unbounded time (and PostgreSQL retains its migration lock). Process bounded, restart-safe batches and add an upgrade-scale regression before merge.
  • Resolve merge risk (P1) - On a large legacy installation, startup can remain unavailable while each unassigned channel root is loaded and updated one at a time; PostgreSQL holds the migration advisory lock throughout that work.
  • Resolve merge risk (P1) - The feature changes persisted route-ID coverage for existing rows, so the repair must preserve one-time, collision-safe, cross-store migration semantics and resume safely after interruption.
  • Complete next step (P2) - The remaining blocker is a narrow, source-proven upgrade-safety defect in two parallel store implementations and is suitable for a focused repair PR.

Findings

  • [P1] Bound the startup citation backfill — apps/api/internal/store/sqlite/route_ids.go:58-61
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 54 files changed; 932 additions, 81 deletions The proposal crosses persistence, migration lifecycle, runtime configuration, web UI, generated assets, documentation, and browser tests.
Legacy backfill targets 4 table passes; 1 newly expanded message-root pass The startup routine handles workspaces, channels, direct conversations, and now all eligible channel-root messages before recording completion.

Merge-risk options

Maintainer options:

  1. Make the backfill resumable (recommended)
    Replace the full synchronous scan with bounded batches and durable progress or an equivalent restart-safe mechanism in both stores, then add upgrade-scale coverage.
  2. Accept the startup cost explicitly
    Land the eager full-store backfill only if maintainers intentionally accept an unbounded one-time readiness delay for large existing deployments.
  3. Pause citation rollout
    Defer this feature if a safe migration design is not worth adding before introducing citation routes for all legacy channel roots.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Implement bounded, restart-safe route-ID backfill batches for legacy channel-root messages in SQLite and PostgreSQL; preserve existing route IDs, keep DM roots and replies out of scope, and add interrupted-upgrade plus scale-oriented regression coverage without editing CHANGELOG.md.

Technical review

Best possible solution:

Use a bounded, resumable cross-store route-ID backfill that records progress only after safely completing batches, then prove a large existing-store upgrade and restart/resume path without changing the durable citation URL contract.

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

Yes, source inspection gives a high-confidence path: start either store against a legacy database containing many channel-root rows without route IDs, then observe migration scan every matching row and update each synchronously before readiness. This review did not execute a large-store run because the checkout is read-only.

Is this the best way to solve the issue?

No: the UI and normal upgrade behavior are well proven, but the current all-at-once startup backfill is not the safest implementation for legacy installations. A bounded, restart-safe migration keeps the requested citation behavior while avoiding an unbounded readiness delay.

Full review comments:

  • [P1] Bound the startup citation backfill — apps/api/internal/store/sqlite/route_ids.go:58-61
    The changed predicate now selects every legacy channel root without a route ID. Migration calls this helper synchronously, and it first retains every ID in memory then executes one update per row, so a large existing store can delay readiness for an unbounded time (and PostgreSQL retains its migration lock). Process bounded, restart-safe batches and add an upgrade-scale regression before merge.
    Confidence: 0.98

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 4843afe2e8eb.

Labels

Label justifications:

  • P2: This is a useful user-facing capability with a concrete upgrade-safety defect, but no evidence of an active production outage.
  • merge-risk: 🚨 compatibility: The branch writes durable public route IDs into existing SQLite and PostgreSQL records during upgrade.
  • merge-risk: 🚨 availability: The migration runs synchronously at startup and can issue one update per legacy channel-root message without a bound.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): The exact-head PR body supplies built-server browser evidence, real SQLite and PostgreSQL upgrade test evidence, and three prepared screenshots that directly show the copy, routing, and clipboard-fallback states.
  • proof: sufficient: Contributor real behavior proof is sufficient. The exact-head PR body supplies built-server browser evidence, real SQLite and PostgreSQL upgrade test evidence, and three prepared screenshots that directly show the copy, routing, and clipboard-fallback states.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The exact-head PR body supplies built-server browser evidence, real SQLite and PostgreSQL upgrade test evidence, and three prepared screenshots that directly show the copy, routing, and clipboard-fallback states.

Evidence

Acceptance criteria:

  • [P1] go test ./apps/api/internal/store/sqlite -run 'TestRouteIDBackfillAllChannelRoots|TestMigrateBackfillsRouteIDsOnce' -count=1.
  • [P1] go test ./apps/api/internal/store/postgres -run 'TestMigrateBackfillsLegacyChannelRootRouteIDs' -count=1.
  • [P1] Add and run focused bounded/resume migration tests for both store packages.
  • [P1] pnpm generate:sqlc only if the repair changes SQL schema or query inputs.

What I checked:

  • Changed migration scope: The PR changes the message backfill predicate from roots with existing replies to every unassigned channel root, expanding the upgrade work to all legacy channel-root messages. (apps/api/internal/store/sqlite/route_ids.go:58, 98044b55ba89)
  • Unbounded per-row startup work: Both store implementations select every matching ID into memory and then issue one route-ID update per ID; there is no batch limit, cursor, checkpoint, or interruption-resume path. (apps/api/internal/store/postgres/route_ids.go:68, 98044b55ba89)
  • Migration blocks readiness: SQLite and PostgreSQL invoke the route-ID backfill synchronously at the end of store migration; PostgreSQL performs migration work while holding its advisory migration lock. (apps/api/internal/store/postgres/postgres.go:111, 98044b55ba89)
  • Existing positive behavior proof: Prepared screenshots visibly show the copy-link menu, direct-route highlight before replies, stable thread routing after a reply, and a selected URL fallback after clipboard denial; the PR body also reports an exact-head built-server Playwright run and real-store migration tests. (docs/proof/message-citation-lifecycle.png, 98044b55ba89)
  • Feature provenance: The pre-existing route-ID backfill behavior appears to date to the public-route-ID implementation; the PR changes its message selection and preserves the same unbounded helper structure. (apps/api/internal/store/sqlite/route_ids.go:64, 4e144f880bf6)
  • Not implemented on current main: The PR head is not contained by a local branch or release tag, while the checkout remains at current main; the feature is therefore not already merged or released. (98044b55ba89)

Likely related people:

  • Shakker: Commit 4e144f8 introduced the current route-ID backfill helper and its migration marker flow, which this PR extends for channel-root citations. (role: introduced public route-ID backfill behavior; confidence: high; commits: 4e144f880bf6; files: apps/api/internal/store/sqlite/route_ids.go, apps/api/internal/store/postgres/route_ids.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Implement bounded, restart-safe backfill behavior in both stores.
  • Add upgrade-scale and interrupted-resume coverage for the backfill.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (26 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-31T20:45:20.113Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T22:15:56.895Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T23:35:34.365Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T04:06:07.468Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T09:34:35.883Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T11:27:17.822Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T12:51:43.044Z sha 98044b5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T14:52:49.090Z sha 98044b5 :: found issues before merge. :: [P1] Bound the startup citation backfill

@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Exact head: 1d15a6395ad00690da9c291050ec1fb194e3842c

The persistent-route finding was traced to a source-path misread rather than a
production defect:

  • CreateMessage accepts only ChannelID, calls InsertChannelMessage, and is
    the channel-root writer that eagerly assigns M...;
  • CreateDirectMessage accepts ConversationID, calls InsertDirectMessage,
    and has no eager route assignment; and
  • the pre-existing SQLite regression already asserted that a new DM root has no
    route until EnsureThreadRouteID is used.

This head adds equivalent real-PostgreSQL regression coverage for eager channel
roots, lazy DM roots, and compatible lazy DM-thread allocation. Both complete
store suites and the full repository gate pass.

The durable PR body now embeds commit-pinned raw screenshots directly and links
the exact-head reproduction plus both store regressions. The built production
bundle/browser proof was rerun on this head (1 passed) and the refreshed
screenshots were visually inspected.

@clawsweeper

clawsweeper Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Jul 25, 2026
@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 26, 2026
@PollyBot13

Copy link
Copy Markdown
Contributor Author

@codex review

@PollyBot13
PollyBot13 force-pushed the feat/message-citations branch from 31e7c72 to a990518 Compare July 29, 2026 18:23
@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 29, 2026
@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Final head is 98044b55ba89a6fe2f49a928eb3ec8a7ea8ea2f6. The PostgreSQL historical-schema upgrade path now has real database coverage alongside SQLite, browser proof, pnpm check, and a clean final Codex review.

@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. 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. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add copyable links for channel messages and thread roots

1 participant