Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ cypress/fixtures/
.idea/*
.vscode/*

# Claude Code
.claude/

# Configuration
/**/app-config.json

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-03-04
67 changes: 67 additions & 0 deletions openspec/changes/fix-empty-module-package-contents/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
## Context

`CreatePackageUseCase.execute()` builds a metadata package from a module by calling `metadataPayloadBuilder.build(syncBuilder)` — an async method that returns `Promise<MetadataPackage>`. A refactor (commit fb983808) replaced the previous `await`-ed call with the builder pattern but dropped the `await` keyword. The result is that a raw Promise object is passed through `mapPackageTo()` and stored as the package `contents`, which serializes to `{}` in the dataStore.

Every downstream operation (import, download JSON, diff, mapping) reads `contents` and finds it empty, producing the four symptoms reported in the ticket.

The "create package from file" path is unaffected because its `contents` parameter is already resolved — the ternary short-circuits before hitting the builder call.

## Goals / Non-Goals

**Goals:**
- Restore correct metadata inclusion in packages generated from modules
- Prevent this class of bug from reoccurring via improved type constraints
- Add a unit test for `CreatePackageUseCase` covering the payload resolution path

**Non-Goals:**
- Fixing or recovering previously-generated broken packages (users can regenerate)
- Changing the package format or storage schema
- Refactoring the broader transformation pipeline

## Decisions

### 1. Add missing `await` in CreatePackageUseCase (line 41)

**Change:** `this.metadataPayloadBuilder.build(syncBuilder)` → `await this.metadataPayloadBuilder.build(syncBuilder)`

**Rationale:** This is the minimal, correct fix. All other call sites (`DiffPackageUseCase`, `DownloadModuleSnapshotUseCase`, `CreatePullRequestUseCase`) already `await` the same method. The function is already `async`, so no signature change is needed.

**Alternatives considered:**
- Making `build()` synchronous — not feasible, it performs API calls to fetch metadata.

### 2. Constrain `mapPackageTo` generic to exclude Promise types

**Change:** Add a type constraint to the `Input` parameter of `TransformationRepository.mapPackageTo` so that `Promise<T>` is rejected at compile time.

**Current signature:**
```typescript
mapPackageTo<Input = MetadataPackage, Output = MetadataPackage>(
destination: number,
payload: Input, // accepts anything, including Promise
...
): Output;
```

**Proposed signature:**
```typescript
mapPackageTo(
destination: number,
payload: MetadataPackage,
...
): MetadataPackage;
```

**Rationale:** The generic type parameters `Input` and `Output` are only defaulted to `MetadataPackage` — they don't constrain it. Every call site in the codebase passes `MetadataPackage` in and expects `MetadataPackage` out, so the generics add no value and actively hide bugs. Removing them makes the interface honest about what it accepts.

**Alternatives considered:**
- Adding a `NoPromise<T>` conditional type constraint — overly clever, harder to understand.
- Keeping generics but adding `extends MetadataPackage` — same effect as removing them, but more verbose.

### 3. Add unit test for CreatePackageUseCase

**Rationale:** The use case currently has no unit tests, which allowed this regression to go undetected. A test that verifies the stored package `contents` is a resolved `MetadataPackage` (not a Promise) will catch this class of bug.

## Risks / Trade-offs

- **[Low] Removing generics from `mapPackageTo`/`mapPackageFrom`** → Any call site that relied on the generic flexibility will fail to compile. A codebase search confirms no call site uses non-default type arguments, so this risk is effectively zero.
- **[Low] Existing broken packages in dataStore** → Packages generated while the bug was active contain empty `contents`. Users need to regenerate these packages. No migration is needed since the storage format itself is correct; only the data was wrong.
24 changes: 24 additions & 0 deletions openspec/changes/fix-empty-module-package-contents/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## Why

When generating a package from a module, the package is saved with empty contents — the actual metadata objects (datasets, data elements, etc.) are missing. This causes four user-visible failures: imports report 0 elements, downloaded JSONs lack metadata, comparisons always show "ok", and the mapping tab is empty. The root cause is a missing `await` on an async call introduced during a refactor (commit fb983808), meaning a Promise object is stored instead of the resolved metadata payload.

