Repository navigation
Fix review findings for Console 2.7.0: worker shutdown flush, RTL field layout, task badge - #88
Merged
Merged
Conversation
Embedded Console is usually opened at http://<ip>:9001, which is not a secure context. There crypto.randomUUID and navigator.clipboard are undefined, so "Calculate folder sizes" threw before sending its request and the decryption-key copy button did nothing, both without feedback. Use the shared makeid() task-ID helper, and copy the key with copy-to-clipboard (already shipped through react-copy-to-clipboard), keeping the field's single overlay button. Add a static guard against secure-context-only APIs and exercise the real Inspect dialog's copy on a plain-HTTP origin. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DaysSelector formatted the expiry with Luxon toFormat and ShareFile parsed it back with new Date(). toFormat falls back to en-US unless a locale is specified, which Console does not do, so links work today. A configured Luxon default locale such as ar-EG, ar-SA or fa, however, would emit native digits that new Date() rejects, and the share link would silently never be generated. Produce the timestamp with Luxon's technical ISO output, which never depends on the locale and is unchanged for every locale today, and guard it with a test. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without the usage database, Refresh scans buckets two at a time through prefix-usage, whose slots (two by default) are shared by every Console session. When another tab, user or folder scan held a slot, the server answered 429 and the bucket was immediately shown as "Update failed". Treat 429 as busy: wait with jittered, capped backoff (about 30 s in total), honour cancellation, and retry the same bucket before reporting a failure. Other status mappings are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
uploadFiles only stopped on an unwrapped io.EOF from NextPart. Any other error (a malformed part header, an empty or boundary-less body, or a body truncated after a boundary, which the standard library reports as a wrapped EOF) left the part nil and p.FormName() panicked inside the request handler. Return those errors to the caller. A wrapped EOF is deliberately not treated as the closing boundary, so a truncated upload is not accepted as complete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
console usage-worker built its scan client and change-stream observer on minio-go's default transport, so it trusted only the system roots. With a private CA in certs/CAs, console server connected to SILO while the worker failed with "certificate signed by unknown authority", and the worker ignored CONSOLE_MINIO_SERVER_TLS_SKIP_VERIFY. Give the worker the server's --certs-dir flag. It resolves the directory without creating it and loads only the trusted roots (system, certs/CAs and public.crt, never private.key), so a read-only or key-protected certs directory cannot stop it; console server's loading is unchanged. Pass the SILO endpoint transport, including its origin-scoped skip-verify policy, through a new usage.Config.Transport field used by both the scanner and the observer. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The worker bounded each bucket scan by the wall clock since the scan started. Fair scheduling yields a large bucket after every slice and puts it behind the others, so that clock also ran while the bucket waited for other buckets' slices. Once the buckets' combined listing time exceeded SCAN_TIMEOUT x CONCURRENT_SCANS they all ended limited/duration, although each one alone finished well within the budget. A limited checkpoint kept the expired start time, so automatic retries ended limited again at once, and only a manual Refresh cleared it, which requeued every bucket into the same race. Record the listing time spent so far in the checkpoint (listing_time_ns) and give each slice the remaining budget. A runaway listing is still bounded; time spent queued is no longer charged. Checkpoints written by earlier workers have no such field and resume with a full budget, so buckets left limited by this bug recover on their next retry; earlier workers ignore the new field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
When the worker stopped, every scan in progress was recorded through Fail as state=error/worker_shutdown with a retry backoff of at least RETRY_INTERVAL (30 s), so a rolling restart showed those buckets as "Update failed" and delayed their completion. Add Store.Release, which returns the leased job to the queue with its checkpoint and progress, ready immediately, without an error code or an attempt, and use it for worker_shutdown. Lease loss is still ignored. Also give the queue-wait budget test a wider timing margin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
The worker subscribed only to object events. Force-deleting a bucket removes its objects without per-object events and emits only s3:BucketRemoved, and incremental reconciliation leaves existing rows alone. A bucket re-created under the same name, possibly by another owner, therefore showed the deleted bucket's totals until an object event, a worker restart or the 24-hour full reconciliation. Subscribe to s3:BucketCreated and s3:BucketRemoved. On removal, clear the row's results and checkpoint, fence any scan in progress by dropping its lease, and queue a rescan, which either retires the row (MissingBucket) or measures the re-created bucket. Coalescing keeps a deletion even when a later event for the same bucket arrives in the same batch. Root ListenNotification permission already covers bucket events, so the worker policy is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
authorizeBucketUsage treated a listing channel that closed without an object as an authorized, empty bucket. minio-go also closes the channel without an error when the context is canceled, and select may take that branch before ctx.Done(), so a canceled request could pass the check. It is safe today only because the later database calls share the canceled context and fail. Return ctx.Err() when the channel closes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
When the budget runs out, the listing channel closes as the scan context expires and select may take either case. The closed-channel exit returned limited/duration without saving the checkpoint, so the database kept the listing time of the last progress write and an automatic retry listed again instead of ending at once. Persist the checkpoint there as well, as the other duration exits do, and cover the race with a busy lister. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A shutdown that cancels a checkpoint, yield or completion write makes RunSlice return the database error rather than worker_shutdown, so the job skipped Release and stayed running until its lease expired (about two minutes by default). Release whenever the worker's context is canceled; the lease guard keeps it a no-op for a job already completed, yielded or taken over. Move the settlement into settleScan to test it deterministically. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Received changes that failed to persist were restored into a map owned by one observer lease and dropped when that observer exited. Reconnect reconciliation re-queues every bucket but cannot tell that a bucket was deleted and re-created, so a deletion lost to a transient database failure left a limited row's old totals in place. Keep the pending changes for the life of the worker and flush them on every observer loop, with or without the lease. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rebuild web-app/build with Node 24.22.1 for the plain-HTTP, share-link expiry and busy-scanner fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
Record the pre-release fixes in the v2.7.0 CHANGELOG section, join its list, and describe the source as 2.7.0 without draft or candidate wording. The v2.7.0 tag is recut on this commit: the earlier tag on 26ce922 only ever produced an unpublished draft. The embedded assets already carry version 2.7.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
The observer flushes received changes only from its event loop. A shutdown that interrupted a reconciliation, at startup, after a reconnect or during the periodic safety net, returned without that flush, so a bucket deletion received meanwhile was dropped when the process exited. Reconnect reconciliation cannot recover it: a limited row keeps its totals and exhausted checkpoint, so a re-created bucket kept showing the old bucket's totals. Flush the pending changes once more when the observer stops. The lease loop waits for the event consumer to finish before returning, so no change arrives after the final flush. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
The RTL conversion rewrote the badge's `right: -1px` as `insetinlineend: -1px`, an unknown CSS property that browsers drop, so the badge on the task button lost its corner position in every language. Use `inset-inline-end`, and guard against run-together logical property names in style templates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
Password, URL, e-mail and number inputs stay LTR in Arabic, so their reserved end padding is on the right, but the overlay button followed the RTL container to the left. Without a start icon, as in user and password forms, the reveal button covered the start of the text; unit selectors on number fields sat over the digits. Lay out these fields' container LTR too, keep error text in its own direction, and check the Arabic login field's geometry in the browser. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
Record the review fixes in the v2.7.0 CHANGELOG section and rebuild the embedded GUI for the badge and RTL field-layout fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
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.
Fixes from the full pre-release review of Console 2.7.0 (
v2.6.0..2aae2c169, Codex review triaged by Claude). This PR stacks on pgsty/silo-console#87. After #87 lands, it is a fast-forward of main, andv2.7.0is recut on its last commit.insetinlineend: -1px, an unknown property written during the RTL conversion; it wasright: -1pxin v2.6.0. The badge is back in its corner, and a static guard rejects run-together logical property names in style templates.Not changed: the
/api/v1/docsSwagger page under a sub-path reverse proxy (P3, docs page only, also broken in 2.6.0).Local checks:
go test ./...pass; golangci-lint reports 0 issues; the usage PostgreSQL integration suite passes.🤖 Generated with Claude Code