feat: add System Prompts library for Expand Prompt button - #9152
Conversation
- Add system_prompts SQLite table (migration 32) seeded with 6 curated default prompts adapted from FLUX.2, HunyuanImage 3.0, Qwen-Image, Z-Image and HiDream - Add CRUD service layer + REST router at /api/v1/system_prompts - Add RTK Query endpoints, management modal (list/create/edit/delete) and a system-prompt picker in the Expand Prompt popover - Persist last picked system prompt + text-LLM model via Redux
- Migration 32 now adds user_id + is_public columns and seeds the 6 default prompts as user_id='system', is_public=TRUE; - Storage layer gains optional user_id scoping on get_many/update/delete, and create requires user_id + is_public - Router uses CurrentUserOrDefault: list scopes to own+public, GET returns 403 for foreign private prompts, PATCH/DELETE require owner or admin - Frontend adds useCanEditSystemPrompt hook, hides edit/delete on prompts the user does not own, shows System/Shared badges in the list, and exposes a 'Share with everyone' toggle in the form when multiuser is on
- Critical: migration_32.py had a 7-space indent on the second cursor.execute(), raising IndentationError on import and blocking server startup. Re-indent and restore the ALTER TABLE backfill block lost in the previous edit. - Medium: drop the heavy import of invokeai.backend.text_llm_pipeline from the migration (which would pull torch+transformers into the migrator import path). Inline DEFAULT_SYSTEM_PROMPT verbatim and rename the seeded row to "Default"; the value still mirrors text_llm_pipeline.DEFAULT_SYSTEM_PROMPT. - Medium: add 7 storage-layer tests covering own/public/admin scoping and the no-mutate guarantees on non-owner update/delete, plus 9 router tests with JWT auth covering 401/403/404 paths, owner is_public flip, and admin override. - Conftest and the existing workflows-multiuser test fixtures now wire a real SqliteSystemPromptRecordsStorage so InvocationServices construction succeeds with the new required parameter.
…zation fixture The new required InvocationServices parameter broke 122 unrelated tests in tests/app/routers/test_multiuser_authorization.py because that file builds its own InvocationServices. Add SqliteSystemPromptRecordsStorage to its fixture the same way the workflows-multiuser fixture and the global conftest were updated in the previous commit.
…ow node Adds a sibling node to TextLLMInvocation that takes a SystemPromptField (a DB-backed preset reference) instead of a free-text system prompt. Selecting a preset in the workflow editor pulls its content from the System Prompts library at run time. The original TextLLMInvocation is unchanged, so users keep the free-text option and can pick the appropriate node per workflow. - New SystemPromptField primitive in app/invocations/fields.py - Shared _run_text_llm helper extracted from TextLLMInvocation; both nodes use it - Frontend wires SystemPromptField as a new stateful field type analogous to StylePresetField (zod schemas, type guards, builders, slice action, color, Combobox renderer backed by useListSystemPromptsQuery) - Pytest covers both behaviours: DB lookup happens with the configured id and forwards the resolved content; SystemPromptNotFoundError short-circuits the pipeline call so the LLM is not invoked
…ests Main's image-move and workflow-call tests construct InvocationServices directly and predate the required system_prompt_records service, so they failed after the merge. Add the argument at the three construction sites.
…-library # Conflicts: # invokeai/frontend/web/openapi.json # invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/fields/InputFieldRenderer.tsx # invokeai/frontend/web/src/features/nodes/store/nodesSlice.ts # invokeai/frontend/web/src/features/nodes/types/field.ts # invokeai/frontend/web/src/features/nodes/util/schema/buildFieldInputTemplate.ts # invokeai/frontend/web/src/services/api/schema.ts # tests/app/routers/test_session_queue_workflow_call.py
… fixture The mock_services() fixture in test_system_prompts_multiuser.py predates the video generation merge, which added five required InvocationServices args (videos, video_files, video_records, board_video_records, gallery). All nine tests in the file errored at setup, failing every python-tests CI job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lstein
left a comment
There was a problem hiding this comment.
Nice feature, and the multi-user model is thought through — the router scoping is careful and the seeded prompts are a genuinely useful addition. A few things need to change before this can go in.
Verification I ran: full pytest suite, pnpm lint:tsc, plus scripted reproductions of items 2, 3 and 4 below.
1. CI blocker — test_system_prompts_multiuser.py couldn't construct InvocationServices (fixed, pushed as b0a4d36)
All six python-tests jobs were red. The new test file's mock_services() fixture predates the video-generation merge (eb9a951248, pulled into this branch at 8dd8c1e5f1):
TypeError: InvocationServices.__init__() missing 5 required positional arguments:
'videos', 'video_files', 'video_records', 'board_video_records', and 'gallery'
All 9 tests in the file errored at setup. I've pushed the fix directly to your branch since it was blocking everything else:
client_state_persistence=ClientStatePersistenceSqlite(db=db),
users=UserService(db),
external_generation=None, # type: ignore
+ videos=None, # type: ignore
+ video_files=None, # type: ignore
+ video_records=None, # type: ignore
+ board_video_records=None, # type: ignore
+ gallery=None, # type: ignore
)None is safe for all five here — Invoker._start() skips them since getattr(None, "start") isn't callable. With that applied: 9 passed in this file and 3314 passed, 113 skipped for the whole suite, so this fixture was the sole cause of the CI failure.
Worth calling out: because this file is new in the PR, those 9 tests have never actually executed in CI. They pass once unblocked, which is good news — the multi-user REST scoping is genuinely verified. But it does mean the two findings below sit in exactly the paths that coverage doesn't reach.
2. TextLLMWithPresetInvocation bypasses the ownership model
invokeai/app/invocations/text_llm.py:122 resolves the preset with no permission check:
record = context._services.system_prompt_records.get(self.system_prompt.system_prompt_id)Every REST endpoint enforces own-or-public-or-admin, and your new tests prove it end to end. The node doesn't. Sequence in multi-user mode: user1 POSTs a private prompt and the response echoes its id; user2 gets 403 on GET /i/{id} (that's your own test_user2_cannot_get_user1_private_prompt), but can still enqueue a text_llm_with_preset graph carrying that id. The content becomes the LLM's system message and is recoverable from the output ("repeat your instructions verbatim").
The pattern to copy already exists from the same multi-user work — call_saved_workflow.py:56-70 reads context._data.queue_item.user_id, looks the user up, and permits only default/owner/public/admin. (context._services access is itself fine; prompt_template.py:44 does the same for style presets.)
It's bounded — an attacker needs the victim's UUID — but it's the one place the PR's stated security model isn't enforced, and the only node-level path with no test.
3. The migration's ALTER TABLE backfill can never run, and leaves those dev DBs permanently broken
migration_2026_07_09_create_system_prompts.py runs PRAGMA table_info + ALTER TABLE … ADD COLUMN for "dev databases that already ran an earlier revision of this migration," and the Merge Plan calls this "safe and idempotent." It's unreachable: SqliteMigrator records applied migrations by stable id in applied_migrations (sqlite_migrator_impl.py:110-113) and skips ids already present, so a DB that ran the earlier revision never re-enters the callback.
I replayed exactly that scenario — apply the full chain, drop the two columns while leaving the applied_migrations row in place, restart:
applied_migrations row: 2026_07_09_create_system_prompts
columns after 'upgrade': ['id', 'name', 'content', 'created_at', 'updated_at']
get_many: OperationalError: no such column: user_id
create: OperationalError: table system_prompts has no column named user_id
So anyone who tested an earlier revision of this branch gets a hard failure on every list and create, with no self-repair. Either bump to a new migration id so the ALTERs actually execute, or drop the dead code and note in the merge plan that affected devs must DROP TABLE system_prompts and delete the applied_migrations row.
4. DELETE of a nonexistent id returns 200 in single-user mode
The existence check in delete_system_prompt sits inside if config.multiuser:, so single-user installs skip it; user_id is then None and SqliteSystemPromptRecordsStorage.delete never inspects cursor.rowcount. Against the real router in single-user mode:
DELETE nonexistent (single-user) -> 200 'null'
GET nonexistent (single-user) -> 404
POST -> 200, is_public=True
DELETE real -> 200; DELETE again -> 200
GET 404s on the same id DELETE reports success for, and deleting a row twice succeeds twice. QA step 2 in the description asserts delete-then-404 semantics, so this contradicts the PR's own contract. The multi-user path is correct (pre-check → 403/404) and your new tests cover it; single-user is the untested gap.
Related asymmetry worth fixing at the same time: update() raises SystemPromptNotFoundError when scoped to a non-owner, but delete() returns silently — and test_delete_with_non_owner_user_id_is_noop locks that in. The router's pre-check masks it today, but it's a trap for the next caller. Having delete() check rowcount and raise would fix both this and the 200-above.
5. Stale manual type augmentation in services/api/endpoints/systemPrompts.ts:5-16
The NOTE says schema.ts hasn't been regenerated yet and the intersections should go once it lands. It has been regenerated in this PR — SystemPromptRecordDTO carries user_id/is_public (schema.ts:31534, 31539) and SystemPromptChanges carries is_public (31500). The intersections are no-ops and the comment now tells the next reader something untrue. Please delete both, per your own instruction in the comment.
Nits (non-blocking)
systemPromptsis inserted beforestylePresetsinen.json, but sorts after it alphabetically.idx_system_prompts_is_publicon a boolean column of a tiny table doesn't buy anything.SystemPromptFieldembeds a bare UUID, so an exported workflow referencing a user-created prompt breaks on another install. The seeded defaults have fixed UUIDs and port fine, andStylePresetFieldhas the identical limitation — so this is consistent with precedent, but a line in the node docstring would save someone a confusing bug report.
Things I attacked that held up
Non-owner and non-admin PATCH/DELETE against private, public and system-owned prompts; get_many scoping for owner/non-owner/admin; extra="forbid" blocking user_id/is_public injection through the create body; the scope_clause f-string (constant, not user input); the self-referential updated_at trigger (matches the migration_1 convention, and recursive_triggers is off); the expandPrompt slice migrate and persistence (matches gallerySlice); the delete/list Redux reducers versus the auto-select effect (transiently reselects a deleted id mid-refetch, but converges); useCanEditSystemPrompt field names against authSlice's zUser (user_id/is_admin — correct); and the stateful-field-type registration, which has full parity with StylePresetField across every touchpoint.
- TextLLMWithPresetInvocation now enforces the same access rules as the REST API before resolving a preset. The record store is unscoped, so a user could previously read another user's private prompt by enqueueing a graph that references its id -- the content becomes the LLM's system message and is recoverable from the output. Mirrors call_saved_workflow's ownership check. - Bump the migration id (and module name) to 2026_07_10_create_system_prompts. The migrator runs each id once, so the ADD COLUMN backfill for dev databases created from an earlier revision of this branch was unreachable; those DBs hit "no such column: user_id" on every list and create with no self-repair. The earlier id was never released, so a new id is free. Also corrects the module docstring: INSERT OR IGNORE is not what keeps deleted defaults deleted. - delete() raises SystemPromptNotFoundError when nothing was deleted, and the router maps that to 404. Single-user installs skipped the existence check, so DELETE reported 200 for ids GET 404s on and deleting a row twice succeeded twice -- contradicting the PR's own QA contract. This also removes the update()/delete() asymmetry. - Drop the stale manual type augmentation in endpoints/systemPrompts.ts; schema.ts already carries user_id/is_public. Nits: move systemPrompts after stylePresets in en.json; drop the boolean index idx_system_prompts_is_public; document the SystemPromptField id-portability limitation in the node docstring. Tests: node-level permission tests for the escalation path and the allowed cases; single-user router tests for the delete contract; migration tests for the backfill, idempotency and id/module-name consistency.
lstein
left a comment
There was a problem hiding this comment.
Thanks — I reviewed 2dc4432534 plus the two main merges on top of it. All four findings from the last round are genuinely addressed, and CI is green across all 16 checks. I re-ran the system-prompt tests locally (65 passed) and pnpm lint:tsc (clean).
Two things in the fixes themselves need another pass, one of them substantive.
What holds up
Node ownership check — _resolve_system_prompt mirrors call_saved_workflow and runs before _run_text_llm; the tests cover the escalation path and all four allowed cases.
Migration id bump — the loader enforces id-matches-module-name (migration_loader.py:104-109), nothing else references the old id, and every statement in the callback is IF NOT EXISTS / INSERT OR IGNORE, so re-running on an already-migrated dev DB is a genuine no-op rather than a "trigger already exists" failure.
Delete contract — rowcount == 0 → raise → 404. transaction() rolls back and re-raises, so no lock or connection leak on the raising path, and the router is the only caller.
Type intersections — gone, tsc clean, and the en.json move is key-identical.
1. The node's is_default bypass reopens the hole it was meant to close
text_llm.py:150 treats record.user_id == "system" as "seeded default, visible to everyone". But "system" is also the synthetic user id handed to every request in single-user mode (auth_dependencies.py:106) — so every prompt ever created on a single-user install is indistinguishable from a seeded default to this check.
Verified end to end against the real router, real storage and the real node:
single-user create -> user_id=system, is_public=True
[admin enables multiuser, PATCHes is_public=False]
bob GET /i/{id} -> 403 (and absent from his list)
node resolved prompt -> "SECRET" (handed to bob)
Single-user → multiuser is the normal upgrade path, so any prompt created before the flag was flipped is permanently un-privatizable at the node layer — which is the disclosure this fix set out to block.
is_default is also the only clause where the node's rule differs from get_system_prompt's. The seeded defaults are is_public=TRUE, so record.is_public already covers them — dropping is_default makes the node and the REST layer agree by construction. (call_saved_workflow can afford the clause because workflow "default" is a real category, not an overloaded owner id.)
2. The now-reachable backfill leaves the seeded defaults private
The point of the id bump was to make the ALTER TABLE path run. It runs now — and ADD COLUMN is_public ... DEFAULT FALSE applies to the pre-existing seed rows, after which INSERT OR IGNORE skips them on primary-key conflict. Replaying it on the exact pre-multiuser table:
('0f8f5b2e-…0000', 'Default', 'system', 0)
… all 7 defaults is_public=0 …
get_many(user_id='alice') -> []
An empty system-prompts list for every non-admin in multiuser mode. test_backfills_multiuser_columns_on_a_pre_multiuser_table asserts ("system", 0, "body") for the user row and never checks the defaults, so it bakes the behaviour in. One statement in the backfill branch fixes it:
cursor.execute(
f"UPDATE system_prompts SET is_public = TRUE WHERE id IN ({','.join('?' * len(DEFAULT_SYSTEM_PROMPTS))});",
[default_id for default_id, _, _ in DEFAULT_SYSTEM_PROMPTS],
)These two interact — fix the backfill first, and then dropping is_default is safe on those databases too.
Nit
Bumping the id means dev DBs already on 2026_07_09 re-run the seed, so any default they had deleted comes back once. Dev-only, but worth a line in the merge plan.
Things I attacked that held up
Delete of a queued prompt mid-graph (clean ValueError, no 500); concurrent double-delete from two tabs (404, not 500); non-admin deleting a seeded default or another user's public prompt (403 via the pre-check); admin unscoped PATCH/DELETE; rowcount semantics across the scoped and unscoped branches; the rollback path on the raising delete; the leftover idx_system_prompts_is_public on dev DBs (cosmetic only, no DROP INDEX needed); the frontend delete now surfacing an error toast where it previously reported success for an already-deleted row (correct); and queue_item.user_id being None, which behaves exactly as call_saved_workflow does.
…visibility Seed the Krea 2 prompt-expansion system message (krea-ai/krea-2, docs/expansion.txt) as an eighth default, verbatim from upstream. Also address review feedback on invoke-ai#9152: Drop the `is_default` clause from TextLLMWithPresetInvocation's ownership check. SYSTEM_PROMPT_DEFAULT_USER_ID ("system") is not only the seeded defaults' owner but also the synthetic user id every request carries in single-user mode, so the clause made every prompt created before an install switched to multiuser readable by anyone via a graph, while GET /system_prompts/i/{id} correctly 403s on it. The seeded defaults are is_public=TRUE, so is_public already covers them and the node's rule is now identical to the router's by construction. Re-share the seeded defaults in the multiuser backfill. ADD COLUMN stamps is_public=FALSE onto pre-existing rows and the seed is INSERT OR IGNORE, so the defaults stayed private and get_many (own OR public) returned an empty list for every non-admin. Scoped to the seeded ids and to the backfill branch so a default a user deliberately made private is never re-shared. Correct the SYSTEM_PROMPT_DEFAULT_USER_ID docstring and the user_id field description, which described the id as meaning "built-in default" - the reading the bypass was built on. Tests: the parametrized node case asserted a private "system"-owned prompt was allowed; corrected and paired with a regression test for the single-user -> multiuser upgrade path. The backfill test now seeds the defaults first and asserts they end up public, plus a test that a re-run leaves privatization alone.
lstein
left a comment
There was a problem hiding this comment.
Both round-2 findings are fixed correctly in 375733a43f. I re-ran the original reproductions against the new code rather than reading the diff alone. LGTM.
is_default dropped from the node
The rule is now is_owner or record.is_public or is_admin — the same predicate as get_system_prompt, so the two agree by construction rather than in parallel. Replaying the escalation from the last round against the real router, storage and node:
node rejected the privatized single-user-era prompt (was: handed over "SECRET")
bob sees 8 prompts (seeded defaults still visible)
node resolved the seeded default for bob (defaults still usable)
owner path still works
Good catch on the parametrized case that had been asserting ("system", False, …) was allowed — that was the bug encoded as a test, and correcting it plus adding the upgrade-path regression is exactly right.
Backfill re-shares the defaults
The UPDATE sits inside the "is_public" not in existing_columns branch, so it only fires on the one-time column add. Replaying the pre-multiuser table:
all 8 defaults -> is_public=1
pre-existing user row -> is_public=0 (correctly left private)
get_many(user_id=…) -> all 8 defaults (was: [])
Scoping it to the branch and to the seeded ids is the right call, and test_backfill_does_not_reshare_a_default_the_user_made_private pins it.
The docstring corrections matter more than they look — the old "owner of the seeded default prompts" wording is what made the bypass read as reasonable, and the explicit "do NOT use this as an is-a-built-in-default test" kills the misreading at the source.
Attacked without success
Fresh DB → CREATE TABLE carries both columns → the UPDATE is skipped → the seed inserts public anyway; a user_id-missing-but-is_public-present table can't exist (both columns were always added together); the f-string builds a constant count of ? with bound parameters; the new …0007 id is in the UPDATE list but absent from a pre-upgrade DB, so it no-ops there and INSERT OR IGNORE adds it public; single-user mode still skips the check entirely; owner, public, admin and seeded-default paths all verified live. 16/16 CI green, 72 tests locally, tsc clean.
Non-blockers
- The Krea 2 prompt was added without bumping the migration id, so any dev DB already on
2026_07_10won't show the eighth prompt. Harmless at release — everyone coming through main runs the migration fresh — but worth a line in the QA steps, or it will look like a missing feature to whoever tests this on an existing branch database. - I checked
krea-ai/krea-2docs/expansion.txt: it exists and all nine rules match the seeded text in the same order. I couldn't get a character-level diff through my fetch tooling, so I'm confirming structural agreement rather than certifying it verbatim. SYSTEM_PROMPT_DEFAULT_USER_IDis now referenced only from a docstring. Keeping it as a labelled trap is defensible — just noting it is no longer used in code.
Nice work on this one — the multi-user model ended up genuinely tight, and the tests now cover the paths that had no coverage at all three rounds ago.
Upstream's invoke-ai#9152 added a System Prompts library to the legacy web app; webv2 called /utilities/expand-prompt with no system prompt at all, so the feature existed on the backend and in one frontend only. The popover now carries a picker over the prompts the account can see, and behind a manage toggle the list that creates, edits and deletes them. Structure follows the prompt-templates feature next door: a pure core module for ownership and selection, a data module for transport, and a catalog hook. Two things are deliberately not copies of upstream: - Ownership is decided by is_public and the owner id, never by comparing against the user id "system". That id also belongs to every prompt created in single-user mode, so treating it as built-in would make an install that later enabled multiuser show its own prompts as read-only. - The selection is resolved on read rather than stored back. The id outlives the record — another tab can delete it, and a shared prompt disappears when its owner unshares it — so a stale id falls back to the first visible prompt, which is also what an unselected picker uses. The four new modules join the prompt-templates UI in the eager editor chunk, which is what the source-owner gate flagged; the browser baseline is re-recorded for +682 bytes (+0.04%) with no owners removed.
Summary
system_promptsSQLite table (migration 32) seeded with 7 curated default prompts: anInvokeAI Default(mirrors the hardcoded fallback intext_llm_pipeline.DEFAULT_SYSTEM_PROMPT) plus 6 prompts adapted from FLUX.2, HunyuanImage 3.0, Qwen-Image (edit + 2512), Z-Image and HiDream/api/v1/system_prompts, with per-user ownership and optional sharing (modeled on the workflow library):user_id+is_publiccolumns,CurrentUserOrDefaultdependency on every endpoint, owner-or-admin checks for PATCH/DELETE, and own+public scoping for LIST in multi-user modeexpandPromptslice)useCanEditSystemPrompthook hides edit/delete on prompts the user does not own, list items show System / Shared badges, and the form exposes a Share with everyone toggle when multi-user is enabledDB-backed preset reference) instead of a free-text system prompt.
Related Issues / Discussions
Closes #9127
QA Instructions
Backend — single-user (default)
uv run --extra cuda invokeai-web— migration 32 runs, creates thesystem_promptstable (withuser_id+is_publiccolumns) and seeds 7 default prompts asuser_id='system',is_public=TRUE.GET /api/v1/system_prompts/→ 7 recordsPOST /api/v1/system_prompts/with{"name":"Test","content":"foo"}→ 200; record echoed back withis_public=true(single-user default)PATCH /api/v1/system_prompts/i/{id}with{"name":"Test2"}→ record updatedDELETE /api/v1/system_prompts/i/{id}→ record gone; subsequentGETreturns 404INSERT OR IGNORE).Backend — multi-user
4. Run with multi-user enabled and create two non-admin users (alice, bob) plus an admin.
5. As alice:
POSTa private prompt → it is owned by alice,is_public=false.6. As bob:
GET /api/v1/system_prompts/→ sees the 7 system defaults plus only public prompts; does not see alice's private prompt.7. As bob:
PATCHorDELETEof alice's prompt id → 403 (or 404 if he does not even know the id, depending on path).8. As alice:
PATCH … {"is_public": true}→ flips her prompt to shared; bob's nextGETnow includes it.9. As admin:
GETreturns all prompts (including alice/bob's privates), and PATCH/DELETE on any prompt succeeds.Frontend
10. Install at least one Text-LLM model (e.g. via the Model Manager starter list).
11. In the Generate tab, click the Expand Prompt button (sparkle icon) over a non-empty prompt.
12. The popover now shows a System Prompt combobox above the model picker — pick one of the seeded defaults, pick a model, click Expand. The positive prompt is replaced;
Ctrl+Zrestores the original.13. Click the pencil icon next to the system-prompt combobox → management modal opens.
- Single-user: Create / Edit / Delete buttons appear on every row.
- Multi-user (non-admin): Edit / Delete only appear on prompts you own; system/shared prompts show a badge instead.
- Multi-user: opening the form for an owned prompt shows the Share with everyone checkbox.
14. Reload the browser → the previously picked system prompt and model are still selected (Redux persistence).
15. Lint + test suite:
pnpm lint && pnpm test:no-watchfrominvokeai/frontend/web/(1 pre-existing flaky timer test innavigation-api.test.tsis unrelated).Merge Plan
This PR adds a new SQLite migration (32) and a new persisted Redux slice. Notes for reviewers / mergers:
system_promptstable withuser_id+is_publicalready in the schema and seeds 7 default rows owned bysystem(public). For dev databases that already ran an earlier revision of this migration without the multi-user columns, the migration also runsPRAGMA table_infoandALTER TABLE … ADD COLUMNto backfill the columns — safe and idempotent.expandPromptslice has amigratestep (sets_version: 1if missing) — an old persisted state without this slice simply starts from initial state, no breaking change for users.Checklist
What's Newcopy (if doing a release after this PR)