Skip to content

fix(api,cli): self-heal duplicate managed comments; re-sync after meta set#471

Merged
Zach Dunn (zachdunn) merged 1 commit into
mainfrom
claude/duplicate-bot-comments-meta-sync-bcb61c
Jul 23, 2026
Merged

fix(api,cli): self-heal duplicate managed comments; re-sync after meta set#471
Zach Dunn (zachdunn) merged 1 commit into
mainfrom
claude/duplicate-bot-comments-meta-sync-bcb61c

Conversation

@zachdunn

Copy link
Copy Markdown
Member

Closes #470.

What

1. Self-healing dedupe of the managed bot comment (server). upsertBotComment's marker hunt now collects every comment carrying the workspace's namespaced marker instead of stopping at the first. The oldest is kept and PATCHed; extras are DELETEd best-effort (a failed delete never fails the caller — the next sync retries). This makes the webhook-vs-CLI create race that duplicated the comment on PR #468 retroactively harmless: the next sync on an affected thread collapses the duplicates. Legacy (unnamespaced) marker comments are only ever adopted, never deleted, since they may belong to another workspace.

2. meta set refreshes the comment (CLI). uploads meta set on a gh/…-keyed object that touches a rendered key (path/state, per #370) now triggers a best-effort bot comment refresh; if the endpoint is unavailable it prints a tip: run uploads comment --pr N stderr hint instead of failing the metadata write. Untouched for non-gh keys and non-rendered keys.

Notes

  • The hunt now pages the whole thread when it runs (to collect duplicates) rather than early-returning on the first hit; it still runs at most ~once per comment thanks to the KV id cache.
  • Hosted-MCP set_metadata has the same re-sync gap; left for the Hosted MCP: comment parity for put (pr/issue targeting + managed comment sync) #392 comment-parity work.
  • TDD: new tests in apps/api/src/github-comment.test.ts and packages/uploads/test/commands-meta.test.ts; full suite (2842 tests) and typecheck pass.

…meta set

- upsertBotComment now collects every namespaced marker comment during the
  hunt, patches the oldest, and deletes the extras best-effort, so a webhook
  vs CLI create race no longer leaves a permanently stale duplicate
- meta set on a gh/-keyed object that touches path/state triggers a
  best-effort bot comment refresh, degrading to a stderr hint

Closes #470
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (2)
  • coderabbit:review
  • review
🚫 Excluded labels (none allowed) (1)
  • wip

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: CHILL

Plan: Pro

Run ID: 9263ba00-63ff-4667-a12e-459f5277320f

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/duplicate-bot-comments-meta-sync-bcb61c

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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
uploads-api 47da706 Commit Preview URL

Branch Preview URL
Jul 23 2026, 05:21 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
uploads-auth 47da706 Commit Preview URL

Branch Preview URL
Jul 23 2026, 05:21 PM

@zachdunn
Zach Dunn (zachdunn) merged commit ee057cc into main Jul 23, 2026
5 checks passed
@zachdunn
Zach Dunn (zachdunn) deleted the claude/duplicate-bot-comments-meta-sync-bcb61c branch July 23, 2026 17:29
Zach Dunn (zachdunn) added a commit that referenced this pull request Jul 23, 2026
…llow-up) (#476)

Closes three docs gaps from #471: the uploads-cli skill, the GitHub App
docs page, and the CLI README didn't yet mention that the server collapses
duplicate managed bot comments, or that `uploads meta set` on path/state
refreshes the comment (and what it prints when it can't).
This was referenced Jul 23, 2026
Zach Dunn (zachdunn) added a commit that referenced this pull request Jul 23, 2026
)

* fix(comment): dedupe duplicate managed comments on explicit resync

PR #471's self-healing dedupe only runs on the marker hunt, and
`upsertBotComment`'s fast path PATCHes the KV-cached comment id with no
listing — so while the cache is warm (30 days) a race-created duplicate
sat untouched until a cache miss forced a hunt.

Add `forceHunt` to `upsertBotComment` and plumb it through
`postManagedComment` (`resync`), the `/github/comment` route body, the
CLI client, and `syncAttachmentsComment`. The explicit resync callers —
`uploads comment`, both `comment` MCP tools, and the `meta set` refresh —
now pay one listing so a duplicate's lifetime is bounded by the next
explicit resync. Attach/screenshot/promote syncs keep the fast path.

Closes #480

* chore(impeccable): extend the broken-image waiver to the hosted MCP worker

PR #477 scoped the broken-image waiver to `packages/uploads/src/**`, which
never covered `apps/mcp/src/**` — so the hosted MCP worker kept reporting
the `<img width=…>` prose in its `put` tool-schema description.

apps/mcp/src is a headless JSON-RPC worker with no rendered surface; the
only `<img` in it is documentation of the markdown the tool emits. Same
class as the CLI, so it belongs on the existing entry rather than a second
one. Verified: detect reports nothing with the config and the finding
returns under --no-config, with the original glob still suppressing.

* chore(impeccable): waive broken-image by class instead of per-directory

Two rounds of per-directory patching left 11 latent findings that would
surface on unrelated edits: apps/api/src (7 comment-markdown builders) and
apps/web/src/lib (4, one an XSS test fixture that must stay malformed).

Waive the class the rule can't distinguish — code that EMITS markup as
strings, plus test fixtures — rather than disabling broken-image globally.
It stays live on apps/web/src/components/**, apps/web/src/pages/**, and
packages/ui/**, which scan clean today and are where a real broken image
would actually ship.

Verified: those four areas report zero with the config, and a planted
<img src=""> in a component, a page, and a UI package is still reported.
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.

Managed comment: create race duplicates the bot comment; no re-sync after meta set

1 participant