## What Changes

- **Fix async/await bug in `CreatePackageUseCase`**: Add the missing `await` keyword when calling `metadataPayloadBuilder.build()` so the resolved `MetadataPackage` is stored instead of a Promise object.
- **Add type safety to `TransformationRepository.mapPackageTo`**: Tighten the generic type parameter to prevent `Promise` objects from being silently accepted as payload input, making this class of bug a compile-time error.

## Capabilities

### New Capabilities

_(none — this is a bug fix, no new capabilities are introduced)_

### Modified Capabilities

_(no spec-level requirement changes — the existing behavior was correct by design, only the implementation was broken)_

## Impact

- **Code**: `src/domain/packages/usecases/CreatePackageUseCase.ts` (primary fix), `src/domain/transformations/repositories/TransformationRepository.ts` (type hardening)
- **User impact**: All module-generated packages will now contain their full metadata payload, restoring import, download, comparison, and mapping functionality
- **Risk**: Minimal — the fix aligns this call site with every other `metadataPayloadBuilder.build()` call in the codebase, all of which already use `await`
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## ADDED Requirements

### Requirement: Package generation SHALL resolve metadata payload before storage

When `CreatePackageUseCase` generates a package from a module, it MUST await the `MetadataPayloadBuilder.build()` call so that the resolved `MetadataPackage` — not a Promise — is stored as the package contents.

#### Scenario: Module package contains metadata objects after generation
- **WHEN** a user generates a package from a module that references a dataset
- **THEN** the stored package `contents` SHALL include the dataset and its dependent metadata objects

#### Scenario: Package contents are serializable as valid metadata JSON
- **WHEN** a module package is generated and saved to the dataStore
- **THEN** the `contents` field SHALL be a plain object with metadata type keys (e.g., `dataSets`, `dataElements`), not a Promise or empty object

### Requirement: TransformationRepository SHALL reject non-MetadataPackage payloads at compile time

The `mapPackageTo` and `mapPackageFrom` methods MUST accept only `MetadataPackage` as the payload type, preventing Promise objects or other incorrect types from being passed silently.

#### Scenario: Passing a Promise to mapPackageTo causes a compile error
- **WHEN** a developer passes `Promise<MetadataPackage>` as the payload argument to `mapPackageTo`
- **THEN** the TypeScript compiler SHALL report a type error

#### Scenario: Passing MetadataPackage to mapPackageTo compiles successfully
- **WHEN** a developer passes a resolved `MetadataPackage` as the payload argument to `mapPackageTo`
- **THEN** the TypeScript compiler SHALL accept the call without error
18 changes: 18 additions & 0 deletions openspec/changes/fix-empty-module-package-contents/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## 1. Fix Missing Await in CreatePackageUseCase

- [ ] 1.1 [BE] Add `await` before `this.metadataPayloadBuilder.build(syncBuilder)` in `src/domain/packages/usecases/CreatePackageUseCase.ts` line 41

## 2. Type Safety Improvement

- [ ] 2.1 [BE] Remove generic type parameters from `mapPackageTo` and `mapPackageFrom` in `src/domain/transformations/repositories/TransformationRepository.ts`, replacing `Input`/`Output` with concrete `MetadataPackage` type
- [ ] 2.2 [BE] Update `TransformationRepositoryImpl` (data layer) to match the new non-generic signature
- [ ] 2.3 [BE] Verify all call sites compile cleanly with the tightened types (`yarn test` type-check)

## 3. Unit Tests

- [ ] 3.1 [BE] Create `CreatePackageUseCase.spec.ts` with a test that verifies the stored package `contents` is a resolved `MetadataPackage` (not a Promise or empty object)
- [ ] 3.2 [BE] Run `yarn test` to confirm all existing tests pass alongside the new test

## 4. Validation

- [ ] 4.1 [BE] Run full type-check and build: `yarn test` and `yarn build core-app`
Loading
Loading