Skip to content

Delete the stored file with its record, localize setup messages, and document both - #313

Open
cf-pages wants to merge 8 commits into
mainfrom
claude/recent-changes-docs-review-ielx4h
Open

Delete the stored file with its record, localize setup messages, and document both#313
cf-pages wants to merge 8 commits into
mainfrom
claude/recent-changes-docs-review-ielx4h

Conversation

@cf-pages

@cf-pages cf-pages commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Summary

Deleting a file in the dashboard used to remove only its KV record, leaving the stored file behind — a billable R2 object nothing pointed at, or a Telegram channel message for a file no longer findable. This makes the delete go through the storage provider, localizes the deployment self-check messages that were Chinese-only, and updates both READMEs to match.

Code changes

Deleting removes the stored file (functions/storage/, functions/api/manage/delete/[id].js)

  • R2 objects are deleted from the bucket; Telegram uploads now record their channel message_id at upload time so the message can be deleted later.
  • A failed R2 delete keeps the record and returns 500, so the row stays in the dashboard and the delete can be retried instead of leaving an orphaned object. A failed Telegram delete is logged and the record still goes — the message may simply be gone already, and it costs the deployment nothing.
  • When the provider could never delete the file (bucket binding removed, or a file uploaded before message ids were recorded), the record is dropped anyway so the row cannot become permanently undeletable.
  • The three-part policy (canDeletedeleteFilebestEffortDelete) lives in deleteStoredFile() in functions/storage/index.js, not in the HTTP handler, so a second delete path does not have to rediscover the ordering.

Localized setup messages (functions/utils/i18n.js, functions/utils/setup-status.js, functions/api/config.js)

  • The self-check diagnostics were Chinese-only, so English deployments read Chinese error text. Wording now lives in one catalog per language, resolved in the order ?lang=SITE_LANGAccept-Languagezh (preserving the pre-i18n default).
  • Each problem still carries its code and params alongside the rendered message, so a frontend can localize on its own instead of displaying ours.

Documentation

  • Documented the new delete behavior and its two caveats: files predating recorded message ids can only have their record removed, and Telegram may keep serving a deleted message's file by file_id for some time — use the blacklist when a file must be certain to stop loading.
  • Corrected the update-log entry that still described deletes as record-only, which contradicted the Image Management section.
  • Documented that image review applies only to image files (by Content-Type or extension) under a 5MB threshold; video, audio and PDF are never reviewed.
  • Clarified that MODERATION_PROVIDER silently falls back to none on an unrecognized value while STORAGE_PROVIDER returns HTTP 500, so spelling matters differently between the two.
  • Added the GET /api/config response shape and noted its Cache-Control: no-store.

Testing

npm test — 124 unit tests pass, including new coverage for the R2 and Telegram delete paths, the retryable-vs-best-effort split, locale negotiation, and the message-id round trip through upload and metadata rewrites.

Not verified against a live runtime: the Pages project returns Cloudflare error 1101 on both the preview and production URLs, independent of this branch (a fresh successful build of this head still 1101s), so the delete paths have unit coverage only.

https://claude.ai/code/session_0156G916tsakziUoofzJ25x9

