fix: make core declarations portable by tonyketcham · Pull Request #239 · FlatbreadLabs/flatbread

tonyketcham · 2026-08-12T17:49:22Z

Use GraphQL's public types in emitted declarations and declare vfile as a runtime dependency because the public API exposes VFile. Add checks that prevent private GraphQL paths and missing declaration dependencies from returning.\n\nTest plan: pnpm verify; strict clean-consumer TypeScript compile.

Depends-On: #238

tonyketcham · 2026-08-12T17:49:29Z

This pull request is part of a Mergify stack:

#	Pull Request	Link
1	release: prepare Flatbread 1.0	#236
2	fix: remove stale release dependencies	#237
3	fix: harden stable package payloads	#238
4	fix: make core declarations portable	#239	👈
5	docs: record upstream release limits	#240
6	chore: move proof to its own repository	#241
7	fix: retarget Cursor agents at external Proof	#242
8	feat: brand agent memory as Proof and the runner as Oven	#244

Use GraphQL's public types in emitted declarations and declare vfile as a runtime dependency because the public API exposes VFile. Add checks that prevent private GraphQL paths and missing declaration dependencies from returning.\n\nTest plan: pnpm verify; strict clean-consumer TypeScript compile. Change-Id: I3f66f068fa24441f598224f2cfbbec48e722695c

tonyketcham · 2026-08-12T18:16:36Z

Revision history

#	Type	Changes	Reason	Date
1	initial	`2bc8968`		2026-08-12 18:16 UTC
2	rebase	`2bc8968 → 5169a39` (rebase only)		2026-08-12 18:16 UTC

cursor

Stale comment

Review verdict

REQUEST_CHANGES — consensus HIGH on packages/core/src/types.test.ts:70-90 (declaration guard is negative-only and build-order-coupled) plus a non-empty coverage plan for the documented packed-install / ExecutionResult contract while non-test source changed.

The source fix itself looks sound (public GraphQL types, explicit ExecutionResult, vfile → dependencies). The blocking issue is that the new guards can false-pass and do not sticky-lock the CHANGELOG contract.

Chunk-bound feedback

HIGH packages/core/src/types.test.ts:70-90 — keep the jsutils ban; add missing-dist message + positive Promise<ExecutionResult> / public from 'graphql' (and ideally from 'vfile') asserts.

MED packages/core/src/providers/base.ts:27-27 — add compile-time Equal<ReturnType<FlatbreadProvider['query']>, Promise<ExecutionResult>>.

MED packages/core/src/types.test.ts:41-43 — assert equality to GraphQLFieldConfig<...>['description'], not only the widened union.

Coverage plan (priority)

Missing dist/index.d.ts → explicit assertion failure (not raw ENOENT).

Emitted declarations contain Promise<ExecutionResult> and public from 'graphql' (keep jsutils ban).

Compile-time sticky assert on FlatbreadProvider['query'] return type.

Prefer a packed-install / clean-consumer tsc smoke for the CHANGELOG claim (or a dedicated CI job if too heavy for AVA).

Stronger Override['description'] equality + optional from 'vfile' emit check / import-specifier ↔ dependencies scan.

Reviewer scoreboard

correctness-and-contracts: 0 findings, 3 coverage gaps, signal:MED

test-coverage-robustness: 4 findings, 6 coverage gaps, signal:HIGH

release-discipline: 0 findings, 2 coverage gaps, signal:MED

docs-and-positioning: 0 findings, 3 coverage gaps, signal:MED

dependency-runtime-surface: 1 findings, 4 coverage gaps, signal:MED

Perspectives: correctness-and-contracts, test-coverage-robustness, release-discipline, docs-and-positioning, dependency-runtime-surface. Models: grok-4.5 (effort=high) / composer-2.5. Scope: a9543f2...5169a39.

_{Sent by Cursor Automation: Flatbread PR Review}

cursor · 2026-08-12T18:24:40Z

+test('core declarations use only the public GraphQL type surface', (t) => {
+  const declarations = readFileSync(
+    new URL('../dist/index.d.ts', import.meta.url),
+    'utf8'
+  );
+
+  t.false(declarations.includes('graphql/jsutils/'));
+});
+
+test('core publishes dependencies used by its declarations', (t) => {
+  const manifest = JSON.parse(
+    readFileSync(new URL('../package.json', import.meta.url), 'utf8')
+  ) as {
+    dependencies?: Record<string, string>;
+    devDependencies?: Record<string, string>;
+  };
+
+  t.is(manifest.dependencies?.vfile, '5.3.4');
+  t.false('vfile' in (manifest.devDependencies ?? {}));
+});
+


