fix(console): route Models and OKF through consumer GraphQL - #203
Conversation
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughWalkthroughThe Console now routes OKF and observed-model operations through authenticated consumer GraphQL. It adds a tenant-scoped local development model store, fallback behavior, acceptance tests, an opt-in live smoke test, and a Models data-door validation gate. ChangesModels consumer GraphQL flow
Sequence Diagram(s)sequenceDiagram
participant OKFRoute as OKF route
participant ConsumerClient as executeConsumerGraphql
participant ConsumerAPI as Consumer GraphQL API
OKFRoute->>ConsumerClient: Submit OKF query or apply mutation
ConsumerClient->>ConsumerAPI: Send authenticated tenant-scoped request
ConsumerAPI-->>ConsumerClient: Return GraphQL response
ConsumerClient-->>OKFRoute: Return mapped HTTP result
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR moves the Console “Models” and OKF data paths off Harness MCP GraphQL and onto the authenticated CommonPlace consumer HTTP GraphQL endpoint, adding a local-dev-only stand-in when the consumer endpoint is unconfigured and strengthening guardrails via tests and a new deny-list gate.
Changes:
- Add an authenticated consumer GraphQL transport (
executeConsumerGraphql) with expected-field validation and standardized failure reasons. - Route observed/declared model reads + mutations and OKF bundle operations through the consumer GraphQL transport, with a non-production local fallback only when the consumer endpoint is unconfigured.
- Add regression coverage (Vitest + live smoke) and a Models data-door gate that rejects agent-door imports/endpoint references across the Models route family.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/records/011-console-single-door.md | Updates the single-door record to include Models/OKF routing and the new Models deny-list gate. |
| apps/console/src/lib/server/observed-model.live.test.ts | Adds an opt-in live smoke test for the deployed Models route. |
| apps/console/src/lib/server/observed-model-harness.ts | Switches Models GraphQL adapter from harness transport to consumer HTTP GraphQL + local-dev fallback. |
| apps/console/src/lib/server/observed-model-harness.test.ts | Adds acceptance tests for Models transport behavior, fallback, and response validation. |
| apps/console/src/lib/server/local-dev-declared-model-store.ts | Introduces an in-memory LocalDevDeclaredModelStore stand-in for unconfigured consumer endpoints (non-prod only). |
| apps/console/src/lib/server/local-dev-declared-model-store.test.ts | Adds unit tests for the LocalDevDeclaredModelStore behavior and tenant/topic isolation. |
| apps/console/src/lib/server/consumer-graphql-client.ts | Adds shared authenticated consumer GraphQL HTTP client with required-field validation. |
| apps/console/src/app/api/observed-model/okf/route.ts | Routes OKF preview/export/import through consumer GraphQL fields (okfModel, okfModelApply). |
| apps/console/src/app/api/observed-model/okf/route.test.ts | Adds acceptance tests for OKF route behavior and invalid-response handling. |
| apps/console/scripts/check-models-data-door.test.mjs | Adds node:test coverage for the Models data-door gate (positive + deliberate-failure cases). |
| apps/console/scripts/check-models-data-door.mjs | Adds a gate script to forbid agent-door references within the Models route family + adapters. |
| apps/console/package.json | Wires the new Models data-door gate into test and gates. |
Suppressed comments (1)
apps/console/src/lib/server/local-dev-declared-model-store.ts:299
unpinLocalDevDeclaredfilters relations usingrelation.id !== declaredId, butdeclaredIdis an object type id (e.g.ot:...) while relation ids arerel:..., so this condition is redundant and misleading. Filtering onobjectTypeId/targetObjectTypeIdis sufficient.
const relations = bucket.declared.relations.filter(
(relation) =>
relation.id !== declaredId
&& relation.objectTypeId !== declaredId
&& relation.targetObjectTypeId !== declaredId,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const prior = bucket.declared.objectTypes.find((type) => type.key === key); | ||
| const objectTypes = prior | ||
| ? bucket.declared.objectTypes.map((type) => (type.id === prior.id ? { ...objectType, id: prior.id } : type)) | ||
| : [...bucket.declared.objectTypes, objectType]; | ||
| const resolvedId = prior?.id ?? objectTypeId; | ||
| const nextFields = [ | ||
| ...bucket.declared.fields.filter((field) => field.objectTypeId !== resolvedId), | ||
| ...fields.map((field) => ({ ...field, objectTypeId: resolvedId, id: `f:${key}:${field.key}` })), | ||
| ]; | ||
| const nextObjectTypes = objectTypes.map((type) => | ||
| type.id === resolvedId ? { ...objectType, id: resolvedId } : type, | ||
| ); |
| | Models via data API | `observed-model-harness.ts` uses the authenticated consumer HTTP GraphQL client; authenticated `LocalDevDeclaredModelStore` substitution is allowed only outside production when no consumer endpoint is configured | | ||
| | OKF via data API | `/api/observed-model/okf` calls consumer `okfModel` and `okfModelApply` fields | | ||
| | Models deny-list | `gate:models-data-door` rejects agent-door imports and endpoint references across the Models route family and its consumer transport adapter | | ||
| | One data URL | Railway console: `CONSOLE_DATA_API_URL` reference to commonplace-api; `THEOREM_NODE_URL` and `THEOREM_GRAPHQL_URL` removed | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/console/src/lib/server/local-dev-declared-model-store.ts (1)
284-300: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
unpinLocalDevDeclaredignores field and relation identifiers.The filters treat
declaredIdas an object type id forobjectTypesandfields. If a caller passes a field id such asf:customer:email, no field is removed, but lines 301-328 still bump the version and supersede the prior version. The harnessunpinDeclaredpath then reportsstatus: 'applied'for an unpin that changed nothing.Add a field-id branch, or return the unchanged model without a version bump when
declaredIdmatches nothing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/server/local-dev-declared-model-store.ts` around lines 284 - 300, The unpinLocalDevDeclared function must handle field and relation IDs instead of always treating declaredId as an object type ID. Add matching logic for field and relation identifiers, or return the unchanged model without version/supersession updates when no object type, field, or relation matches; ensure unpinDeclared reports no applied change for unknown IDs.apps/console/src/lib/server/local-dev-declared-model-store.test.ts (1)
31-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a repeated declaration of the same name.
The suite does not declare the same
nameSingulartwice. That case exposes the unreachablepriorbranch reported inapps/console/src/lib/server/local-dev-declared-model-store.tslines 203-242, where the second declaration createsshipment_2. Add a test that asserts the intended behavior after the store fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/server/local-dev-declared-model-store.test.ts` around lines 31 - 50, Extend the test suite around declareLocalSchema with a repeated declaration using the same nameSingular, such as shipment, after the initial declaration in the existing test. Assert the intended result for the second declaration, including the generated shipment_2 key and its declared status, so the prior-branch behavior in declareLocalSchema is covered.apps/console/src/lib/server/observed-model-harness.test.ts (1)
208-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the mutation local development fallback.
The suite tests the unconfigured fallback only for
readObservedModels. The new fallback branches inunpinDeclared(lines 844-861) anddeclareSchema(lines 878-890) ofapps/console/src/lib/server/observed-model-harness.tsare not exercised. Add two tests withCONSOLE_DATA_API_URLunset that assert each mutation returns the local store result and thatfetchis not called.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/server/observed-model-harness.test.ts` around lines 208 - 236, Add separate tests for unpinDeclared and declareSchema with CONSOLE_DATA_API_URL unset, covering their local-development fallback branches. Assert each mutation returns the expected local store result and that fetch is not called, alongside the existing observed-model harness tests.apps/console/src/lib/server/consumer-graphql-client.ts (1)
110-120: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider rejecting null expected fields.
hasOwnPropertyaccepts an explicitnullvalue. A GraphQL error envelope with partial data, for example{"data":{"okfModel":null},"errors":[...]}, is already caught by the errors check. However, an upstream that returns{"data":{"okfModel":null}}with no errors passes validation. The OKF route then returns anullJSON body with status 200.♻️ Proposed stricter field validation
const missingField = expectedFields.find( - (field) => !Object.prototype.hasOwnProperty.call(payload.data, field), + (field) => payload.data?.[field] === undefined || payload.data[field] === null, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/server/consumer-graphql-client.ts` around lines 110 - 120, Update the expected-field validation in the response-checking function around missingField so a field is considered invalid when it is absent or its value is null. Preserve the existing 502 response, error prefix, and upstream_error reason for either case, while continuing to accept present non-null values.apps/console/src/lib/server/observed-model-harness.ts (1)
825-829: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
pinObservedhas no local development fallback, unlikeunpinDeclaredanddeclareSchema.When the consumer endpoint is unset outside production,
readObservedModels,readDeclaredModel,unpinDeclared, anddeclareSchemareturn local store results.pinObservedreturnsobserved_model_graphql_unconfiguredwith status 404.restoreDeclaredModel,proposeSchemaChange, andcompileDeclaredModelbehave the same way. A developer can therefore declare and unpin object types locally but cannot pin an observed type.Either add the fallback for
pinObserved, or document which Models operations require a configured consumer endpoint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/lib/server/observed-model-harness.ts` around lines 825 - 829, Update the pinObserved handler to detect an unset consumer endpoint outside production and return the corresponding local-store result, matching the fallback behavior used by unpinDeclared and declareSchema. Keep the configured-endpoint path using executeGraphql with PIN_MUTATION unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/console/src/lib/server/local-dev-declared-model-store.ts`:
- Around line 203-242: Redefinition currently generates suffixed duplicate keys
because prior lookup occurs only after uniqueKey; in
apps/console/src/lib/server/local-dev-declared-model-store.ts:203-242, resolve
the existing object type using the raw slug before calling uniqueKey, then
remove the redundant prior lookup and no-op nextObjectTypes remap while
preserving the existing object type ID and fields on redeclaration. In
apps/console/src/lib/server/local-dev-declared-model-store.test.ts:31-50, add
coverage declaring the same nameSingular twice and assert the expected single
object type count and stable key.
In `@docs/records/011-console-single-door.md`:
- Line 66: The “One data URL” row incorrectly claims THEOREM_GRAPHQL_URL was
removed; update the row to limit that claim to the Models and OKF cutover or
explicitly state that Proactivity and Filing CommonPlace GraphQL flows still
require THEOREM_GRAPHQL_URL. Ensure those flows use THEOREM_GRAPHQL_URL and
never fall back to CONSOLE_HARNESS_URL.
---
Nitpick comments:
In `@apps/console/src/lib/server/consumer-graphql-client.ts`:
- Around line 110-120: Update the expected-field validation in the
response-checking function around missingField so a field is considered invalid
when it is absent or its value is null. Preserve the existing 502 response,
error prefix, and upstream_error reason for either case, while continuing to
accept present non-null values.
In `@apps/console/src/lib/server/local-dev-declared-model-store.test.ts`:
- Around line 31-50: Extend the test suite around declareLocalSchema with a
repeated declaration using the same nameSingular, such as shipment, after the
initial declaration in the existing test. Assert the intended result for the
second declaration, including the generated shipment_2 key and its declared
status, so the prior-branch behavior in declareLocalSchema is covered.
In `@apps/console/src/lib/server/local-dev-declared-model-store.ts`:
- Around line 284-300: The unpinLocalDevDeclared function must handle field and
relation IDs instead of always treating declaredId as an object type ID. Add
matching logic for field and relation identifiers, or return the unchanged model
without version/supersession updates when no object type, field, or relation
matches; ensure unpinDeclared reports no applied change for unknown IDs.
In `@apps/console/src/lib/server/observed-model-harness.test.ts`:
- Around line 208-236: Add separate tests for unpinDeclared and declareSchema
with CONSOLE_DATA_API_URL unset, covering their local-development fallback
branches. Assert each mutation returns the expected local store result and that
fetch is not called, alongside the existing observed-model harness tests.
In `@apps/console/src/lib/server/observed-model-harness.ts`:
- Around line 825-829: Update the pinObserved handler to detect an unset
consumer endpoint outside production and return the corresponding local-store
result, matching the fallback behavior used by unpinDeclared and declareSchema.
Keep the configured-endpoint path using executeGraphql with PIN_MUTATION
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ef984433-0a95-4348-920d-42b277aa1994
📒 Files selected for processing (12)
apps/console/package.jsonapps/console/scripts/check-models-data-door.mjsapps/console/scripts/check-models-data-door.test.mjsapps/console/src/app/api/observed-model/okf/route.test.tsapps/console/src/app/api/observed-model/okf/route.tsapps/console/src/lib/server/consumer-graphql-client.tsapps/console/src/lib/server/local-dev-declared-model-store.test.tsapps/console/src/lib/server/local-dev-declared-model-store.tsapps/console/src/lib/server/observed-model-harness.test.tsapps/console/src/lib/server/observed-model-harness.tsapps/console/src/lib/server/observed-model.live.test.tsdocs/records/011-console-single-door.md
| const key = uniqueKey(bucket.declared, slugify(input.nameSingular || input.labelSingular)); | ||
| const objectTypeId = `ot:${key}`; | ||
| const contentAnchor = `local:${key}:v${bucket.versionSeq + 1}`; | ||
| const objectType: ObjectTypeMetadata = { | ||
| id: objectTypeId, | ||
| key, | ||
| label: input.labelSingular || input.nameSingular || key, | ||
| description: input.description, | ||
| nodeLabel: input.nodeLabel || input.labelSingular || key, | ||
| enforcement: input.enforcement, | ||
| nameSingular: input.nameSingular || key, | ||
| namePlural: input.namePlural || `${key}s`, | ||
| labelIdentifierField: input.labelIdentifierField || input.fields[0]?.key || 'id', | ||
| system: input.system, | ||
| contentAnchor, | ||
| provider: { kind: 'declared-record' }, | ||
| }; | ||
| const fields: FieldMetadata[] = input.fields.map((field) => ({ | ||
| id: `f:${key}:${field.key}`, | ||
| objectTypeId, | ||
| key: field.key, | ||
| label: field.label, | ||
| ...(field.description ? { description: field.description } : {}), | ||
| fieldType: field.fieldType, | ||
| required: field.required, | ||
| system: field.system, | ||
| })); | ||
|
|
||
| const prior = bucket.declared.objectTypes.find((type) => type.key === key); | ||
| const objectTypes = prior | ||
| ? bucket.declared.objectTypes.map((type) => (type.id === prior.id ? { ...objectType, id: prior.id } : type)) | ||
| : [...bucket.declared.objectTypes, objectType]; | ||
| const resolvedId = prior?.id ?? objectTypeId; | ||
| const nextFields = [ | ||
| ...bucket.declared.fields.filter((field) => field.objectTypeId !== resolvedId), | ||
| ...fields.map((field) => ({ ...field, objectTypeId: resolvedId, id: `f:${key}:${field.key}` })), | ||
| ]; | ||
| const nextObjectTypes = objectTypes.map((type) => | ||
| type.id === resolvedId ? { ...objectType, id: resolvedId } : type, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Re-declaration semantics in the local store are undefined and untested. uniqueKey on line 203 always returns a key that no object type uses, so the prior upsert branch on line 231 is unreachable and a repeated declaration creates a suffixed duplicate such as shipment_2.
apps/console/src/lib/server/local-dev-declared-model-store.ts#L203-L242: resolve the prior object type by the raw slug before callinguniqueKey, then remove the redundantpriorlookup and the no-opnextObjectTypesremap.apps/console/src/lib/server/local-dev-declared-model-store.test.ts#L31-L50: add a test that declares the samenameSingulartwice and asserts the intended object type count and key.
📍 Affects 2 files
apps/console/src/lib/server/local-dev-declared-model-store.ts#L203-L242(this comment)apps/console/src/lib/server/local-dev-declared-model-store.test.ts#L31-L50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/lib/server/local-dev-declared-model-store.ts` around lines
203 - 242, Redefinition currently generates suffixed duplicate keys because
prior lookup occurs only after uniqueKey; in
apps/console/src/lib/server/local-dev-declared-model-store.ts:203-242, resolve
the existing object type using the raw slug before calling uniqueKey, then
remove the redundant prior lookup and no-op nextObjectTypes remap while
preserving the existing object type ID and fields on redeclaration. In
apps/console/src/lib/server/local-dev-declared-model-store.test.ts:31-50, add
coverage declaring the same nameSingular twice and assert the expected single
object type count and stable key.
| | Models via data API | `observed-model-harness.ts` uses the authenticated consumer HTTP GraphQL client; authenticated `LocalDevDeclaredModelStore` substitution is allowed only outside production when no consumer endpoint is configured | | ||
| | OKF via data API | `/api/observed-model/okf` calls consumer `okfModel` and `okfModelApply` fields | | ||
| | Models deny-list | `gate:models-data-door` rejects agent-door imports and endpoint references across the Models route family and its consumer transport adapter | | ||
| | One data URL | Railway console: `CONSOLE_DATA_API_URL` reference to commonplace-api; `THEOREM_NODE_URL` and `THEOREM_GRAPHQL_URL` removed | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the THEOREM_GRAPHQL_URL claim.
Limit this row to the Models and OKF cutover, or state which flows still require THEOREM_GRAPHQL_URL. The current text says that the variable was removed, but Proactivity and Filing GraphQL requests must use it.
As per coding guidelines, use THEOREM_GRAPHQL_URL for Proactivity and Filing CommonPlace GraphQL requests. Never fall back to CONSOLE_HARNESS_URL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/records/011-console-single-door.md` at line 66, The “One data URL” row
incorrectly claims THEOREM_GRAPHQL_URL was removed; update the row to limit that
claim to the Models and OKF cutover or explicitly state that Proactivity and
Filing CommonPlace GraphQL flows still require THEOREM_GRAPHQL_URL. Ensure those
flows use THEOREM_GRAPHQL_URL and never fall back to CONSOLE_HARNESS_URL.
Source: Coding guidelines
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4085b76b51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } { | ||
| const bucket = ensureBucket(topicId, tenant); | ||
| const objectTypes = bucket.declared.objectTypes.filter((type) => type.id !== declaredId); | ||
| const fields = bucket.declared.fields.filter((field) => field.objectTypeId !== declaredId); |
There was a problem hiding this comment.
When the consumer endpoint is unset outside production, the declared-fields UI submits field.id to this local fallback, but this predicate compares only field.objectTypeId with that ID. The field therefore remains in the registry even though the route returns an applied receipt and advances the schema version; filter by field.id as well while retaining the object-type cascade.
AGENTS.md reference: apps/console/AGENTS.md:L107-L107
Useful? React with 👍 / 👎.
| readonly declared: DeclaredModel; | ||
| } { | ||
| const bucket = ensureBucket(topicId, tenant); | ||
| const key = uniqueKey(bucket.declared, slugify(input.nameSingular || input.labelSingular)); |
There was a problem hiding this comment.
When the consumer endpoint is unset outside production, applyFieldEdit rebuilds the declaration with the existing nameSingular and expectedContentAnchor, but this call always uniquifies a matching key, such as changing customer to customer_2. Every field edit therefore adds a second object type while leaving the original field unchanged despite reporting success; resolve an existing type before generating a unique key and replace that type's fields.
AGENTS.md reference: apps/console/AGENTS.md:L107-L107
Useful? React with 👍 / 👎.
| RESTORE_MUTATION, | ||
| { versionId }, | ||
| 'mutate', | ||
| ['restoreDeclaredModel'], |
There was a problem hiding this comment.
When the consumer endpoint is unset outside production, model reads expose the local store's version history and the UI enables its restore actions, but this operation has no equivalent local fallback and immediately returns the unconfigured 404. After any local declaration or unpin, users can select and compare those local versions but every Restore action fails; either restore the selected snapshot in LocalDevDeclaredModelStore or stop presenting its versions as restorable.
AGENTS.md reference: apps/console/AGENTS.md:L107-L107
Useful? React with 👍 / 👎.
Summary
LocalDevDeclaredModelStoreonly when the consumer endpoint is unset, only outside production, and only after authenticated tenant resolutionBackend dependency
Depends on Theorem PR https://github.com/Travis-Gilbert/Theorem/pull/515.
The backend PR must merge and deploy before this consumer branch is deployed. Its
commonplace-apischema provides the Models and OKF fields used here.Authentication note
Tenant identity is credential-derived, not supplied as a client GraphQL argument. The configured owner tenant can still use the existing service-key fallback until principal-token issuance is available across the deployment. That broader identity closure remains follow-up plan work and is not represented here as already solved.
Validation
Passed:
git diff --checkNot run:
Door boundary
Product data now uses
commonplace-apiHTTP GraphQL. Harness MCP remains available for agent control, planning, coordination, and memory, but is not a Models or OKF data fallback.Summary by CodeRabbit
New Features
Bug Fixes
Tests