Skip to content

Implement shared failure-code catalog for order lifecycle errors#50

Open
K1NGD4VID wants to merge 9 commits into
karagozemin:masterfrom
K1NGD4VID:feature/failure-code-catalog
Open

Implement shared failure-code catalog for order lifecycle errors#50
K1NGD4VID wants to merge 9 commits into
karagozemin:masterfrom
K1NGD4VID:feature/failure-code-catalog

Conversation

@K1NGD4VID

Copy link
Copy Markdown

closes #48

@vercel

vercel Bot commented Jun 28, 2026

Copy link
Copy Markdown

@K1NGD4VID is attempting to deploy a commit to the karagoz's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jun 28, 2026

Copy link
Copy Markdown

@K1NGD4VID Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the failure-code catalog work. I checked this against #48 and cannot merge it yet because multiple required checks fail.

Commands run:

  • pnpm install --frozen-lockfile — passed
  • pnpm --filter @oversync/coordinator test — failed
  • pnpm --filter @oversync/frontend exec vitest run — passed, 23/23
  • pnpm --filter @oversync/frontend build — failed
  • git diff --check master...HEAD — failed

Blocking failures:

  1. Coordinator tests fail because the SQLite schema used by tests does not have the new failure_code column:

Error: no such column: failure_code

This occurs when constructing OrdersRepository for api-errors.test.ts and order-service.test.ts. Please update the schema/migration/test setup so fresh test DBs include failure_code.

  1. Frontend build fails because the frontend imports new SDK exports that are not actually available from @oversync/sdk/types:

Module @oversync/sdk/types has no exported member FAILURE_CODE_CATALOG.
Module @oversync/sdk/types has no exported member FailureCode.

Please fix the package exports/build path so consumers can import those symbols.

  1. git diff --check master...HEAD reports trailing whitespace in:
  • coordinator/test/api-errors.test.ts
  • packages/sdk/src/types/index.ts

After fixes, please rerun:

  • pnpm --filter @oversync/coordinator test
  • pnpm --filter @oversync/frontend exec vitest run
  • pnpm --filter @oversync/frontend build
  • git diff --check master...HEAD

Then I can re-review the actual #48 acceptance criteria.

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the follow-up. I re-ran the checks on the latest commit. The whitespace issue is fixed and the frontend vitest suite passes, but this still cannot be merged yet.

Commands run:

pnpm install --frozen-lockfile                         # passed
pnpm --filter @oversync/coordinator test              # failed
pnpm --filter @oversync/frontend exec vitest run      # passed, 23/23
pnpm --filter @oversync/frontend build                # failed
git diff --check origin/master...HEAD                 # passed

Remaining blockers:

  1. Coordinator API error tests still fail because FAILURE_CODE_CATALOG is undefined at runtime in coordinator/src/server/routes/orders.ts:
