Skip to content

feat(generate): add workspace asset library#188

Open
DrHepa wants to merge 2 commits into
lightningpixel:devfrom
DrHepa:feat/workspace-asset-library
Open

feat(generate): add workspace asset library#188
DrHepa wants to merge 2 commits into
lightningpixel:devfrom
DrHepa:feat/workspace-asset-library

Conversation

@DrHepa

@DrHepa DrHepa commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a Workspace Asset Library to the Generate area so users can browse indexed workspace outputs and safely reopen supported assets without touching the viewer implementation.

What changed

Electron registry / IPC / preload

  • Adds an Electron-side artifact registry service that indexes workspace assets under supported workspace roots.
  • Adds structured list/read/open results for renderer-safe consumption.
  • Wires workspace library IPC handlers.
  • Adds a scoped preload API for workspace library list/read/open calls.

Shared contracts

  • Adds shared Workspace Asset Library types for entries, capabilities, source scopes, openability, read/open requests, structured errors, warnings, manifests, source links, and provenance-style metadata.
  • Adds lightweight artifact/source metadata types used by the library boundary.

Renderer projection / service

  • Adds renderer projection that normalizes registry entries into UI-safe entries.
  • Supports open targets: self, linked-source, and unavailable.
  • Adds fail-closed handling for unsupported or unsafe assets.
  • Adds a renderer service wrapper that validates read/open requests before crossing IPC.

GeneratePage UI

  • Adds a Library button and popover in Generate.
  • Supports refresh, search, sorting, grouped/collapsible display by source scope and capability, selection, open status messaging, and safe open behavior.
  • Preserves the existing Generate open/job flow instead of introducing a new viewer-state model.

Safety / path validation

  • Restricts registry access to allowed workspace-relative paths.
  • Rejects unsafe workspace paths, self-links, mismatched source links, Windows absolute paths, and UNC paths.
  • Keeps unsupported entries visible but unavailable rather than throwing or opening unsafe paths.

Tests

  • Adds focused tests for shared types, Electron registry behavior, IPC/preload wiring, renderer projection/service behavior, UI helper behavior, linked-source opens, and Windows/UNC path rejection.

Explicit non-goals

  • No direct Viewer3D changes.
  • No private viewer-state stack or viewer-side artifact tooling is introduced.
  • Advanced .ply/.splat viewer opening remains out of scope unless routed through safe supported sources.

Changed areas

Area Files
Electron registry/IPC/preload electron/main/artifact-registry-service.ts, electron/main/ipc-handlers.ts, electron/preload/electron-api.ts, electron/preload/index.ts
Shared contracts src/shared/types/assetLibrary.ts, src/shared/types/artifacts.ts, src/shared/types/electron.d.ts
Renderer services/projection src/areas/generate/assetLibraryProjection.ts, src/areas/generate/assetLibraryService.ts, src/areas/generate/assetLibraryUi.ts
Generate UI src/areas/generate/GeneratePage.tsx
Tests *.test.ts files added for all new boundaries

Test plan

  • node --test --experimental-strip-types src/shared/types/assetLibrary.test.ts src/areas/generate/assetLibraryProjection.test.ts src/areas/generate/assetLibraryService.test.ts src/areas/generate/assetLibraryUi.test.ts electron/main/artifact-registry-service.test.ts electron/preload/artifact-registry-preload.test.ts
  • npm test
  • Confirmed PR diff has no src/areas/generate/components/Viewer3D.tsx changes
  • Manually validated the Workspace Library flow on Windows in a clean upstream/dev-based worktree

Review notes

This PR is intentionally scoped to Workspace Library only. It does not include the separate ViewerAuthoringHost/Phase 1B work so the diff remains focused and reviewable.

@Lorchie

Lorchie commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Review — Workspace Asset Library

Reviewed the full diff (17 files), ran the tests, and built the branch locally. Overall this is a well-architected, honestly-scoped PR (description matches the diff, no Viewer3D changes, clean layering: shared types → Electron registry → renderer projection → service → UI, with solid path-validation defense-in-depth).

