Delete the stored file with its record, localize setup messages, and document both - #313
Open
cf-pages wants to merge 8 commits into
Open
Delete the stored file with its record, localize setup messages, and document both#313cf-pages wants to merge 8 commits into
cf-pages wants to merge 8 commits into
Conversation
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
Deploying tpic with
|
| 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 |
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
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)message_idat upload time so the message can be deleted later.canDelete→deleteFile→bestEffortDelete) lives indeleteStoredFile()infunctions/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)?lang=→SITE_LANG→Accept-Language→zh(preserving the pre-i18n default).codeandparamsalongside the renderedmessage, so a frontend can localize on its own instead of displaying ours.Documentation
file_idfor some time — use the blacklist when a file must be certain to stop loading.Content-Typeor extension) under a 5MB threshold; video, audio and PDF are never reviewed.MODERATION_PROVIDERsilently falls back tononeon an unrecognized value whileSTORAGE_PROVIDERreturns HTTP 500, so spelling matters differently between the two.GET /api/configresponse shape and noted itsCache-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