HIGH (consensus: test-coverage-robustness, dependency-runtime-surface)

The declaration guard is negative-only (t.false(...includes('graphql/jsutils/'))) and build-order-coupled (readFileSync on dist/index.d.ts → opaque ENOENT if dist is missing). Empty or wrong declarations can still pass, and nothing locks Promise<ExecutionResult> / public from 'graphql' (or from 'vfile') stickiness.

Minimal fix: fail missing dist with an explicit assertion message; keep the jsutils ban; add positive asserts that declarations include Promise<ExecutionResult> and public graphql/vfile imports; optionally assert every bare import specifier is covered by package.json dependencies.

cursor · 2026-08-12T18:24:40Z

   * @returns GraphQL response
   */
-  async query(args: Omit<GraphQLArgs, 'schema'>) {
+  async query(args: Omit<GraphQLArgs, 'schema'>): Promise<ExecutionResult> {


MED (test-coverage-robustness)

Explicit Promise<ExecutionResult> fixes the ObjMap leak, but nothing type- or emit-checks that return contract. Promise<any> / unknown would still satisfy a jsutils-only ban.

Minimal fix: add Equal<ReturnType<FlatbreadProvider['query']>, Promise<ExecutionResult>> and/or a dist positive match for query(...): Promise<ExecutionResult>.

cursor · 2026-08-12T18:24:40Z

+type OverrideDescriptionMatchesGraphQL = Assert<
+  Equal<Override['description'], string | null | undefined>
+>;


MED (test-coverage-robustness)

Equal<Override['description'], string | null | undefined> also held for the old Maybe<string> import, so it does not prove portable public-surface derivation.

Minimal fix: assert equality to GraphQLFieldConfig<unknown, unknown>['description'] and rely on stronger dist positives for the portability claim.

cursor

Review verdict

REQUEST_CHANGES — Consensus HIGH on packages/core/src/types.test.ts:70-90: the new portability suite is negative-only (graphql/jsutils/ absent) plus a manifest pin, so a wrong-but-jsutils-free emit, a dropped ExecutionResult/vfile surface, or a stale/missing dist can still ship green under test:ava. Source contracts themselves look correct; the merge blocks on under-locked regression coverage of the documented packed-types fix.

Chunk-bound feedback (priority)

HIGH types.test.ts:70-90 — Add positive .d.ts substrings (ExecutionResult, query(…): Promise<ExecutionResult>, from 'vfile'), fail clearly on missing/stale dist, and ideally require every from '…' in dist/index.d.ts ∈ dependencies.
MED providers/base.ts:27 — Lock ReturnType<FlatbreadProvider['query']> to Promise<ExecutionResult> (and/or .d.ts signature check).
MED types.test.ts:41-43 — Pin Override['description'] to GraphQLFieldConfig<…>['description'], not the expanded union alone.
MED package.json vfile — Keep the runtime move; assert declarations still name vfile (manifest pin alone is incomplete).
LOW CHANGELOG — Separate Maybe removal vs ExecutionResult (ObjMap) return annotation when wording is next touched.

Coverage plan

Positive emit asserts + fresh-dist guard (HIGH).
Declaration dependency closure over from '…' specifiers (HIGH/MED).
Compile-time Equal for query return and Override.description (MED).
Keep vfile dependencies pin; optional graphql mirror (MED).

Reviewer scoreboard

correctness-and-contracts: COMMENT — solid contract read; undersold emit-gap severity.
test-coverage-robustness: REQUEST_CHANGES — decisive on false-pass / positive-emit holes.
release-discipline: COMMENT — stack/1.0.0 fold-in OK; CHANGELOG mechanism wording LOW.
dependency-runtime-surface: APPROVE — packaging surface correct; specifier∈deps folds into coverage plan.

Models: grok-4.5 (HIGH) / composer-2.5 (MED). Canvas: dag-review-5169a39.canvas.tsx.

_{Sent by Cursor Automation: Flatbread PR Review}

cursor · 2026-08-12T18:25:37Z

+test('core declarations use only the public GraphQL type surface', (t) => {
+  const declarations = readFileSync(
+    new URL('../dist/index.d.ts', import.meta.url),
+    'utf8'
+  );
+
+  t.false(declarations.includes('graphql/jsutils/'));


HIGH — Negative-only portability guard.

t.false(…includes('graphql/jsutils/')) plus the manifest pin below can still green a wrong emit (no ExecutionResult / no vfile), or a stale/missing dist under bare test:ava.

Minimal fix: After ensuring dist/index.d.ts is present/fresh (clear failure if not), assert positive substrings for public ExecutionResult / query(…): Promise<ExecutionResult>, from 'vfile', and ideally that every from '…' package in the emit appears under dependencies.

cursor · 2026-08-12T18:25:37Z

   * @returns GraphQL response
   */
-  async query(args: Omit<GraphQLArgs, 'schema'>) {
+  async query(args: Omit<GraphQLArgs, 'schema'>): Promise<ExecutionResult> {


MED — Explicit Promise<ExecutionResult> is the right public return, but nothing type- or .d.ts-locks FlatbreadProvider['query'] to that contract.

Minimal fix: Add Equal<ReturnType<FlatbreadProvider['query']>, Promise<ExecutionResult>> and/or a .d.ts signature substring check beside the emit suite.

cursor · 2026-08-12T18:25:37Z

+type OverrideDescriptionMatchesGraphQL = Assert<
+  Equal<Override['description'], string | null | undefined>
+>;


MED — This pins the expanded Maybe<string> union, not stickiness to GraphQLFieldConfig<…>['description']. A hardcoded identical union still passes.

Minimal fix: Equal<Override['description'], GraphQLFieldConfig<unknown, unknown>['description']> (optional null assignability) and rely on a strengthened emit scan for published stickiness.

cursor · 2026-08-12T18:25:37Z

    "matcher": "5.0.0",
-    "plur": "5.1.0"
+    "plur": "5.1.0",
+    "vfile": "5.3.4"


MED — Runtime vfile placement is correct, but the new test only checks manifest shape, not that declarations still name vfile.

Minimal fix: Assert dist/index.d.ts imports vfile alongside the existing dependencies pin. Exact 5.3.4 is fine if stack pin discipline is intentional.

cursor · 2026-08-12T18:25:37Z

+- `@flatbread/core` no longer emits type declarations a packed install cannot
+  resolve. Its `.d.ts` files reached into the private paths
+  `graphql/jsutils/Maybe` and `graphql/jsutils/ObjMap`; they now use public
+  GraphQL types, and `FlatbreadProvider.query()` declares its return as
+  GraphQL's public `ExecutionResult`. `vfile@5.3.4` moves from devDependencies
+  to dependencies, because the public types name `VFile`.


LOW — Outcome is accurate post-build, but the bullet attributes both Maybe and ObjMap to “.d.ts reached into private paths.” Only Maybe was removed in source; ObjMap leakage was closed by the explicit ExecutionResult return on query().

Minimal fix: Split or qualify so both mechanisms are visible.

tonyketcham mentioned this pull request Aug 12, 2026

docs: record upstream release limits #240

Merged

tonyketcham force-pushed the stack/tonyketcham/toeknee/extract-proof-repo/harden-stable-pkg-payloads--69eee151 branch from 2d66900 to a9543f2 Compare August 12, 2026 18:16

tonyketcham force-pushed the stack/tonyketcham/toeknee/extract-proof-repo/make-core-declarations-portable--3f66f068 branch from 2bc8968 to 5169a39 Compare August 12, 2026 18:16

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Base automatically changed from stack/tonyketcham/toeknee/extract-proof-repo/harden-stable-pkg-payloads--69eee151 to main August 12, 2026 19:24

tonyketcham merged commit d9c5e11 into main Aug 12, 2026
20 of 38 checks passed

tonyketcham deleted the stack/tonyketcham/toeknee/extract-proof-repo/make-core-declarations-portable--3f66f068 branch August 12, 2026 19:24

Navigation Menu

Uh oh!

Uh oh!

fix: make core declarations portable - #239

fix: make core declarations portable#239
tonyketcham merged 1 commit into
mainfrom
stack/tonyketcham/toeknee/extract-proof-repo/make-core-declarations-portable--3f66f068

tonyketcham commented Aug 12, 2026

Uh oh!

tonyketcham commented Aug 12, 2026 •

edited

Loading

Uh oh!

tonyketcham commented Aug 12, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

tonyketcham commented Aug 12, 2026

Uh oh!

tonyketcham commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonyketcham commented Aug 12, 2026

Revision history

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review verdict

Chunk-bound feedback

Coverage plan (priority)

Reviewer scoreboard

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Review verdict

Chunk-bound feedback (priority)

Coverage plan

Reviewer scoreboard

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tonyketcham commented Aug 12, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading