♻️ refactor: Native Prompts Behind a Shared Service and Adapter - #16506
AtefBellaaj wants to merge 2 commits into
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baedb6a7f0
ℹ️ 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".
| logger.error('[resolvePromptGroup] Error reading prompt group', error); | ||
| return null; |
There was a problem hiding this comment.
Propagate database failures from access resolution
When the prompt-group query throws for an ordinary request, this catch converts the operational failure to null; canAccessResource then interprets it as a missing resource and returns 404. The analogous prompt resolver below does the same, so a transient database outage makes existing prompts appear deleted instead of producing the middleware's 500 response. Let query failures propagate and reserve null for an actual absence.
AGENTS.md reference: AGENTS.md:L108-L120
Useful? React with 👍 / 👎.
| revision != null && | ||
| revision._id != null && | ||
| revision.groupId === groupId && | ||
| revision.type != null && |
There was a problem hiding this comment.
Resolve prompt revisions that lack a type
When a group was created with the still-supported initial payload { prompt: '...' } without type, or an existing stored revision predates that field, this predicate rejects both the populated revision and the subsequent database read. Consequently resolvePrompt reports the production or exact selection as unavailable even though the revision exists. Normalize such revisions to a compatible default or migrate/validate them without breaking existing stored prompts.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bc53813fe39859fcf6e13124f9adcb672bcb48d8 and verified on f51d81f29094614f4c704b88b8355127598d4b89. Missing revision types resolve as text; the real-database native adapter tests cover Production and exact selection.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
91a0207 to
5181378
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5181378642
ℹ️ 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".
| let promptService; | ||
|
|
||
| function getPromptService() { | ||
| promptService ??= createPromptServiceFromDb({ db, grantPermission }); |
There was a problem hiding this comment.
Keep the prompt service lifecycle out of legacy CJS
The ??= branch makes legacy /api own a process-wide lazy service lifecycle, so alternate prompt sources, isolated app instances, or dependency resets must now work around this module cache instead of receiving an injected service. Keep this CJS layer to requires and calls into the TypeScript module, and have the application composition pass the constructed service to its consumers.
AGENTS.md reference: AGENTS.md:L77-L81
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bc53813fe39859fcf6e13124f9adcb672bcb48d8 and verified on f51d81f29094614f4c704b88b8355127598d4b89. The lazy CJS service cache is gone. The router constructs the TypeScript service with its database and permission dependencies and passes it into the handlers. Prompt service and route suites pass.
5181378 to
a9098c2
Compare
a9098c2 to
bc53813
Compare
|
@codex review again |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Head |
Summary
The prompt route handlers in
api/server/routes/prompts.jshold the selection, validation, content-protection and mutation rules for native prompts and call the database methods directly, so a second prompt source would have to copy all of them.This PR moves that behavior into
packages/api/src/prompts. A shared prompt service owns validation, content protection, catalog projection, Usage and the creator ownership grant. A native source adapter owns Production and exact-revision selection over the data-schemas methods, with plain string IDs. HTTP handlers map service results and stage-tagged database errors to each route's existing status and body./apikeeps only the router, middleware and wiring. The access middleware passes the record it loaded to the handler, so the handler does not read it again.The data-schemas prompt methods now throw on database failure instead of returning
{ message }, and takenameandcategoryfor listing instead of a Mongo filter. The service takes one source adapter, and every prompt group uses it.How it works
Database failures are wrapped in
PromptStoreErrorwith areadorwritestage. The handler uses the stage to keep the existing response, for example 200{ message: 'Error saving prompt' }when a revision save fails. A source that cannot perform a mutation leaves it out, and the service returns anunsupportedresult.The access resolvers return
nullfor a malformed ID or a missing record, which gives the access check's 404. A read failure propagates, which gives the access check's 500.Type of change
Testing
groupId, and one read per loaded-record route.npx tsc --noEmitinpackages/apiandpackages/data-schemas.518137864: packages/apisrc/prompts100 passed; data-schemassrc/methods/prompt46 passed; api prompt routes, controllers and access middleware 1625 passed, 1 skipped.npm run static-checks -- --against origin/devpassed.Risk / compatibility
Each route keeps its status codes and bodies, except:
GET /api/promptswithoutgroupIdreturns 400{ error: 'Invalid or missing groupId' }. It no longer returns the caller's own prompts, or all prompts forREAD_PROMPTScallers.POST /api/promptsvalidates the first revision like a new revision: a missing or unknowntypereturns 400{ error: 'Prompt type must be "text" or "chat"' }. Before, the group was created and the revision was stored without a type. The client form always sends a type.Stored revisions without a type stay readable; selection treats them as
text, the same default the client uses. No schema, stored data or configuration changes.Checklist
🤖 Generated with Claude Code