Feat/s3 file browser v1 by dklOrdix · Pull Request #208 · stackabletech/cockpit

dklOrdix · 2026-06-18T11:08:13Z

Closes #129

…in +layout.svelte

…ebar imports Co-authored-by: Copilot <copilot@github.com>

Co-authored-by: Copilot <copilot@github.com>

…s/utils Co-authored-by: Copilot <copilot@github.com>

Co-authored-by: Copilot <copilot@github.com>

feat(storage): initial Filebrowser UI implementation

Co-authored-by: Copilot <copilot@github.com>

…ation S3 File Browser Page Navigation (Pagination)

* #96 disable the storage browser if feature flag is set * #96 fix svelte-check * #96 fix lint * #96 disable storage browser by default

* #102 refactor pagination controls into a separate component * #102 simplified pagination props

* #93 add modal preview for file display * #93 fix broken e2e tests * #93 add preview for parquet files, fix scrollbar visibility in preview, allow ms excel csv data for preview * #93 improve parquet row fetch handling * #93 fix lint * #93 move preview endpoint logic into separate files * #93 disable parquet preview

* refactor(ObjectTable): extract File and Folde rows into seperate components * style: format code * style: use camelCase * style: use camelCase

* feat: implement single-file download functionality and error handling for S3 storage * feat: refactor S3 download endpoint to use new API structure and improve error handling * chore: remove obsolete note about static download route segment in tech debt documentation * style: format code * fix: improve type safety in S3 error handling * fix: preview action * feat: add context menu header and improve file selection handling in FileExplorer * fix: reset selection state on context menu close and refine download action logic * style: format files * style: remove pointer events from Actions row on ContextMenu * #91 fix lint --------- Co-authored-by: F3l1x1vo <fli@ordix.de>

stackable-cla · 2026-09-10T10:12:42Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ F3l1x1vo
❌ dklOrdix
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

# Conflicts: # TECH_DEBT.md # src/lib/server/trino/queries.ts # vite.config.ts