TypeError: Cannot read properties of undefined (reading 'VALIDATION_FAILED')\n  at sendError src/server/routes/orders.ts:48:18\n\ntest/api-errors.test.ts: 5 failed / 5 passed\n```\n\nThe missing-order case also returns 500 instead of the expected 404 because of the same error path.\n\n2. Frontend build still fails because the new SDK symbols are not exported to consumers from `@oversync/sdk/types`:\n\n```\nsrc/components/TransactionHistory.tsx(6,10): error TS2305: Module '@oversync/sdk/types' has no exported member 'FAILURE_CODE_CATALOG'.\nsrc/components/TransactionHistory.tsx(6,37): error TS2305: Module '@oversync/sdk/types' has no exported member 'FailureCode'.\n```\n\nPlease fix the runtime/package exports path for `FAILURE_CODE_CATALOG` and `FailureCode`; then I can re-check and merge.

…lves without a build step

The SDK exports map (and main/types fields) pointed every entry at
./dist/<name>/index.{js,d.ts}. When dist/ was missing, stale, or empty,
consumers silently got undefined for FAILURE_CODE_CATALOG at runtime
and TS2305 'no exported member' from tsc -- a single shared root cause
behind the coordinator API error mapper TypeError and the frontend
TransactionHistory TS2305 errors.

Switch every exports entry plus main/types to ./src/<name>.ts. With
moduleResolution: bundler in coordinator and frontend tsconfigs,
tsc + vite + vitest + tsx consume the TS source directly, so no
precompiled dist artifact is required for dev or test.

Also:
- Add 'sideEffects': false on the SDK package for tree-shaking.
- Add a 'default' condition on every exports entry so future
  require() callers don't hit ERR_PACKAGE_PATH_NOT_EXPORTED.
- Pre-bundle @oversync/sdk and @oversync/sdk/types in frontend
  vite.optimizeDeps.include to absorb the new on-the-fly esbuild scan.
- Add packages/sdk/test/exports.test.ts which imports the catalog via
  the package subpath, the package main entry, and the source shortcut,
  with toBeDefined() guards and an exhaustive switch terminating in
  const _: never = code so any future FailureCode drift fails tsc at
  compile time and fails vitest loudly if exports silently resolve to
  undefined again.

Verified: SDK 31/31 vitest passes, all three (SDK, coordinator,
frontend) tsc --noEmit pass, frontend full 'tsc && vite build' passes.
@karagozemin

Copy link
Copy Markdown
Owner

I checked the current PR state before reviewing code, and GitHub still reports this branch as CONFLICTING / DIRTY against current master. I cannot merge or give a final code approval while the branch cannot be merged.

Please rebase or merge latest master, resolve the conflicts, and push the updated branch. Once GitHub reports it as mergeable, I can run the relevant package tests and re-review.

* upstream/master: ~40 commits including:
  - feat(frontend): add HTLC lifecycle timeline, stale order banner
  - feat(coordinator): add aggregate order metrics endpoint
  - feat(resolver): add dry-run readiness command
  - feat(sdk): add capability matrix, explorer URL helpers
  - Various docs, CI, and relayer improvements

Conflicts resolved:
  - coordinator/src/services/order-service.ts: kept both getOrderMetrics()
    and failureCode parameter in markStatus()
  - frontend/src/components/TransactionHistory.tsx: kept both HtlcTimeline
    and FAILURE_CODE_CATALOG imports
@K1NGD4VID

Copy link
Copy Markdown
Author

@karagozemin Please review it now

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the update. I rechecked this from the merge ref under the karagozemin account.

Passing checks:

  • git diff --check origin/master...HEAD
  • pnpm --filter @oversync/sdk build

Still blocked:

  • pnpm --filter @oversync/coordinator test fails before the affected route suites can run. Both coordinator/test/api-errors.test.ts and coordinator/test/http-routes.test.ts fail to load coordinator/src/server/routes/orders.ts because Vite/Vitest cannot resolve @oversync/sdk/types:
Failed to load url @oversync/sdk/types (resolved id: @oversync/sdk/types) in /private/tmp/oversync-review/coordinator/src/server/routes/orders.ts. Does the file exist?

Please update the SDK subpath export / workspace resolution so coordinator tests can import @oversync/sdk/types, or switch the coordinator import to an already-resolvable SDK entrypoint. After that, I can re-run and merge if the coordinator suite is green.

- Add @oversync/sdk to vitest deps.inline so Vite resolves the
  workspace package's TS source files for subpath exports
- Export sendError from orders.ts and use it in secrets.ts for
  consistent VALIDATION_FAILED responses
- Update http-routes.test.ts expectations from validation_error to
  VALIDATION_FAILED
@K1NGD4VID

K1NGD4VID commented Jun 30, 2026

Copy link
Copy Markdown
Author

Please reveiw and merge @karagozemin

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the update. I rechecked the merge ref from the maintainer account. The SDK checks pass, but coordinator tests fail and the PR still contains a generated DB file.

Commands run:

git diff --check master...pr-50-merge
pnpm --filter @oversync/sdk test
pnpm --filter @oversync/sdk build
pnpm --filter @oversync/coordinator test

Passing:

@oversync/sdk test: 53 tests passed
@oversync/sdk build: passed

Failing:

FAIL test/api-errors.test.ts
FAIL test/http-routes.test.ts
Error: Failed to load url @oversync/sdk/types in coordinator/src/server/routes/orders.ts. Does the file exist?

Please fix the coordinator import/build path for the shared failure-code types, or export/import the catalog from a package path that Vitest can resolve in the workspace. Also please remove coordinator/oversync.db from the PR; generated local SQLite databases should not be committed. After that I can rerun coordinator/frontend checks.

K1NGD4VID added 3 commits July 1, 2026 15:15
…ed DB

- Switch all @oversync/sdk/types imports to @oversync/sdk (main entry
  already re-exports types/index.ts). Avoids Vitest subpath resolution
  issues with workspace-linked packages.
- Remove coordinator/oversync.db from git tracking and add *.db to
  .gitignore so generated SQLite files aren't committed.
- Remove @oversync/sdk/types from frontend optimizeDeps.include since
  that subpath is no longer imported directly.
* upstream/master: ~15 commits including:
  - feat(frontend): add deployment self-check, backend status banner,
    HTLC receipt card, investor demo mode
  - feat(resolver): add read-only preflight command
  - feat(coordinator): add CORS allowlist validation, order transitions
  - Various docs, CI, and validation scripts

Conflicts resolved:
  - coordinator/src/server/routes/orders.ts: kept upstream's GET /orders/:id
    and GET /orders/:id/transitions with sendError-style error handling;
    removed duplicate route
  - coordinator/src/persistence/orders-repo.ts: kept both getTransitions()
    and failureCode parameter on setStatus
…ode-catalog

# Conflicts:
#	coordinator/src/server/routes/orders.ts
#	packages/sdk/src/types/index.ts
@K1NGD4VID

Copy link
Copy Markdown
Author

Kindly review and merge @karagozemin

@karagozemin

karagozemin commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Rechecked: thanks for the updates. However this PR now has merge conflicts with main, so I cannot merge it as-is. Could you rebase/merge the latest main, resolve the conflicts and push? I will take another look and merge afterwards.

@K1NGD4VID

Copy link
Copy Markdown
Author

fixed please review now @karagozemin

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.

[Wave 6 15/15] Structured failure catalog for cross-chain order lifecycle

2 participants