fix(core): a field belongs to the plugin that introduced it by borisno2 · Pull Request #1776 · OpenSaasAU/stack

borisno2 · 2026-10-07T11:28:35Z

Summary

The plugin engine records which plugin introduced each field (addList/extendList); app-declared fields belong to the app.
extendList redeclaring a field owned by a different plugin now throws, naming both plugins, the list and the field.
Redeclaring an app-declared or own field still replaces per key (RAG's passes unaffected).
ADR-0077 records the decision (and why deep-merging access was rejected); CLAUDE.md "Deep Merging" line corrected; patch changeset added.

Test plan

New plugin-engine tests: cross-plugin via extendList, via addList, own field, app field, new field on plugin-created list
@opensaas/stack-rag suite passes
pnpm lint / format

Closes #1676

🤖 Generated with Claude Code

https://claude.ai/code/session_01Cmjr3Cq9wqd6xok31R15hJ

Generated by Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cmjr3Cq9wqd6xok31R15hJ

vercel · 2026-10-07T11:28:37Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
stack-docs	Ready	Preview	Oct 7, 2026 11:32am UTC

changeset-bot · 2026-10-07T11:28:44Z

🦋 Changeset detected

Latest commit: 04ae3e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@opensaas/stack-core	Patch
@opensaas/stack-auth	Patch
@opensaas/stack-cli	Patch
@opensaas/stack-rag	Patch
@opensaas/stack-storage	Patch
@opensaas/stack-tiptap	Patch
@opensaas/stack-ui	Patch
@opensaas/stack-storage-s3	Patch
@opensaas/stack-storage-vercel	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

chatgpt-codex-connector · 2026-10-07T11:28:44Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-07T11:30:57.704479Z	`3792fe3`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

borisno2 · 2026-10-07T11:30:16Z

Code review for PR #1776 (issue #1676, plugin engine field-ownership guard)

Static review of the diff. I did not run the test suite. The diff touches more than plugin-engine.ts, so the findings below cover all changed files. They are ordered roughly by relevance to this PR.

Directly related to the field-ownership guard

packages/core/src/config/plugin-engine.ts:218: application-declared fields are not tracked as owners.
- extendList merges with { ...existing.fields, ...extension.fields }. If the application declares title and plugin A extends it with fields: { title }, the application's access and hooks are silently overwritten.
- claimFields then records A as the owner, so a later plugin B redeclaring title throws.
- The silent-discard problem the guard targets therefore still exists for application-to-plugin collisions. Only plugin-to-plugin collisions are caught.
- Suggest seeding ownership with the application's own fields, or deciding explicitly that plugins may override them and documenting that.
packages/core/src/access/field-access.ts:412: callerSupplied default is inverted when inputData is undefined.
- The new loop treats every key as caller-supplied when args.inputData is undefined. A caller of filterWritableFields that does not pass inputData (an internal or plugin path) would have hook-set write: 'hooks' fields denied.
- That contradicts the new rule that hook output is trusted. It needs an explicit test for internal callers.
packages/ui/src/lib/serializeFieldConfig.ts:119: hook-only fields serialize as readOnly, but required validation and the create payload may still include them.
- A write: 'hooks' field with isRequired could block the create form. Alternatively the form could submit its value and get a field-level access denial from the server.
- I did not confirm this in useItemForm, so confidence is low.

Other findings in the diff

packages/auth/src/mcp/better-auth.ts:245 (medium confidence): createOAuthProtectedResourceHandler forwards to the bare /.well-known/oauth-protected-resource path with no auth base path. The authorization-server handler uses basePath, so this likely 404s inside Better Auth's router and MCP clients get no RFC 9728 metadata.
packages/core/src/secured/read.ts:1269: writtenRowQueryable adds an extra scoped first() round trip to every non-sudo create, update and delete on lists with filter-based query access. A non-string, non-number id (for example bigint) returns false, so the whole row is stripped to system fields.
packages/core/src/context/transaction-boundary.ts:228: reportAfterTransactionFailures awaits the user's onAfterTransactionError callback serially on the write's resolution path. A slow or hanging callback delays a write that has already committed. It should be fire-and-forget with its own catch.
packages/core/src/lib/client-safe-error.ts:17: The client-safe allowlist is closed. InvalidFieldAccessResultError, TransactionRolledBackError, WriteMatchedNothingError and plain hook-thrown Errors now show as a generic internal error in admin forms. Any new engine error class has to be added by hand.
packages/auth/src/server/index.ts:710: The session fill-in adds an uncached database read on every getSessionFromAuth call for fields better-auth's session lacks. It selects by field name rather than column, so a db.map-renamed column would be dropped silently.
packages/auth/src/server/session-fill-in.ts:38: assertSessionFieldsResolvable runs full getAuthTables derivation twice, once via each helper. Startup-only and minor; a single call returning both tables would remove the duplication.
packages/core/src/context/index.ts:606: clientFailure logs non-safe errors in full but gives the client no correlation id to match against the server log.

Generated by Claude Code

borisno2 · 2026-10-07T11:30:32Z

Triage of the review above: the real diff against main is 5 files (plugin-engine + test, ADR-0077, CLAUDE.md, changeset). Every finding outside plugin-engine.ts (field-access, serializeFieldConfig, better-auth, read.ts, etc.) comes from the reviewer diffing against a stale base and isn't part of this PR. The one on-topic finding, plugins silently replacing app-declared fields, is intentional: the issue's acceptance criteria require that to keep working (e.g. RAG defaulting dimensions on an app embedding()). No code changes needed.

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3792fe325c

ℹ️ 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".

chatgpt-codex-connector · 2026-10-07T11:30:55Z

          ...extension.fields,
        }

+        claimFields(name, Object.keys(extension.fields ?? {}), plugin.name)


Keep app-declared fields unclaimed

When two plugins both refine a field originally declared by the application, this call assigns the field to the first plugin even though that plugin did not introduce it. The second plugin then hits the cross-plugin ownership check and throws, contradicting the documented behavior that app-declared fields remain app-owned and may be redeclared. Only claim extension keys that were not already present in existing.fields, or explicitly track application ownership.

Useful? React with 👍 / 👎.

Good catch. extendList now claims only keys not already on the list, so app-declared fields stay app-owned. Added a regression test with two successive plugin refinements of an app field.

Generated by Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cmjr3Cq9wqd6xok31R15hJ

github-actions · 2026-10-07T11:53:30Z