feat: add typed prompt relationships#174
Open
legeling wants to merge 2 commits into
Open
Conversation
Code Review by Qodo
1. Relation IPC lacks validation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough新增 ChangesPrompt 图关系功能全链路
Sequence Diagram(s)sequenceDiagram
participant User
participant PromptListView
participant usePromptStore
participant database.ts
participant promptApi as preload promptApi
participant Main as prompt.ipc.ts
participant PromptRelationDB
User->>PromptListView: 拖拽 Prompt B 到 Prompt A 中心区域
PromptListView->>PromptListView: 设置 pendingRelationDrop + 计算菜单坐标
User->>PromptListView: 点击菜单项 (e.g. related_to)
PromptListView->>usePromptStore: createRelation(dto)
usePromptStore->>database.ts: createPromptRelation(dto)
database.ts->>promptApi: createRelation(dto)
promptApi->>Main: ipcRenderer.invoke(PROMPT_RELATION_CREATE)
Main->>PromptRelationDB: create(dto)
PromptRelationDB-->>Main: PromptRelation
Main->>Main: syncWorkspace()
Main-->>promptApi: PromptRelation
promptApi-->>database.ts: PromptRelation
database.ts-->>usePromptStore: PromptRelation
usePromptStore->>usePromptStore: 更新 relations 状态
usePromptStore-->>PromptListView: relations 更新，渲染徽标
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
PR Summary by QodoAdd typed prompt relationships with SQLite persistence, UI chooser, and backup support WalkthroughsDescription• Persist non-tree prompt relationships in new SQLite prompt_relations table. • Add drag-and-drop relationship chooser and render relation badges in the prompt list. • Export/import prompt relations in desktop backups with strict validation and restore support. Diagramgraph TD
UI["Prompt list UI"] --> Store["Prompt store"] --> RDB["Renderer DB API"] --> Preload["Preload prompt API"] --> IPC["Main IPC"] --> RelDB["PromptRelationDB"] --> SQLite[("SQLite")]
Backup["Backup service"] --> RDB
High-Level AssessmentThe following are alternative approaches to this PR: 1. Unify all relationship kinds into prompt_relations (including grouped_under)
2. Store relations as JSON metadata on prompts
3. Make relation creation purely UI-side until a dedicated editor exists
Recommendation: The PR’s approach is the best tradeoff for V1: keep grouped_under on prompts.parent_id for fast tree operations, and introduce a dedicated prompt_relations table for typed graph edges with DB-enforced integrity. The canonicalization/idempotent-create behavior for related_to is a pragmatic UX/data-quality choice and keeps future graph exploration feasible without destabilizing the existing tree model. File ChangesEnhancement (23)
Tests (5)
Documentation (5)
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+284
to
+308
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_CREATE, async (_, data: CreatePromptRelationDTO) => { | ||
| const relation = relationDb.create(data); | ||
| syncWorkspace(); | ||
| return relation; | ||
| }); | ||
|
|
||
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_LIST, async (_, query?: PromptRelationQuery) => { | ||
| return relationDb.list(query); | ||
| }); | ||
|
|
||
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_UPDATE, async (_, id: string, data: UpdatePromptRelationDTO) => { | ||
| const relation = relationDb.update(id, data); | ||
| if (relation) { | ||
| syncWorkspace(); | ||
| } | ||
| return relation; | ||
| }); | ||
|
|
||
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_DELETE, async (_, id: string) => { | ||
| const deleted = relationDb.delete(id); | ||
| if (deleted) { | ||
| syncWorkspace(); | ||
| } | ||
| return deleted; | ||
| }); |
There was a problem hiding this comment.
1. Relation ipc lacks validation 📘 Rule violation ⛨ Security
8.2
The new prompt relation IPC handlers accept unvalidated inputs and do not translate failures into structured error responses. Malformed payloads or thrown DB errors can propagate unpredictably across the main-process IPC boundary.
Agent Prompt
## Issue description
IPC handlers for prompt relations do not validate payloads/ids and do not return structured error objects on failure.
## Issue Context
Per IPC boundary requirements, handlers must reject malformed inputs and propagate structured errors without risking main-process instability.
## Fix Focus Areas
- apps/desktop/src/main/ipc/prompt.ipc.ts[284-308]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+744
to
+749
| <div | ||
| className="fixed z-50 w-44 rounded-md border border-border bg-popover p-1 shadow-lg" | ||
| style={{ | ||
| left: pendingRelationDrop.x, | ||
| top: pendingRelationDrop.y, | ||
| }} |
There was a problem hiding this comment.
2. Inline style added to menu 📘 Rule violation ⚙ Maintainability
8.6
A new JSX style={{ ... }} prop is introduced for the relation chooser menu positioning. This
violates the Tailwind-only styling requirement and adds hard-to-maintain inline styling.
Agent Prompt
## Issue description
The relation chooser menu uses an inline `style` prop for `left/top`, which is prohibited.
## Issue Context
UI styling must use Tailwind classes only; inline style props are disallowed.
## Fix Focus Areas
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[744-749]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
100
to
+107
| ? backup.exportedAt | ||
| : new Date().toISOString(), | ||
| prompts: Array.isArray(backup?.prompts) ? backup.prompts : [], | ||
| promptRelations: Array.isArray(backup?.promptRelations) | ||
| ? backup.promptRelations | ||
| : Array.isArray((backup as any)?.relations) | ||
| ? (backup as any).relations | ||
| : [], |
There was a problem hiding this comment.
3. backup as any type assertion 📘 Rule violation ⚙ Maintainability
8.1
New as any assertions are introduced when normalizing imported backups. This weakens type safety and can hide runtime shape mismatches for backup payloads.
Agent Prompt
## Issue description
`normalizeImportedBackup()` uses `(backup as any)` to access legacy fields (`promptVersions`, `relations`), violating the no-`any`/restricted assertions rule.
## Issue Context
TypeScript strictness requires avoiding `any` and unsafe assertions; prefer explicit legacy envelope types and runtime type guards.
## Fix Focus Areas
- apps/desktop/src/renderer/services/database-backup-format.ts[89-107]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+996
to
+998
| if (normalizedMessage.includes("prompt relationships require")) { | ||
| return "该备份包含 Prompt 关系数据，需要在桌面版中导入。"; | ||
| } |
There was a problem hiding this comment.
4. Chinese string added in code 📘 Rule violation ⚙ Maintainability
8.3
A new hardcoded Chinese user-facing message is added in a non-locale TypeScript file. This violates the rule forbidding Chinese characters outside locale JSON resources.
Agent Prompt
## Issue description
Hardcoded Chinese text was added in a non-locale source file.
## Issue Context
All Chinese characters must live only in locale JSON files; user-facing strings should be localized via i18n keys.
## Fix Focus Areas
- apps/desktop/src/renderer/services/database-backup.ts[996-998]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+105
to
+120
| list(query: PromptRelationQuery = {}): PromptRelation[] { | ||
| const clauses: string[] = []; | ||
| const values: string[] = []; | ||
|
|
||
| if (query.promptId) { | ||
| if (query.direction === "outgoing") { | ||
| clauses.push("source_prompt_id = ?"); | ||
| values.push(query.promptId); | ||
| } else if (query.direction === "incoming") { | ||
| clauses.push("target_prompt_id = ?"); | ||
| values.push(query.promptId); | ||
| } else { | ||
| clauses.push("(source_prompt_id = ? OR target_prompt_id = ?)"); | ||
| values.push(query.promptId, query.promptId); | ||
| } | ||
| } |
There was a problem hiding this comment.
6. Undirected query misses edges 🐞 Bug ≡ Correctness
PromptRelationDB canonicalizes related_to as an undirected edge by sorting endpoints, but
list() applies incoming/outgoing filters strictly to source_prompt_id or target_prompt_id.
As a result, list({ promptId, direction: 'outgoing'|'incoming', kind: 'related_to' }) will
silently drop relations depending on prompt id ordering.
Agent Prompt
### Issue description
`PromptRelationDB.normalizeEndpoints()` stores `related_to` relations in a canonical (sorted) orientation, but `PromptRelationDB.list()` treats `direction: 'incoming'|'outgoing'` as directional storage. This makes directional queries for `related_to` incomplete.
### Issue Context
`related_to` is intended to be undirected (canonicalized), so querying should not depend on which endpoint ended up in `source_prompt_id`.
### Fix Focus Areas
- packages/db/src/prompt-relation.ts[105-126]
- packages/db/src/prompt-relation.ts[173-186]
### Suggested fix
In `list()` when `query.kind === 'related_to'` and `query.promptId` is set, ignore `query.direction` and always add the symmetric predicate:
- `clauses.push('(source_prompt_id = ? OR target_prompt_id = ?)')`
- `values.push(query.promptId, query.promptId)`
(Optionally) if `query.kind` is not specified but `direction` is, consider whether `related_to` should appear in both incoming/outgoing sets; if yes, include it via an OR condition keyed on `kind = 'related_to'`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
spec/changes/active/desktop-prompt-relationship-tree/design.md (1)
47-52: 💤 Low value规范文档中存在连续句式重复，建议用更多样的表达方式提高可读性。 三个文件在列举功能或场景时，均出现多个连续句子以相同动词或结构开头的情况。虽然内容准确，但改善句式多样性能提升文档质量。
spec/changes/active/desktop-prompt-relationship-tree/design.md#L47-L52：第 47-52 行的四个列表项均以"Add"开头，建议用更多样的动词（如"Reject"、"Prevent"、"Create"）重述。spec/changes/active/desktop-prompt-relationship-tree/implementation.md#L15-L20：第 15-20 行的四个功能项中三个以"Added"开头，建议用不同的表达（如"Introduced"、"Rendered"、"Extended"）来改进可读性。spec/changes/active/desktop-prompt-relationship-tree/specs/prompt-relationships/spec.md#L42-L53：第 48-53 行的六个场景项中五个以"When a user"或类似结构开头，可用"Pressing"、"Deleting"、"Exporting"等简洁表述来避免重复。🤖 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 `@spec/changes/active/desktop-prompt-relationship-tree/design.md` around lines 47 - 52, Improve sentence structure variety across three specification documents to enhance readability. In spec/changes/active/desktop-prompt-relationship-tree/design.md lines 47-52, replace the repetitive "Add" verb used in multiple list items with varied verbs such as "Reject" for self-relation constraints and "Prevent" for duplicate prevention. In spec/changes/active/desktop-prompt-relationship-tree/implementation.md lines 15-20, replace the three consecutive items starting with "Added" by using alternative expressions like "Introduced" or "Extended" to describe the features. In spec/changes/active/desktop-prompt-relationship-tree/specs/prompt-relationships/spec.md lines 42-53, replace the five scenario items starting with "When a user" with more concise and varied verb-based phrases such as "Pressing", "Deleting", or "Exporting" to avoid repetitive grammatical structure while maintaining clarity.apps/desktop/src/renderer/services/database-backup.ts (1)
996-998: 💤 Low value硬编码中文字符串应使用 i18n。
新增的错误消息遵循了
formatBackupImportError函数中的现有模式，但根据编码指南，所有面向用户的文本都应通过t()进行国际化处理。建议后续统一重构此函数以支持 i18n。As per coding guidelines: "All text visible to users must go through t() from react-i18next"
🤖 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/desktop/src/renderer/services/database-backup.ts` around lines 996 - 998, The hardcoded Chinese string in the `formatBackupImportError` function violates i18n guidelines. Replace the hardcoded string "该备份包含 Prompt 关系数据，需要在桌面版中导入。" with a call to the `t()` function from react-i18next, using an appropriate translation key. Add the Chinese text as a translation entry in the i18n configuration files to support multiple languages.Source: Coding guidelines
apps/desktop/tests/integration/services/database-backup-filesystem.integration.test.ts (1)
434-440: ⚡ Quick win补充关系
note的回写断言，避免元数据静默丢失当前仅校验
sourcePromptId/targetPromptId/kind，但此用例初始数据包含note: "Shared context"。建议在恢复后同时断言note，确保关系元数据在文件系统备份链路中完整往返。🤖 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/desktop/tests/integration/services/database-backup-filesystem.integration.test.ts` around lines 434 - 440, The test assertion for promptRelations in the backup-restore verification is incomplete. It currently only validates sourcePromptId, targetPromptId, and kind fields using expect.objectContaining, but the initial test data includes a note field with value "Shared context". Add the note field to the expect.objectContaining object in the assertion to verify that the relationship metadata (including the note) is properly preserved and restored through the filesystem backup chain, preventing silent metadata loss.apps/desktop/tests/unit/services/database-backup-format.test.ts (1)
260-311: ⚡ Quick win让宽松模式关系过滤用例同时覆盖“保留有效关系”分支
这个用例只断言“全部丢弃”，无法防止实现退化为“无条件清空
promptRelations”。建议同一 payload 增加一条双端点都有效的 relation，并断言它被保留、skipped.promptRelations仅统计无效项。🤖 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/desktop/tests/unit/services/database-backup-format.test.ts` around lines 260 - 311, The current test case for the lenient parser only verifies that invalid prompt relations are dropped, but doesn't verify that valid relations are preserved, which means a buggy implementation could unconditionally empty all promptRelations without being caught. Add another prompt to the payload (such as "prompt-keep-2") that will be retained, modify one of the existing promptRelations to reference only kept prompts (for example, update the relation-keep to have targetPromptId "prompt-keep-2" that now exists), and update the assertions to expect backup.promptRelations to contain this one valid relation while skipped.promptRelations should be 1 (counting only the relation-drop that references the dropped prompt-drop).
🤖 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/desktop/src/main/ipc/prompt.ipc.ts`:
- Around line 284-308: The four relation IPC handlers (PROMPT_RELATION_CREATE,
PROMPT_RELATION_LIST, PROMPT_RELATION_UPDATE, PROMPT_RELATION_DELETE) lack input
validation and error handling. For each handler, add runtime type validation for
the input parameters (validate that id is a non-empty string, query is a valid
object if provided, and data matches the expected DTO structure), wrap the
handler logic in a try/catch block, and return a structured error response
object (instead of throwing or crashing) when validation fails or database
operations throw errors. This prevents malformed payloads from reaching the
database layer and ensures consistent error responses to the renderer process.
In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx`:
- Around line 459-486: The commitRelationDrop callback clears the UI state
(setPendingRelationDrop) before awaiting the async onCreateRelation operation,
and since the function is called with void at the click handler, any Promise
rejection from onCreateRelation will go unhandled. Wrap the await
onCreateRelation call in a try-catch block to handle potential errors (such as
duplicate relations or IPC failures), and either restore the UI state on error
or display an error message to the user so they receive feedback when the
relation creation fails.
In `@apps/desktop/src/renderer/stores/prompt.store.ts`:
- Around line 96-100: The current implementation combines the critical prompt
data fetch with the secondary relations data fetch in a single Promise.all(),
which means if the relations query fails, both prompts and relations fail to
update. Since relations are enhancement data and should not block the main
prompt list availability, separate the relations query from the main data fetch.
Keep the getAllPrompts() call as the primary data source, and handle the
listPromptRelations() query independently with error handling that allows it to
fail gracefully (either silently or with a default empty value) without
preventing the prompts from being set in the store. This way, the prompt list
will be available even if the relations query fails.
In `@apps/desktop/tests/unit/main/prompt-relation-db.test.ts`:
- Around line 31-132: The test suite for prompt relations is missing
comprehensive validation of the note field and other text fields for malicious
and complex input handling. Add new test cases that verify the
relationDb.create() and relationDb.update() methods correctly store and retrieve
the note field with high-risk inputs including XSS vectors (like
<script>alert(1)</script>), HTML entities, event handler strings, CJK and emoji
characters (including multi-codepoint sequences), RTL text, zero-width
characters, control characters (including \x00), and strings exceeding 10KB in
length. Each test should verify that values are stored and retrieved without
silent truncation, contamination, or data loss through write→read round-trip
validation.
In `@packages/db/src/prompt-relation.ts`:
- Around line 146-171: The normalizeCreateInput method does not validate or
remove null bytes from string inputs (sourcePromptId, targetPromptId, and note),
which can cause data loss when writing to SQLite. Add null byte validation or
cleaning for all string fields in the normalizeCreateInput method to either
reject inputs containing null bytes or strip them before returning.
Additionally, ensure the update method applies the same null byte handling to
maintain consistency across all database write operations, and add test cases
that verify this behavior by attempting to create or update relations with null
bytes in these fields.
---
Nitpick comments:
In `@apps/desktop/src/renderer/services/database-backup.ts`:
- Around line 996-998: The hardcoded Chinese string in the
`formatBackupImportError` function violates i18n guidelines. Replace the
hardcoded string "该备份包含 Prompt 关系数据，需要在桌面版中导入。" with a call to the `t()`
function from react-i18next, using an appropriate translation key. Add the
Chinese text as a translation entry in the i18n configuration files to support
multiple languages.
In
`@apps/desktop/tests/integration/services/database-backup-filesystem.integration.test.ts`:
- Around line 434-440: The test assertion for promptRelations in the
backup-restore verification is incomplete. It currently only validates
sourcePromptId, targetPromptId, and kind fields using expect.objectContaining,
but the initial test data includes a note field with value "Shared context". Add
the note field to the expect.objectContaining object in the assertion to verify
that the relationship metadata (including the note) is properly preserved and
restored through the filesystem backup chain, preventing silent metadata loss.
In `@apps/desktop/tests/unit/services/database-backup-format.test.ts`:
- Around line 260-311: The current test case for the lenient parser only
verifies that invalid prompt relations are dropped, but doesn't verify that
valid relations are preserved, which means a buggy implementation could
unconditionally empty all promptRelations without being caught. Add another
prompt to the payload (such as "prompt-keep-2") that will be retained, modify
one of the existing promptRelations to reference only kept prompts (for example,
update the relation-keep to have targetPromptId "prompt-keep-2" that now
exists), and update the assertions to expect backup.promptRelations to contain
this one valid relation while skipped.promptRelations should be 1 (counting only
the relation-drop that references the dropped prompt-drop).
In `@spec/changes/active/desktop-prompt-relationship-tree/design.md`:
- Around line 47-52: Improve sentence structure variety across three
specification documents to enhance readability. In
spec/changes/active/desktop-prompt-relationship-tree/design.md lines 47-52,
replace the repetitive "Add" verb used in multiple list items with varied verbs
such as "Reject" for self-relation constraints and "Prevent" for duplicate
prevention. In
spec/changes/active/desktop-prompt-relationship-tree/implementation.md lines
15-20, replace the three consecutive items starting with "Added" by using
alternative expressions like "Introduced" or "Extended" to describe the
features. In
spec/changes/active/desktop-prompt-relationship-tree/specs/prompt-relationships/spec.md
lines 42-53, replace the five scenario items starting with "When a user" with
more concise and varied verb-based phrases such as "Pressing", "Deleting", or
"Exporting" to avoid repetitive grammatical structure while maintaining clarity.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c41012fe-acb8-4c99-821b-400f5142c4d1
📒 Files selected for processing (33)
apps/desktop/src/main/database/index.tsapps/desktop/src/main/ipc/index.tsapps/desktop/src/main/ipc/prompt.ipc.tsapps/desktop/src/preload/api/prompt.tsapps/desktop/src/renderer/components/layout/MainContent.tsxapps/desktop/src/renderer/components/prompt/PromptListView.tsxapps/desktop/src/renderer/i18n/locales/de.jsonapps/desktop/src/renderer/i18n/locales/en.jsonapps/desktop/src/renderer/i18n/locales/es.jsonapps/desktop/src/renderer/i18n/locales/fr.jsonapps/desktop/src/renderer/i18n/locales/ja.jsonapps/desktop/src/renderer/i18n/locales/zh-TW.jsonapps/desktop/src/renderer/i18n/locales/zh.jsonapps/desktop/src/renderer/services/database-backup-format.tsapps/desktop/src/renderer/services/database-backup.tsapps/desktop/src/renderer/services/database.tsapps/desktop/src/renderer/stores/prompt.store.tsapps/desktop/tests/integration/services/database-backup-filesystem.integration.test.tsapps/desktop/tests/unit/components/prompt-list-view-relations.test.tsxapps/desktop/tests/unit/main/prompt-relation-db.test.tsapps/desktop/tests/unit/services/database-backup-format.test.tsapps/desktop/tests/unit/services/database-backup.test.tspackages/db/src/index.tspackages/db/src/init.tspackages/db/src/prompt-relation.tspackages/db/src/schema.tspackages/shared/constants/ipc-channels.tspackages/shared/types/prompt.tsspec/changes/active/desktop-prompt-relationship-tree/design.mdspec/changes/active/desktop-prompt-relationship-tree/implementation.mdspec/changes/active/desktop-prompt-relationship-tree/proposal.mdspec/changes/active/desktop-prompt-relationship-tree/specs/prompt-relationships/spec.mdspec/changes/active/desktop-prompt-relationship-tree/tasks.md
Comment on lines
+284
to
+308
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_CREATE, async (_, data: CreatePromptRelationDTO) => { | ||
| const relation = relationDb.create(data); | ||
| syncWorkspace(); | ||
| return relation; | ||
| }); | ||
|
|
||
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_LIST, async (_, query?: PromptRelationQuery) => { | ||
| return relationDb.list(query); | ||
| }); | ||
|
|
||
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_UPDATE, async (_, id: string, data: UpdatePromptRelationDTO) => { | ||
| const relation = relationDb.update(id, data); | ||
| if (relation) { | ||
| syncWorkspace(); | ||
| } | ||
| return relation; | ||
| }); | ||
|
|
||
| ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_DELETE, async (_, id: string) => { | ||
| const deleted = relationDb.delete(id); | ||
| if (deleted) { | ||
| syncWorkspace(); | ||
| } | ||
| return deleted; | ||
| }); |
There was a problem hiding this comment.
请在关系 IPC 处理器中补齐入参校验与结构化错误返回。
Line 284-308 的 4 个新增 handler 直接信任 renderer 传入数据，且未做统一 try/catch。建议先校验 id/query/data 的运行时类型，再统一返回结构化错误对象，避免把 malformed payload 直接下沉到 DB 并产生非结构化拒绝。
建议修复（示例）
+ const toIpcError = (error: unknown) => ({
+ ok: false as const,
+ error: { message: error instanceof Error ? error.message : "Unknown error" },
+ });
+
+ const assertRelationId = (value: unknown): string => {
+ if (typeof value !== "string" || value.trim().length === 0) {
+ throw new Error("Relation id is required");
+ }
+ return value;
+ };
+
ipcMain.handle(IPC_CHANNELS.PROMPT_RELATION_CREATE, async (_, data: CreatePromptRelationDTO) => {
- const relation = relationDb.create(data);
- syncWorkspace();
- return relation;
+ try {
+ // TODO: 替换为仓库统一的 runtime DTO 校验函数
+ const relation = relationDb.create(data);
+ syncWorkspace();
+ return { ok: true as const, data: relation };
+ } catch (error) {
+ return toIpcError(error);
+ }
});As per coding guidelines, “apps/desktop/src/main/ipc/**: All IPC handlers must validate input types and reject malformed payloads before processing” and “IPC handlers must catch errors and return structured error responses, never crash the main process silently”.
🤖 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/desktop/src/main/ipc/prompt.ipc.ts` around lines 284 - 308, The four
relation IPC handlers (PROMPT_RELATION_CREATE, PROMPT_RELATION_LIST,
PROMPT_RELATION_UPDATE, PROMPT_RELATION_DELETE) lack input validation and error
handling. For each handler, add runtime type validation for the input parameters
(validate that id is a non-empty string, query is a valid object if provided,
and data matches the expected DTO structure), wrap the handler logic in a
try/catch block, and return a structured error response object (instead of
throwing or crashing) when validation fails or database operations throw errors.
This prevents malformed payloads from reaching the database layer and ensures
consistent error responses to the renderer process.
Source: Coding guidelines
Comment on lines
+459
to
+486
| const commitRelationDrop = useCallback( | ||
| async (kind: PromptRelationKind) => { | ||
| if (!pendingRelationDrop) { | ||
| return; | ||
| } | ||
|
|
||
| const { sourcePromptId, targetPromptId } = pendingRelationDrop; | ||
|
|
||
| if (kind === 'grouped_under') { | ||
| if (canMoveToParent(sourcePromptId, targetPromptId)) { | ||
| setExpandedIds((current) => new Set(current).add(targetPromptId)); | ||
| onMovePrompt( | ||
| sourcePromptId, | ||
| targetPromptId, | ||
| getChildren(targetPromptId).length, | ||
| ); | ||
| } | ||
| setPendingRelationDrop(null); | ||
| return; | ||
| } | ||
|
|
||
| setPendingRelationDrop(null); | ||
| await onCreateRelation?.({ | ||
| sourcePromptId, | ||
| targetPromptId, | ||
| kind, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
关系创建失败时缺少错误处理，可能产生未处理 Promise 拒绝。
当前先关闭菜单再异步创建关系，且点击处用 void 丢弃返回 Promise；一旦创建失败（如重复关系/IPC 报错），这里没有兜底处理，用户也拿不到反馈。
建议修复
const commitRelationDrop = useCallback(
async (kind: PromptRelationKind) => {
if (!pendingRelationDrop) {
return;
}
const { sourcePromptId, targetPromptId } = pendingRelationDrop;
@@
- setPendingRelationDrop(null);
- await onCreateRelation?.({
- sourcePromptId,
- targetPromptId,
- kind,
- });
+ if (!onCreateRelation) {
+ setPendingRelationDrop(null);
+ return;
+ }
+
+ try {
+ await onCreateRelation({
+ sourcePromptId,
+ targetPromptId,
+ kind,
+ });
+ setPendingRelationDrop(null);
+ } catch (error) {
+ console.error("Failed to create prompt relation:", error);
+ }
},Also applies to: 768-769
🤖 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/desktop/src/renderer/components/prompt/PromptListView.tsx` around lines
459 - 486, The commitRelationDrop callback clears the UI state
(setPendingRelationDrop) before awaiting the async onCreateRelation operation,
and since the function is called with void at the click handler, any Promise
rejection from onCreateRelation will go unhandled. Wrap the await
onCreateRelation call in a try-catch block to handle potential errors (such as
duplicate relations or IPC failures), and either restore the UI state on error
or display an error message to the user so they receive feedback when the
relation creation fails.
Comment on lines
+96
to
+100
| const [prompts, relations] = await Promise.all([ | ||
| db.getAllPrompts(), | ||
| db.listPromptRelations(), | ||
| ]); | ||
| set({ prompts, relations }); |
There was a problem hiding this comment.
关系查询失败会阻断主 Prompt 列表加载。
这里把核心数据和关系数据绑在同一个 Promise.all，任一失败都会走 catch，导致 prompts 也不更新。关系是增强信息，不应让主列表不可用。建议将关系查询降级为“失败不阻断主流程”。
建议修复
fetchPrompts: async () => {
set({ isLoading: true });
try {
- // Get data from IndexedDB
- const [prompts, relations] = await Promise.all([
- db.getAllPrompts(),
- db.listPromptRelations(),
- ]);
- set({ prompts, relations });
+ const prompts = await db.getAllPrompts();
+ let relations = get().relations;
+ try {
+ relations = await db.listPromptRelations();
+ } catch (relationError) {
+ console.warn("Failed to fetch prompt relations:", relationError);
+ }
+ set({ prompts, relations });
} catch (error) {
console.error("Failed to fetch prompts:", error);
} finally {
set({ isLoading: false });
}
},🤖 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/desktop/src/renderer/stores/prompt.store.ts` around lines 96 - 100, The
current implementation combines the critical prompt data fetch with the
secondary relations data fetch in a single Promise.all(), which means if the
relations query fails, both prompts and relations fail to update. Since
relations are enhancement data and should not block the main prompt list
availability, separate the relations query from the main data fetch. Keep the
getAllPrompts() call as the primary data source, and handle the
listPromptRelations() query independently with error handling that allows it to
fail gracefully (either silently or with a default empty value) without
preventing the prompts from being set in the store. This way, the prompt list
will be available even if the relations query fails.
Comment on lines
+31
to
+132
| it("creates and lists directed prompt relations", () => { | ||
| const source = createPrompt("Source"); | ||
| const target = createPrompt("Target"); | ||
|
|
||
| const relation = relationDb.create({ | ||
| sourcePromptId: source.id, | ||
| targetPromptId: target.id, | ||
| kind: "depends_on", | ||
| note: "needs context", | ||
| }); | ||
|
|
||
| expect(relation.sourcePromptId).toBe(source.id); | ||
| expect(relation.targetPromptId).toBe(target.id); | ||
| expect(relation.kind).toBe("depends_on"); | ||
| expect(relation.note).toBe("needs context"); | ||
| expect(relationDb.list({ promptId: source.id, direction: "outgoing" })).toEqual([ | ||
| relation, | ||
| ]); | ||
| expect(relationDb.list({ promptId: target.id, direction: "incoming" })).toEqual([ | ||
| relation, | ||
| ]); | ||
| }); | ||
|
|
||
| it("canonicalizes related_to as an undirected relation", () => { | ||
| const first = createPrompt("A"); | ||
| const second = createPrompt("B"); | ||
|
|
||
| const original = relationDb.create({ | ||
| sourcePromptId: second.id, | ||
| targetPromptId: first.id, | ||
| kind: "related_to", | ||
| }); | ||
| const duplicate = relationDb.create({ | ||
| sourcePromptId: first.id, | ||
| targetPromptId: second.id, | ||
| kind: "related_to", | ||
| }); | ||
|
|
||
| expect(duplicate.id).toBe(original.id); | ||
| expect(relationDb.list({ promptId: first.id })).toHaveLength(1); | ||
| expect(relationDb.list({ promptId: second.id })).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("rejects invalid relation endpoints and kinds", () => { | ||
| const prompt = createPrompt("Prompt"); | ||
|
|
||
| expect(() => | ||
| relationDb.create({ | ||
| sourcePromptId: prompt.id, | ||
| targetPromptId: prompt.id, | ||
| kind: "related_to", | ||
| }), | ||
| ).toThrow("Prompt relation cannot point to itself"); | ||
| expect(() => | ||
| relationDb.create({ | ||
| sourcePromptId: prompt.id, | ||
| targetPromptId: "missing", | ||
| kind: "related_to", | ||
| }), | ||
| ).toThrow("Target prompt does not exist"); | ||
| expect(() => | ||
| relationDb.create({ | ||
| sourcePromptId: prompt.id, | ||
| targetPromptId: "missing", | ||
| kind: "grouped_under" as "related_to", | ||
| }), | ||
| ).toThrow("Unsupported prompt relation kind"); | ||
| }); | ||
|
|
||
| it("updates and deletes relations", () => { | ||
| const source = createPrompt("Source"); | ||
| const target = createPrompt("Target"); | ||
| const relation = relationDb.create({ | ||
| sourcePromptId: source.id, | ||
| targetPromptId: target.id, | ||
| kind: "variant_of", | ||
| }); | ||
|
|
||
| const updated = relationDb.update(relation.id, { | ||
| kind: "next_step", | ||
| note: "run after source", | ||
| }); | ||
|
|
||
| expect(updated?.kind).toBe("next_step"); | ||
| expect(updated?.note).toBe("run after source"); | ||
| expect(relationDb.delete(relation.id)).toBe(true); | ||
| expect(relationDb.list()).toEqual([]); | ||
| }); | ||
|
|
||
| it("deletes relations when a referenced prompt is deleted", () => { | ||
| const source = createPrompt("Source"); | ||
| const target = createPrompt("Target"); | ||
| relationDb.create({ | ||
| sourcePromptId: source.id, | ||
| targetPromptId: target.id, | ||
| kind: "next_step", | ||
| }); | ||
|
|
||
| expect(relationDb.list()).toHaveLength(1); | ||
| expect(promptDb.delete(source.id)).toBe(true); | ||
| expect(relationDb.list()).toEqual([]); | ||
| }); |
There was a problem hiding this comment.
补齐关系文本字段的恶意输入与多语言回写测试
当前仅覆盖普通文本，缺少对 prompt_relations.note / 相关文本字段的高风险输入回写验证。建议补充 write→read 场景：<script>alert(1)</script>、HTML entities、事件处理器字符串、CJK/emoji(含多码点)/RTL/零宽字符、控制字符（含 \x00）及 10KB+ 长文本，确保不会发生静默截断或污染。
As per coding guidelines, “Store and retrieve <script>alert(1)</script>...”, “Full round-trip ... CJK/emoji/RTL/zero-width...”, and “Test 10KB+ strings ...” are required for **/*.test.ts.
🤖 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/desktop/tests/unit/main/prompt-relation-db.test.ts` around lines 31 -
132, The test suite for prompt relations is missing comprehensive validation of
the note field and other text fields for malicious and complex input handling.
Add new test cases that verify the relationDb.create() and relationDb.update()
methods correctly store and retrieve the note field with high-risk inputs
including XSS vectors (like <script>alert(1)</script>), HTML entities, event
handler strings, CJK and emoji characters (including multi-codepoint sequences),
RTL text, zero-width characters, control characters (including \x00), and
strings exceeding 10KB in length. Each test should verify that values are stored
and retrieved without silent truncation, contamination, or data loss through
write→read round-trip validation.
Source: Coding guidelines
Comment on lines
+146
to
+171
| private normalizeCreateInput( | ||
| data: CreatePromptRelationDTO, | ||
| ): CreatePromptRelationDTO { | ||
| this.assertPromptId(data.sourcePromptId, "Source prompt id"); | ||
| this.assertPromptId(data.targetPromptId, "Target prompt id"); | ||
| this.assertRelationKind(data.kind); | ||
|
|
||
| if (data.sourcePromptId === data.targetPromptId) { | ||
| throw new Error("Prompt relation cannot point to itself"); | ||
| } | ||
|
|
||
| this.assertPromptExists(data.sourcePromptId, "Source prompt does not exist"); | ||
| this.assertPromptExists(data.targetPromptId, "Target prompt does not exist"); | ||
|
|
||
| const endpoints = this.normalizeEndpoints( | ||
| data.sourcePromptId, | ||
| data.targetPromptId, | ||
| data.kind, | ||
| ); | ||
|
|
||
| return { | ||
| ...data, | ||
| ...endpoints, | ||
| note: data.note ?? null, | ||
| }; | ||
| } |
There was a problem hiding this comment.
建议在写入前校验或移除 null byte。
根据编码规范，SQLite 字符串输入可能在 null byte (\x00) 处截断丢失数据。当前 assertPromptId 只检查非空，note 字段也未过滤 null byte。建议在 normalizeCreateInput 和 update 中对所有字符串字段（sourcePromptId、targetPromptId、note）进行 null byte 校验或清理，并在测试中覆盖这一行为。
🛡️ 建议的修复示例
private assertPromptId(value: string, label: string): void {
if (typeof value !== "string" || value.trim().length === 0) {
throw new Error(`${label} is required`);
}
+ if (value.includes('\x00')) {
+ throw new Error(`${label} contains null byte`);
+ }
}或在 normalizeCreateInput 中统一处理：
private normalizeCreateInput(
data: CreatePromptRelationDTO,
): CreatePromptRelationDTO {
this.assertPromptId(data.sourcePromptId, "Source prompt id");
this.assertPromptId(data.targetPromptId, "Target prompt id");
this.assertRelationKind(data.kind);
+ const cleanNote = data.note?.replace(/\x00/g, '') ?? null;
+
if (data.sourcePromptId === data.targetPromptId) {
throw new Error("Prompt relation cannot point to itself");
}
this.assertPromptExists(data.sourcePromptId, "Source prompt does not exist");
this.assertPromptExists(data.targetPromptId, "Target prompt does not exist");
const endpoints = this.normalizeEndpoints(
data.sourcePromptId,
data.targetPromptId,
data.kind,
);
return {
...data,
...endpoints,
- note: data.note ?? null,
+ note: cleanNote,
};
}As per coding guidelines: "SQLite string inputs can lose data around null bytes (\x00); input validation should strip or reject null bytes before database writes and test this behavior"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private normalizeCreateInput( | |
| data: CreatePromptRelationDTO, | |
| ): CreatePromptRelationDTO { | |
| this.assertPromptId(data.sourcePromptId, "Source prompt id"); | |
| this.assertPromptId(data.targetPromptId, "Target prompt id"); | |
| this.assertRelationKind(data.kind); | |
| if (data.sourcePromptId === data.targetPromptId) { | |
| throw new Error("Prompt relation cannot point to itself"); | |
| } | |
| this.assertPromptExists(data.sourcePromptId, "Source prompt does not exist"); | |
| this.assertPromptExists(data.targetPromptId, "Target prompt does not exist"); | |
| const endpoints = this.normalizeEndpoints( | |
| data.sourcePromptId, | |
| data.targetPromptId, | |
| data.kind, | |
| ); | |
| return { | |
| ...data, | |
| ...endpoints, | |
| note: data.note ?? null, | |
| }; | |
| } | |
| private normalizeCreateInput( | |
| data: CreatePromptRelationDTO, | |
| ): CreatePromptRelationDTO { | |
| this.assertPromptId(data.sourcePromptId, "Source prompt id"); | |
| this.assertPromptId(data.targetPromptId, "Target prompt id"); | |
| this.assertRelationKind(data.kind); | |
| const cleanNote = data.note?.replace(/\x00/g, '') ?? null; | |
| if (data.sourcePromptId === data.targetPromptId) { | |
| throw new Error("Prompt relation cannot point to itself"); | |
| } | |
| this.assertPromptExists(data.sourcePromptId, "Source prompt does not exist"); | |
| this.assertPromptExists(data.targetPromptId, "Target prompt does not exist"); | |
| const endpoints = this.normalizeEndpoints( | |
| data.sourcePromptId, | |
| data.targetPromptId, | |
| data.kind, | |
| ); | |
| return { | |
| ...data, | |
| ...endpoints, | |
| note: cleanNote, | |
| }; | |
| } |
🤖 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 `@packages/db/src/prompt-relation.ts` around lines 146 - 171, The
normalizeCreateInput method does not validate or remove null bytes from string
inputs (sourcePromptId, targetPromptId, and note), which can cause data loss
when writing to SQLite. Add null byte validation or cleaning for all string
fields in the normalizeCreateInput method to either reject inputs containing
null bytes or strip them before returning. Additionally, ensure the update
method applies the same null byte handling to maintain consistency across all
database write operations, and add test cases that verify this behavior by
attempting to create or update relations with null bytes in these fields.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
发布说明