Open
Conversation
tonyxiao
force-pushed
the
feat/ergonomic-openapi-cli
branch
from
ce19e05 to
8b01bc2
Compare
tonyxiao
force-pushed
the
chore/repo-reorg-harness-engineering
branch
2 times, most recently
from
826e4b7 to
8cda3d4
Compare
tonyxiao
force-pushed
the
feat/ergonomic-openapi-cli
branch
from
bd19fe6 to
c299605
Compare
tonyxiao
changed the base branch from
chore/repo-reorg-harness-engineering
to
main
There was a problem hiding this comment.
Pull request overview
This PR significantly expands the repo by adding a new docs build system (Markdoc-based static site), new runtime applications (apps/engine, apps/service, apps/supabase, apps/visualizer), and updates to Docker/Compose + CI workflows—despite the PR title/description claiming a small, additive change limited to packages/ts-cli/src/openapi/ergonomic/.
Changes:
- Introduces a new docs site generator (
docs/build.mjs) + many new architecture/engine/service docs and diagrams. - Adds new deployable apps (
@stripe/sync-engine,@stripe/sync-service, Supabase integration, schema visualizer) with tests and build configs. - Updates operational tooling: Dockerfile, root compose, Verdaccio config, CI workflows, editor settings.
Reviewed changes
Copilot reviewed 157 out of 545 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/service/scenarios.md | Adds service-layer scenario coverage documentation |
| docs/service/entities.puml | Adds service entities diagram |
| docs/service/cli.md | Documents service/engine CLI spec and flag mappings |
| docs/service/ARCHITECTURE.md | Adds service architecture overview |
| docs/requirements_docs.txt | Removes MkDocs requirements list |
| docs/public/style.css | Adds CSS for new static docs site |
| docs/public/engine.html | Adds Scalar API reference wrapper page |
| docs/plans/plan-008-scope-rename-stripe-sync.md | Adds RFC plan for npm scope rename |
| docs/plans/plan-007-supabase-fan-out-implementation.md | Adds Supabase fan-out implementation plan |
| docs/plans/plan-005-cli-progress-display.md | Adds plan for CLI progress rendering |
| docs/plans/plan-002-move-openapi-to-source-stripe.md | Adds plan for OpenAPI code relocation |
| docs/plans/backlog.md | Adds backlog list |
| docs/package.json | Adds docs build package manifest |
| docs/openapi/webhook.json | Adds standalone webhook OpenAPI doc |
| docs/layout.html | Adds docs HTML layout template |
| docs/index.md | Rewrites docs homepage content |
| docs/guides/tsconfig.md | Adds TS config rationale guide |
| docs/guides/publishing.md | Adds publishing/Docker packaging guide |
| docs/guides/local-registries.md | Adds local registry (Verdaccio) guide |
| docs/guides/cli-design-lessons.md | Adds CLI design notes |
| docs/guides/cli-config-resolution.md | Adds CLI config resolution spec |
| docs/engine/sync-engine-types.ts | Adds protocol type docs |
| docs/engine/sync-engine-api.ts | Adds engine interface docs |
| docs/engine/state-flow.puml | Adds state flow sequence diagram |
| docs/engine/scenarios.md | Adds engine scenarios doc |
| docs/engine/protocol.md | Adds protocol overview doc |
| docs/edge-function.md | Removes old Supabase edge function doc |
| docs/docker.md | Removes old Docker doc |
| docs/d2.mjs | Adds D2 → SVG renderer helper |
| docs/contributing.md | Removes old contributing guide |
| docs/build.mjs | Adds Markdoc-based docs build pipeline |
| docs/architecture/quality.md | Adds quality scorecard |
| docs/architecture/principles.md | Adds architectural “golden principles” |
| docs/architecture/decisions.md | Adds design decision records |
| docs/.gitignore | Ignores Vercel metadata in docs |
| docker/compose.yml | Removes legacy docker compose under docker/ |
| demo/write-to-sheets.sh | Adds demo helper script for Sheets destination |
| demo/write-to-postgres.sh | Adds demo helper script for Postgres destination |
| demo/stripe-to-postgres.sh | Adds demo for full Stripe → Postgres run |
| demo/stripe-to-google-sheets.sh | Adds demo for Stripe → Sheets run |
| demo/reset-postgres.sh | Adds helper for wiping Postgres schemas/tables |
| demo/read-from-stripe.sh | Adds demo helper for Stripe source read |
| compose.yml | Adds root compose with Postgres/stripe-mock/Temporal/Verdaccio |
| apps/visualizer/src/app/page.tsx | Adds visualizer root page + loading skeleton |
| apps/visualizer/src/app/layout.tsx | Adds visualizer app layout + metadata |
| apps/visualizer/src/app/globals.css | Adds Tailwind import for visualizer |
| apps/visualizer/postcss.config.mjs | Adds PostCSS config for Tailwind |
| apps/visualizer/package.json | Adds visualizer package manifest |
| apps/visualizer/next.config.ts | Enables WASM + COOP/COEP headers for PGlite |
| apps/visualizer/README.md | Documents visualizer and data artifacts |
| apps/visualizer/.gitignore | Ignores build and tsbuildinfo outputs |
| apps/supabase/vitest.integration.config.ts | Adds Vitest integration test config |
| apps/supabase/vitest.e2e.config.ts | Adds Vitest e2e test config |
| apps/supabase/vitest.config.ts | Adds default Vitest config exclusions |
| apps/supabase/tsconfig.json | Adds TS config for Supabase app |
| apps/supabase/src/lib.ts | Fixes ESM import extension for supabase exports |
| apps/supabase/src/index.ts | Adds Supabase app barrel exports |
| apps/supabase/src/edge-functions/deno.json | Changes Deno import mapping for DB client |
| apps/supabase/src/edge-function-code.ts | Adds ?raw import of bundled edge function code |
| apps/supabase/src/tests/supabase.e2e.test.ts | Adds Supabase E2E coverage (real project + Stripe) |
| apps/supabase/src/tests/install.e2e.test.ts | Adds Supabase install/uninstall e2e test |
| apps/supabase/src/tests/edge-runtime.smoke.test.ts | Adds local edge-runtime container smoke test |
| apps/supabase/src/tests/deploy.e2e.test.ts | Adds edge function deploy/invoke e2e |
| apps/supabase/src/tests/bundle.test.ts | Adds bundle quality tests for ?raw output |
| apps/supabase/package.json | Adds publishable Supabase integration package |
| apps/supabase/build.mjs | Reworks build pipeline to esbuild + d.ts emit |
| apps/service/tsconfig.json | Adds TS config for service app |
| apps/service/src/temporal/workflows.ts | Adds Temporal workflow implementation |
| apps/service/src/temporal/worker.ts | Adds Temporal worker factory |
| apps/service/src/temporal/types.ts | Adds Temporal activity/workflow types |
| apps/service/src/temporal/bridge.ts | Adds WorkflowClient bridge for service orchestration |
| apps/service/src/temporal/activities.ts | Adds activities calling engine/service HTTP endpoints |
| apps/service/src/logger.ts | Adds service logger with redaction |
| apps/service/src/lib/stores.ts | Adds store interfaces for pipelines/state/logs |
| apps/service/src/lib/stores-fs.ts | Adds filesystem-backed store implementations |
| apps/service/src/lib/stores-fs.test.ts | Adds tests for FS stores |
| apps/service/src/lib/service.ts | Adds core SyncService orchestration (in-process + Temporal) |
| apps/service/src/lib/schemas.ts | Adds Pipeline + LogEntry Zod schemas |
| apps/service/src/lib/resolve.ts | Adds Pipeline → engine config resolution helper |
| apps/service/src/index.ts | Adds service public barrel exports |
| apps/service/src/cli/index.ts | Adds CLI entry re-export |
| apps/service/src/api/webhook-app.ts | Adds standalone webhook ingress app + OpenAPI |
| apps/service/src/api/index.ts | Adds API barrel export |
| apps/service/src/api/app.test.ts | Adds API tests including OpenAPI + pipelines + webhook |
| apps/service/package.json | Adds publishable service package manifest |
| apps/engine/vitest.config.ts | Adds engine vitest config + timeouts/excludes |
| apps/engine/tsconfig.json | Adds engine TS config |
| apps/engine/src/serve-command.ts | Adds serve command wiring + connector resolver |
| apps/engine/src/logger.ts | Adds engine logger with redaction |
| apps/engine/src/lib/state-store.ts | Adds StateStore interface + noop implementation |
| apps/engine/src/lib/source-test.ts | Adds test source connector |
| apps/engine/src/lib/source-exec.ts | Adds subprocess source adapter |
| apps/engine/src/lib/select-state-store.ts | Adds convention-based state store loader |
| apps/engine/src/lib/resolver.test.ts | Adds connector resolver tests |
| apps/engine/src/lib/pipeline.ts | Adds pipeline utility stages (filter, persist, pipe) |
| apps/engine/src/lib/ndjson.ts | Adds NDJSON parsing helpers |
| apps/engine/src/lib/ndjson.test.ts | Adds NDJSON parsing tests |
| apps/engine/src/lib/index.ts | Adds engine library barrel |
| apps/engine/src/lib/exec.test.ts | Adds exec adapter tests (real connector bins) |
| apps/engine/src/lib/exec-helpers.ts | Adds subprocess spawn helpers |
| apps/engine/src/lib/destination-test.ts | Adds test destination connector |
| apps/engine/src/lib/destination-exec.ts | Adds subprocess destination adapter |
| apps/engine/src/index.ts | Adds app entry exports |
| apps/engine/src/cli/index.ts | Adds CLI executable entrypoint |
| apps/engine/src/cli/command.ts | Adds OpenAPI-generated CLI + serve command |
| apps/engine/src/api/index.ts | Adds standalone API server entrypoint |
| apps/engine/src/tests/sync.test.ts | Adds dockerized Postgres sync lifecycle tests |
| apps/engine/src/tests/docker.test.ts | Adds Docker image build/run tests |
| apps/engine/package.json | Adds publishable engine package manifest |
| README.md | Removes old repo README |
| Dockerfile | Reworks Docker build to use pnpm deploy layout |
| AGENTS.md | Rewrites contributor/agent instructions and repo map |
| .vscode/settings.json | Updates VS Code settings (Deno paths, Ruby LSP) |
| .vscode/extensions.json | Adds VS Code extension recommendation |
| .verdaccio/config.yaml | Adds Verdaccio config for local publish testing |
| .ruby-version | Pins Ruby version for temporal Ruby infra |
| .prettierignore | Ignores docs build output and terraform dirs |
| .npmrc | Adds scoped registry env-based config |
| .github/workflows/release.yml | Changes release workflow to “promote by SHA” |
| .github/workflows/docs.yml | Removes old MkDocs publishing workflow |
| .github/workflows/audit.yml | Adds scheduled repo audit workflow |
| .eslintrc.js | Removes old ESLint config file |
| .dockerignore | Allows dist/ into Docker context by removing ignore |
Files not reviewed (1)
- docs/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
apps/supabase/build.mjs:45
- The generated wrapper module re-exports
defaultfrom everynode:/npm:target. Many Node built-ins and packages do not provide a default export (e.g.node:fs,node:path), which will cause a module-linking error ('does not provide an export named default') when the bundled code is evaluated. To make this wrapper reliable, remove theexport { default } ...line (or only emit it for modules where a default export is guaranteed).
const spec = args.path
return {
loader: 'js',
contents: `
export * from ${JSON.stringify(spec)};
export { default } from ${JSON.stringify(spec)};
`,
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+3
| # Install deps and create standalone deployment | ||
| # Expects pre-built dist/ directories in the build context (from `pnpm build`) | ||
| FROM node:24-alpine AS build |
There was a problem hiding this comment.
The PR title/description claims the change is strictly additive and limited to packages/ts-cli/src/openapi/ergonomic/, but this diff introduces large-scale changes across docs, new apps (apps/engine, apps/service, apps/supabase, apps/visualizer), CI workflows, and Docker/Compose. Please update the PR title/description to match the actual scope (or split into separate PRs) so reviewers can validate the intended behavior and rollout implications accurately.
Comment on lines
+21
to
+23
| summary: 'Ingest a Stripe webhook event', | ||
| description: | ||
| "Receives a raw Stripe webhook event, verifies its signature using the pipeline's webhook secret, and enqueues it for processing by the active pipeline.", |
There was a problem hiding this comment.
The route description claims webhook signature verification, but the implementation forwards { body, headers } directly to push_event with no verification/authentication. This allows unauthenticated callers to enqueue arbitrary events for any pipeline_id. Mandatory fix: verify the Stripe signature using the pipeline’s webhook secret (or require an authentication mechanism) before accepting the request, and add a negative test asserting invalid signatures are rejected.
Comment on lines
+41
to
+47
| app.openapi(webhookRoute, async (c) => { | ||
| const { pipeline_id } = c.req.valid('param') | ||
| const body = await c.req.text() | ||
| const headers = Object.fromEntries(c.req.raw.headers.entries()) | ||
| push_event(pipeline_id, { body, headers }) | ||
| return c.text('ok', 200) | ||
| }) |
There was a problem hiding this comment.
The route description claims webhook signature verification, but the implementation forwards { body, headers } directly to push_event with no verification/authentication. This allows unauthenticated callers to enqueue arbitrary events for any pipeline_id. Mandatory fix: verify the Stripe signature using the pipeline’s webhook secret (or require an authentication mechanism) before accepting the request, and add a negative test asserting invalid signatures are rejected.
| ): Promise<StateStore & { close?(): Promise<void> }> { | ||
| try { | ||
| const { name: destName, ...destConfig } = params.destination | ||
| const pkg = await import(`@stripe/sync-state-${destName}`) |
There was a problem hiding this comment.
Dynamic importing based on destination.name means runtime behavior is driven by user-controlled config. At minimum, constrain destName to an expected safe pattern (e.g. lowercase alphanumerics + dashes) before constructing an import specifier. This reduces the risk of unexpected resolution behavior and makes failures easier to diagnose.
Suggested change
| const pkg = await import(`@stripe/sync-state-${destName}`) | |
| const safeDestName = String(destName) | |
| // Constrain destination name to a safe pattern to avoid unexpected import resolution. | |
| if (!/^[a-z0-9-]+$/.test(safeDestName)) { | |
| return noopStateStore() | |
| } | |
| const pkg = await import(`@stripe/sync-state-${safeDestName}`) |
Comment on lines
+84
to
+93
| export function fileLogSink(filePath: string): LogSink { | ||
| return { | ||
| write(pipelineId, entry) { | ||
| const dir = join(filePath, '..') | ||
| ensureDir(dir) | ||
| const line = JSON.stringify({ pipelineId, ...entry }) + '\n' | ||
| writeFileSync(filePath, line, { flag: 'a' }) | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
LogSink.write() is documented as 'Fire-and-forget, non-blocking', but writeFileSync(..., { flag: 'a' }) blocks the event loop for every log line. For anything beyond trivial usage this will degrade request throughput and increase tail latency. Prefer an async append (or a buffered write stream) so logging doesn’t block pipeline execution.
|
|
||
| ## How It Works | ||
| ## Quick start | ||
|
|
There was a problem hiding this comment.
The quick start flags (--database-url, --schema) don’t match the CLI flag naming documented elsewhere in this PR (e.g. docs/service/cli.md maps --postgres-url / --postgres-schema). Please align the homepage example with the actual CLI flags to prevent copy/paste failures.
tonyxiao
force-pushed
the
feat/ergonomic-openapi-cli
branch
from
c299605 to
bc3e74e
Compare
tonyxiao
force-pushed
the
feat/ergonomic-openapi-cli
branch
from
bc3e74e to
79e77c1
Compare
Collaborator
Author
|
Thanks for the review, Copilot. The large diff (545 files) was an artifact of the old base branch ( The suppressed comment about |
tonyxiao
force-pushed
the
feat/ergonomic-openapi-cli
branch
from
5b11581 to
7cd15dd
Compare
- Flatten docs/pages/ into docs/{architecture,engine,service,plans,guides}
- Update build.mjs to scan from docs/ root with exclusion filter
- Rewrite AGENTS.md as a progressive-disclosure map with package table
- Add docs/architecture/{principles,decisions,quality}.md
- Add e2e/layers.test.ts enforcing source/dest isolation, protocol
independence, connector deps, and service isolation
- Extract tsconfig.base.json, all packages extend it
- Delete stale .eslintrc.js, TODOs.md; add docs/plans/backlog.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Committed-By-Agent: claude
- scripts/audit-repo.sh: local script using Claude Code CLI to audit docs accuracy, architecture violations, dead code, quality gaps, and convention drift. Supports --pr flag to auto-fix and create PR. - .github/workflows/audit.yml: weekly GitHub Action (Monday 9am UTC) that runs the audit. Warns if ANTHROPIC_API_KEY secret is missing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Committed-By-Agent: claude
…N-in-header params Adds a new `createErgonomicCli()` that detects OAS 3.1 `contentMediaType` + `contentSchema` on header params and decomposes them into human-friendly flags (--source, --source-config, --destination, --streams, --config) with config cascade support (flags > env vars > file). Strictly additive — no existing files modified. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Committed-By-Agent: claude
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Committed-By-Agent: claude
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Committed-By-Agent: claude
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Committed-By-Agent: claude
…tls-semantics Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Committed-By-Agent: claude
tonyxiao
force-pushed
the
feat/ergonomic-openapi-cli
branch
from
362b3a6 to
b553a59
Compare
…stgres and temporal tests
Both tests called stripe.products.list({ limit: 1 }) and updated the same product,
causing intermittent failures when both ran in parallel. Each test now creates its
own unique fixture product and filters events by product id.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude
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.
Summary
createErgonomicCli()that decomposes JSON-in-header OpenAPI params into human-friendly CLI flagscontentMediaType+contentSchema, resolves$ref, and generates per-property flags with roles (name,config,list,json,scalar,base-config)config.tsutilities)packages/ts-cli/src/openapi/ergonomic/, no existing files modifiedBefore:
sync-engine read --x-pipeline '{"source":{"name":"stripe","api_key":"sk_test_..."},...}'After:
sync-engine read --source stripe --source-config '{"api_key":"sk_test_..."}' --destination postgres --streams accountsTest plan
pnpm testin packages/ts-cli)pnpm buildpassespnpm format+eslintpass🤖 Generated with Claude Code