Verified locally

  • ✅ npx electron-vite build → success (main + preload + renderer, 1002 modules). This is the real CI gate and it's green.
  • ✅ The 6 new test files pass: 28/28 (node --test --experimental-strip-types).
  • ✅ Path validation rejects .., Windows-absolute (C:), UNC (\), encoded escapes (%2e), self-links and non-GLB sources. Fail-closed.

None of the items below block the build, but they're worth cleaning up before merge.

Should fix

  1. Dead / duplicated preload module
    electron/preload/electron-api.ts (createElectronApi) is only imported by its own test (artifact-registry-preload.test.ts). The real preload (index.ts:153-159) redefines the library API inline and never uses createElectronApi. So the test validates a module that doesn't run in production, and the two definitions can drift. Worse, the live inline version types
    read/open as { workspacePath: string } — it omits sourceWorkspacePath (linked-source opens still work only because the whole object is forwarded to ipcRenderer.invoke). The correctly-typed version is the dead one.
    → Either consume createElectronApi from index.ts, or delete electron-api.ts and fix the inline request types.

Minor

  1. Dead export — ArtifactRef in src/shared/types/artifacts.ts is never used anywhere (ArtifactProvenance is). Remove it.

  2. Phantom field — createAssetLibraryOpenJob (assetLibraryUi.ts:164) sets prompt: … on the job, but GenerationJob has no prompt field. It's stripped at build time and never read. Remove it (or add it to the type if intended).

  3. Import convention — the new source files use explicit .ts extensions in imports (./assetLibraryService.ts, etc.), which no other file in the repo does. It builds fine and is needed for node --test, but it diverges from the codebase convention.

  4. Tests not wired into npm test — the suite globs electron/main/*.test.mjs, so the new *.test.ts files only run via the manual command in the test plan. Consider integrating them so they don't silently rot.

Nit

Path-validation logic is duplicated across the registry service, projection and renderer service. It's intentional defense-in-depth across the IPC boundary, so fine — just flagging for future maintenance.

@DrHepa

DrHepa commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — agreed on the cleanup points. I pushed a follow-up commit: 183c566 fix(generate): address workspace library review cleanup.

Addressed:

  1. Preload drift / dead module

    • electron/preload/index.ts now exposes createElectronApi(ipcRenderer, webFrame) directly.
    • The factory is now the production preload surface, not a test-only duplicate.
    • read/open are typed with AssetLibraryReadRequest / AssetLibraryOpenRequest, including sourceWorkspacePath.
  2. Dead export

    • Removed unused ArtifactRef from src/shared/types/artifacts.ts.
  3. Phantom field

    • Removed the non-existent prompt field from createAssetLibraryOpenJob.
  4. Import convention

    • Runtime/source imports now use extensionless repo convention.
    • Test imports keep explicit .ts where needed for Node's TypeScript test execution.
  5. Tests wired into npm test

    • Added the new TS asset-library/preload tests to npm test.
    • Added a small test-only loader so Node can execute the TS tests while production source keeps extensionless imports.

Validation run locally:

  • npm test → PASS
    • Python unittest: 4/4
    • TS asset-library/preload tests: 34/34
    • existing Electron .mjs tests: 3/3
  • Confirmed no src/areas/generate/components/Viewer3D.tsx diff.

No build was run on my side for this cleanup pass.

@DrHepa DrHepa force-pushed the feat/workspace-asset-library branch from 183c566 to f8e965f Compare June 19, 2026 15:20
@DrHepa

DrHepa commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the GeneratePage conflict by rebasing this branch onto latest upstream/dev (3b1b137).

Conflict resolution notes:

  • Preserved the upstream light-settings refactor in GeneratePage.tsx.
  • Kept the Workspace Library UI/service integration on top of the new GeneratePage shape.
  • Confirmed no src/areas/generate/components/Viewer3D.tsx diff.

Validation after rebase:

  • npm test → PASS
    • Python unittest: 4/4
    • TS asset-library/preload tests: 34/34
    • Electron .mjs tests: 3/3

Branch was updated with git push --force-with-lease after the rebase.

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.

2 participants