Bohreromir · 2026-09-16T09:21:57Z

 const chromiumExecutablePath = process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH;

 export default defineConfig({
+  workers: process.env.CI ? 1 : 1,


I see, but the env.ci check does nothing so it can be set to 1 directly.

And please add a comment, so we know in the future

Bohreromir

A lot of the Playwright tests fail for me, do they for you too?

Here is some claude as a starting point:

storage/preview.spec.ts — PDF preview: timed out waiting for tbody tr or "This bucket is empty" after navigating to a bucket route.
storage/recent-items.spec.ts — removes a deleted file from Recent Files: timed out (60s, ×3 attempts via retries) waiting for the "Toggle selection mode" button.
storage/recent-items.spec.ts — removes files/location when a directory is deleted: Error: Channel closed / browser already closed mid-action.

The screenshots are the key clue. In both failures #1 and #2, the captured screenshot shows the Storage overview page (bucket card grid + Recent Files tab) — not the bucket browsing page the test expected. So page.goto(bucketRoute(...)) is redirecting back to /storage instead of opening the bucket.

This is masked by a bug in the test helper waitForObjectsLoaded() (e2e/storage/helpers.ts:185-192): it waits for tbody tr or "This bucket is empty" as a generic "did objects load" signal. But the Storage overview page's Recent Files tab also renders a

, so when the redirect happens and Recent Files has an entry, that locator matches by coincidence — the helper reports success on the wrong page. The test then tries to click "Toggle selection mode" (a bucket-only control), which never appears, and burns the full 60s test.slow() timeout, ×2 retries.

Bohreromir

Please also fix the new lint warnings introduced:

security/detect-non-literal-fs-filename (11)
- src/architecture/code-size-limits.spec.ts:23,38,53
- src/architecture/helpers.ts:57 (plus detect-non-literal-fs-filename for readdirSync at :29)
- src/architecture/i18n-compliance.spec.ts:39
- src/architecture/server-logging.spec.ts:29,56
- src/architecture/ui-pattern-enforcement.spec.ts:35,47,73,91

svelte/no-navigation-without-resolve (8):
- src/lib/components/storage/landing/RecentItems.svelte:132,158,195,203,216
- src/routes/(app)/settings/connections/[id]/edit/+page.svelte:77
- src/routes/(app)/storage/+error.svelte:86
- src/routes/(app)/storage/+layout.svelte:42,45

security/detect-non-literal-regexp (1):
- e2e/storage/archive.spec.ts:174

F3l1x1vo · 2026-09-17T12:57:22Z

A lot of the Playwright tests fail for me, do they for you too?

Sometimes they do fail locally for me when developing under resource strain. This flakiness also happens in the GitHub Actions. I've hopefully improved this in b94619d.

Bohreromir · 2026-09-18T08:13:35Z

 Migrations are stored in `src/lib/server/migrations/`.

+The current migration history is a single baseline migration. Existing databases
+created with the prior migration sequence must be reset before applying it.


Wait, is this deployed somewhere?

Bohreromir · 2026-09-18T08:16:13Z

+/**
+ * Shared helpers for architecture fitness tests.
+ *
+ * ArchUnitTS only scans TypeScript (.ts) source files. Svelte (.svelte) and


Bohreromir · 2026-09-18T08:20:46Z

+      'svelte/no-navigation-without-resolve': 'error',
+      'svelte/no-restricted-html-elements': [
+        'error',
+        { elements: ['dialog'], message: 'Use the shared Modal component instead of <dialog>.' }


ha, smart! ill see that one in the future :D

Bohreromir · 2026-09-18T08:27:32Z

- * conflict  - new name already exists in the bucket
- */
-export type RenameState = 'idle' | 'editing' | 'checking' | 'ok' | 'conflict';
+export type { Resolution, RenameState };


is this reexporting intentional?

Bohreromir · 2026-09-18T08:30:53Z

+| Server / Client Boundary | `src/lib/client`, `stores`, `storage`, `editor`, `types` must not import `src/lib/server/**`                         |
+| No Circular Dependencies | `src/lib/**` (excluding Trino) and `src/routes/**` must be cycle-free                                                |
+| Naming Conventions       | Stores → `*.svelte.ts`; lib components → PascalCase `.svelte`; server files → no `.svelte.ts` extension              |
+| Code Size Limits         | `.ts` files < 2 400 LOC; `.svelte` files < 1 100 LOC; test files < 1 000 LOC                                         |


Bohreromir · 2026-09-18T10:59:06Z

+  const isLegacyMode = $derived(text !== '' && headers.length === 0);
+  const isSimpleMode = $derived(!isLegacyMode && !fetchRows && headers.length > 0);
+
+  // ── Legacy mode: text-based CSV rendered via PapaParse ──


This has got nothing todo with legacy in this implementation. The "legacy" code path is used exclusive for streaming tsv. Please name it accordingly.

On another note: Whats the reason for differentiation between csv and tsv? Shouldn't make a difference what the limiter is for presenting it to the user

Bohreromir · 2026-09-18T11:44:12Z

+  let name = $state('');
+  let inputEl = $state<HTMLInputElement | null>(null);
+
+  const NAME_INVALID_CHARS = /[^\w\s./()\-+@,:;!$*'=]/g;


This also exists in StorageBreadcrumb.svelte:82-86.
Both also duplicate the === '.' || === '..' check

And Rename.modal has neither of them, so one can just create a valid file and then rename it to something bad.

And this is where it gets even worse, this is only client side validation, not server side!

Please write a shared zod schema, validate it here and in RenameModal, the breadcrumb and then finally in create/move/copy's +server.ts handlers

Bohreromir · 2026-09-18T11:58:17Z

+  const connections = rows.map((row) => {
+    let endpoint: string | null = null;
+    try {
+      const payload = JSON.parse(decrypt(row.encryptedPayload, getKey())) as {


as only verifies buildtime. This check and if (!body.sourceKeys?.length) below can be checked with a zod schema to protect the runtime too.

in api/trino/query/+server.ts via Schema.safeParse(body) its already done correct.

The other endpoints also miss zod (pi/storage/copy, move, delete, connections +server.ts)

Bohreromir · 2026-09-18T12:00:19Z

+  if (rows.length === 0) return null;
+
+  try {
+    const payload = JSON.parse(


please use zod here as well as in settings/connections/**/+page.server.ts

we already have a shcema here: src/lib/storage/schemas.ts

Bohreromir · 2026-09-18T12:06:50Z

+  });
+
+  it('maps access_denied code', () => {
+    expect(getActionErrorMessage(new StorageError('access_denied', ''))).toBe('Access denied');


not to sure about these tests. We set the label to something static and the test if the msg is realy the msg we set in this file. What are we testing in here?

Bohreromir · 2026-09-18T12:31:12Z

+  key: Buffer
+): string {
+  const hmacKey = deriveHmacSubkey(key);
+  const material = [


this can cause collides.
claude suggests:
Fix: use an encoding where the boundary is unambiguous regardless of field content, e.g. length-prefix each field before concatenating (${endpoint.length}:${endpoint}${region.length}:${region}...), or hash each field independently and concatenate the fixed-size digests instead of the raw strings.

Bohreromir · 2026-09-18T12:42:51Z

+
+// ── ActionError (backward-compatible alias) ──────────────────────────────────
+
+export class ActionError extends StorageError {


pls remove, and in clipboard.svelte.ts:296 remove that err instanceof ActionError as it would never have been true anyways. just check for instanceof StorageError

Bohreromir · 2026-09-18T12:45:48Z

+  tabs: PersistedTab[];
+  activeTabId: string;
+  /** Fingerprint of the connection that saved these tabs.
+   *  Absent in data saved before this field was introduced (treated as a match


What Model are you using? It really does like to add backwards compatibility were its not needed.

Bohreromir · 2026-09-18T12:46:46Z

+  clipboardState: ClipboardState;
+
+  /**
+   * Delegated clipboard data getter for backward compatibility.


What backward compatibility?

dklOrdix and others added 30 commits May 4, 2026 09:18

refactor(frontend): clean up and restructure app

161ce0e

fix: add lint:fix script to package.json and clean up trailing comma …

d2de208

…in +layout.svelte

docs: update AGENTS.md to include Material UI icon library usage

c81b4d0

refactor(header): move components for header into own directory

9add069

chore: dedupe lockfile

8343022

fix(sidebar): add hover cursor style to toggle button

4bc8e93

chore(types): centralise navigation and auth types; update header/sid…

952dc80

…ebar imports Co-authored-by: Copilot <copilot@github.com>

feat(storage): initial Filebrowser UI implementation

cba7866

Co-authored-by: Copilot <copilot@github.com>

refactor(storage): extract service layer and consolidate storage type…

5de8d9f

…s/utils Co-authored-by: Copilot <copilot@github.com>

Merge branch 'feat/s3-file-browser' into feat/s3-file-browser-ui

20909ae

chore(storage): remove outdated STORAGE_REORG_PLAN.md

2b33533

fix(lint): resolve errors

aeac85d

refactor(tests): update ThemeToggle tests to use checkbox role

f2ba284

Co-authored-by: Copilot <copilot@github.com>

style: format code

af9b651

fix(tests): update theme toggle role from button to checkbox

8373974

Co-authored-by: Copilot <copilot@github.com>

fix: revert changes to Theme Toggle

7aff59b

fix: revert changes to Theme Toggle

266a045

Merge pull request #90 from stackabletech/feat/s3-file-browser-ui

805f693

feat(storage): initial Filebrowser UI implementation

#80 use pagination controls to navigate the file browser

3386182

#80 show page counter for pagination control

b77bcfa

Co-authored-by: Copilot <copilot@github.com>

#80 fix lint and svelte check

48b552e

Co-authored-by: Copilot <copilot@github.com>

fix lint error

7c9733f

Merge pull request #100 from stackabletech/feat/s3-file-browser-pagin…

9d85f57

…ation S3 File Browser Page Navigation (Pagination)

File Browser: Feature Flag (#105)

b4e25ec

* #96 disable the storage browser if feature flag is set * #96 fix svelte-check * #96 fix lint * #96 disable storage browser by default

Rename option STACKABLE_UI_COMPLETION_DISABLED (#106)

c36f1c8

S3 File Browser: Shared Pagination Component (#103)

ff43b22

* #102 refactor pagination controls into a separate component * #102 simplified pagination props

Merge branch 'main' into feat/s3-file-browser

ed9be69

Feat/s3 file browser refactor object table (#120)

184fa84

* refactor(ObjectTable): extract File and Folde rows into seperate components * style: format code * style: use camelCase * style: use camelCase

Merge remote-tracking branch 'origin/main' into feat/s3-file-browser-v1

18190cd

# Conflicts: # TECH_DEBT.md # src/lib/server/trino/queries.ts # vite.config.ts

lfrancke requested changes Sep 10, 2026

View reviewed changes

Comment thread src/lib/server/storage/archive.ts Outdated

Comment thread src/architecture/i18n-compliance.spec.ts Outdated

Comment thread src/architecture/code-size-limits.spec.ts Outdated

F3l1x1vo added 3 commits September 14, 2026 08:08

#208 fix bucket shared data, solve i18n violations, remove comment

bcad89b

Merge branch 'main' into feat/s3-file-browser-v1

0ea15e0

fix e2e test

6380967

F3l1x1vo mentioned this pull request Sep 14, 2026

Add/Edit Bookmark Dialog #278

Open

Bohreromir requested changes Sep 15, 2026

View reviewed changes

Bohreromir reviewed Sep 15, 2026

View reviewed changes

Comment thread src/lib/server/storage/archive.ts Outdated

F3l1x1vo added 4 commits September 16, 2026 09:28

#278 implement changes suggested in code review

c1ccfb0

update comment

5a2a7e7

fix unit test

ccf16d6

fix e2e test

8271e55

Bohreromir requested changes Sep 16, 2026

View reviewed changes

Bohreromir reviewed Sep 16, 2026

View reviewed changes

#278 implement changes suggested in code review

cc43ef3

Bohreromir requested changes Sep 17, 2026

View reviewed changes

#278 strengthen e2e tests to be less flaky

b94619d

Bohreromir requested changes Sep 17, 2026

View reviewed changes

F3l1x1vo added 8 commits September 18, 2026 08:42

#208 remove TECH_DEBT mentions

5121fcb

#208 implement changes suggested in code review

5db9aaf

#208 use eslint size limits

71d8828

#208 use eslint check-file

acda359

#208 merge db migrations

ec02386

#208 implement changes suggested in code review

17bdaa9

Merge main into feat/s3-file-browser-v1

063f694

#208 fix broken tests and lint from merge

98b4f20

Bohreromir requested changes Sep 18, 2026

View reviewed changes


		// ── ActionError (backward-compatible alias) ──────────────────────────────────

		export class ActionError extends StorageError {

Uh oh!

Conversation

dklOrdix commented Jun 18, 2026 • edited by F3l1x1vo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stackable-cla Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Bohreromir left a comment

Choose a reason for hiding this comment

Uh oh!

Bohreromir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

F3l1x1vo commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dklOrdix commented Jun 18, 2026 •

edited by F3l1x1vo

Loading

stackable-cla Bot commented Sep 10, 2026 •

edited

Loading

F3l1x1vo commented Sep 17, 2026 •

edited

Loading