claude and others added 2 commits July 25, 2026 05:45
The provider release (#308) documented its new environment variables, but
a few behaviors that surprise operators were left out:

- image review only inspects images: files are sent to the model when the
  Content-Type is image/* or the extension is a known image type, and
  images over 5MB are skipped, so video/audio/PDF uploads are never
  reviewed — point readers at whitelist mode when they need every file
  gated
- an unrecognized MODERATION_PROVIDER silently disables review, while an
  unrecognized STORAGE_PROVIDER fails uploads with a 500
- with R2, Cloudflare's request body limit (100MB on Free) becomes the
  per-file cap, and deleting a file in the dashboard leaves the object in
  the bucket, still billable
- document the GET /api/config response shape, since the docs invite
  custom frontends to build against it
- fix the update log date of the release entry, which duplicated the date
  of the preceding entry

Both languages stay in sync (25 matching headings). Tests: 88 passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying tpic with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3466d12
Status: ✅  Deploy successful!
Preview URL: https://be17b79b.tpic.pages.dev
Branch Preview URL: https://claude-recent-changes-docs-r.tpic.pages.dev

View logs

claude added 5 commits July 25, 2026 07:11
Resolved the update log conflict in README-zh.md and fixed two things the
merge exposed:

- main added a "deployment self-check and test infrastructure" entry to
  the Chinese update log but not to the English one, so README.md was
  missing it entirely — added the English entry
- main's new entry claimed July 25, which collided with the date fix on
  this branch; the pluggable storage/review entry is now dated July 24,
  its actual landing date, so all three entries stay distinct

Also refreshed the documented GET /api/config response, since #309 added
the ready/setup/problems fields after this branch documented the shape.

Tests: 99 passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
A merge of main was pushed to this branch concurrently. Its resolution
dropped main's "deployment self-check and test infrastructure" update log
entry from README-zh.md and left the July 25 date collision in place, and
it predates the GET /api/config ready/setup/problems fields.

The resolution on this side is a superset of it — same content, plus that
entry restored in both languages and the new response fields documented —
so the tree from this side is kept.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
Two problems the docs review turned up, both fixed rather than documented.

Deleting a file in the dashboard removed only the KV record, so the R2
object stayed in the bucket with nothing pointing at it, still billed as
stored bytes. Deletes now go through the storage provider:

- r2 gains deleteFile/canDelete; telegram deliberately has neither, since
  no message id is kept and its files cannot be removed
- a failed bucket delete keeps the record and returns 500, so the row
  stays visible and the delete is retryable instead of orphaning silently
- a missing binding (switched back to Telegram) drops the record anyway,
  because the object is out of reach for good and the row would otherwise
  be undeletable

The deployment self-check answered in Chinese only, so English
deployments read Chinese diagnostics. utils/i18n.js now negotiates a
locale (?lang=, then SITE_LANG, then Accept-Language, then zh — the
previous behavior when nothing matches) and holds the en/zh catalogs.
setup-status emits { severity, code, params } and the wording is rendered
per request; /api/config echoes the negotiated locale, and the homepage
labels follow it so a message never sits under a mismatched-language
heading. Problem codes and params are part of the response, so a custom
frontend can supply its own wording.

Docs: the R2 note and dashboard note now describe deletion as it works,
SITE_LANG is in the configuration reference, and the /api/config section
covers locale negotiation and the problem codes. Both languages in sync.

Tests: 117 passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
Telegram-backed files can be deleted after all: deleteMessage removes the
channel message, and the message id was already in the sendPhoto response
we discarded. The previous commit claimed Telegram files could not be
removed, which confused "we never recorded the id" with a real limit.

- uploads record the channel message id; the storage contract's upload now
  returns { id, metadata } so a provider can persist its own fields
- normalizeMetadata carries messageId through, since serving a file
  rewrites its metadata and dropping it would silently make the file
  undeletable after its first view
- telegram gains canDelete/deleteFile: files uploaded before ids were
  recorded report canDelete false, so their row is still removable
- a failed Telegram delete is best effort (the message may already be
  gone, and the file costs the deployment nothing), while a failed R2
  delete stays fatal and retryable because those bytes are billed

Docs and both dashboards now say deletion removes the stored file, with
the two honest caveats: the bot must still be a channel admin allowed to
delete messages, and Telegram may keep serving a deleted message's file by
file_id for a while, so the blacklist remains the way to guarantee a file
stops loading.

Verified the R2 path end-to-end against wrangler after the contract
change: upload -> 200, delete -> object gone, refetch -> 404.

Tests: 124 passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
…ication

Deleting a stored file needed canDelete, deleteFile and bestEffortDelete
combined in exactly one order, and that combination lived in the delete
endpoint — so any second delete path would have had to rediscover it. Move it
into storage/index.js as deleteStoredFile(), leaving the endpoint with a single
retryable branch.

Also drop duplication the delete work introduced: deleteTelegramMessage now
goes through sendToTelegram instead of hand-rolling the bot API call (and gains
its network retry), r2's deleteFile reuses validateConfig for the binding check,
SUPPORTED_LOCALES derives from the message catalog it must match, and the
Accept-Language parse loses an index field that stable sort already covers.

The record delete and the short-link delete are independent, so they now run
together rather than one after the other.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
@cf-pages
cf-pages requested a review from Copilot July 27, 2026 03:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ource file

Entry 8 of the update log described the dashboard delete as it behaved when the
feature was introduced — record only, source file left on Telegraph/Telegram.
That now contradicts the same file's Image Management section, which documents
the delete going through the storage provider. Point the entry at that section
instead of restating the old behavior.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0156G916tsakziUoofzJ25x9
@cf-pages cf-pages changed the title Docs: Clarify image moderation scope and R2 storage behavior Delete the stored file with its record, localize setup messages, and document both Jul 27, 2026
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.

4 participants