Add project automation management#155
Open
friuns2 wants to merge 14 commits into
Open
Conversation
Review Summary by Qodo(Agentic_describe updated until commit ddd9014)Add project automation management with combined Automations panel
WalkthroughsDescription• Add project-scoped cron automations with sidebar management UI • Create top-level Automations panel combining thread and project automations • Implement project automation API endpoints and TOML serialization • Display automation indicators and counts in sidebar threads and projects • Sort automations by active status first, then newest creation date Diagramflowchart LR
A["Project/Thread"] -->|"Add automation"| B["Automation Dialog"]
B -->|"Save"| C["TOML Storage"]
C -->|"cwds array"| D["Project Cron"]
C -->|"target_thread_id"| E["Thread Heartbeat"]
D -->|"GET /project-automations"| F["Automations Panel"]
E -->|"GET /thread-automations"| F
F -->|"Display"| G["Combined List"]
G -->|"Sort: Active→Paused→Newest"| H["Sorted Rows"]
H -->|"Edit"| B
File Changes1. src/api/codexGateway.ts
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+3403
to
+3507
| async function listProjectCronAutomations(): Promise<Record<string, ThreadAutomationRecord[]>> { | ||
| const automationRoot = getCodexAutomationsDir() | ||
| const next: Record<string, ThreadAutomationRecord[]> = {} | ||
| let entries | ||
| try { | ||
| entries = await readdir(automationRoot, { withFileTypes: true }) | ||
| } catch { | ||
| return next | ||
| } | ||
|
|
||
| for (const entry of entries) { | ||
| if (!entry.isDirectory()) continue | ||
| const automation = await readAutomationRecordFromFile(join(automationRoot, entry.name, 'automation.toml')) | ||
| if (!automation || automation.kind !== 'cron' || automation.cwds.length === 0) continue | ||
| for (const cwd of automation.cwds) { | ||
| next[cwd] = [...(next[cwd] ?? []), automation] | ||
| } | ||
| } | ||
|
|
||
| for (const automations of Object.values(next)) { | ||
| automations.sort((first, second) => { | ||
| const firstCreatedAt = first.createdAtMs ?? 0 | ||
| const secondCreatedAt = second.createdAtMs ?? 0 | ||
| if (firstCreatedAt !== secondCreatedAt) return firstCreatedAt - secondCreatedAt | ||
| return first.id.localeCompare(second.id) | ||
| }) | ||
| } | ||
|
|
||
| return next | ||
| } | ||
|
|
||
| async function readProjectCronAutomations(projectName: string): Promise<ThreadAutomationRecord[]> { | ||
| const all = await listProjectCronAutomations() | ||
| return all[projectName] ?? [] | ||
| } | ||
|
|
||
| async function readProjectCronAutomation(projectName: string, automationId = ''): Promise<ThreadAutomationRecord | null> { | ||
| const automations = await readProjectCronAutomations(projectName) | ||
| if (automationId) return automations.find((automation) => automation.id === automationId) ?? null | ||
| return automations[0] ?? null | ||
| } | ||
|
|
||
| async function writeProjectCronAutomation(input: { | ||
| projectName: string | ||
| id?: string | ||
| name: string | ||
| prompt: string | ||
| rrule: string | ||
| status: ThreadAutomationStatus | ||
| }): Promise<ThreadAutomationRecord> { | ||
| const projectName = input.projectName.trim() | ||
| const name = input.name.trim() | ||
| const prompt = input.prompt.trim() | ||
| const rrule = input.rrule.trim() | ||
| if (!projectName || !name || !prompt || !rrule) { | ||
| throw new Error('projectName, name, prompt, and rrule are required') | ||
| } | ||
|
|
||
| const automationRoot = getCodexAutomationsDir() | ||
| await mkdir(automationRoot, { recursive: true }) | ||
| const existing = input.id ? await readProjectCronAutomation(projectName, input.id.trim()) : null | ||
| const entries = await readdir(automationRoot, { withFileTypes: true }).catch(() => []) | ||
| const existingIds = new Set(entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name)) | ||
| const id = existing?.id ?? resolveUniqueAutomationId(existingIds, projectName, name) | ||
| const automationDir = join(automationRoot, id) | ||
| const now = Date.now() | ||
| const record: ThreadAutomationRecord = { | ||
| id, | ||
| kind: 'cron', | ||
| name, | ||
| prompt, | ||
| rrule, | ||
| status: input.status, | ||
| targetThreadId: null, | ||
| cwds: [projectName], | ||
| createdAtMs: existing?.createdAtMs ?? now, | ||
| updatedAtMs: now, | ||
| nextRunAtMs: null, | ||
| } | ||
|
|
||
| await mkdir(automationDir, { recursive: true }) | ||
| await writeFile(join(automationDir, 'automation.toml'), serializeAutomationToml(record), 'utf8') | ||
| const memoryPath = join(automationDir, 'memory.md') | ||
| try { | ||
| await stat(memoryPath) | ||
| } catch { | ||
| await writeFile(memoryPath, '', 'utf8') | ||
| } | ||
| return record | ||
| } | ||
|
|
||
| async function deleteProjectCronAutomation(projectName: string, automationId = ''): Promise<boolean> { | ||
| const normalizedProjectName = projectName.trim() | ||
| const normalizedAutomationId = automationId.trim() | ||
| if (normalizedAutomationId) { | ||
| const automation = await readProjectCronAutomation(normalizedProjectName, normalizedAutomationId) | ||
| if (!automation) return false | ||
| await rm(join(getCodexAutomationsDir(), automation.id), { recursive: true, force: true }) | ||
| return true | ||
| } | ||
|
|
||
| const automations = await readProjectCronAutomations(normalizedProjectName) | ||
| if (automations.length === 0) return false | ||
| await Promise.all(automations.map((automation) => rm(join(getCodexAutomationsDir(), automation.id), { recursive: true, force: true }))) | ||
| return true |
There was a problem hiding this comment.
2. Multi-cwd cron data loss 🐞 Bug ≡ Correctness
Project cron automations are indexed under every entry in cwds, but saving overwrites cwds to a single value and deleting removes the entire automation folder; a cron automation associated with multiple projects can be unintentionally modified/removed for other projects. This can lead to cross-project automation loss when editing or deleting from a single project (including project removal cleanup).
Agent Prompt
### Issue description
Project cron automations can legally have multiple `cwds`, but the current implementation (a) overwrites `cwds` to a single entry on save and (b) deletes the entire automation directory when removing an automation for a single cwd. This causes silent cross-project data loss for shared/multi-cwd cron automations.
### Issue Context
- `listProjectCronAutomations()` indexes the same automation under every cwd in `cwds`.
- Deleting by cwd currently removes the automation directory (`rm .../<automation.id>`), affecting all cwd associations.
- Saving an existing cron automation always writes `cwds: [projectName]`, dropping other cwd associations.
### Fix Focus Areas
- src/server/codexAppServerBridge.ts[3403-3432]
- src/server/codexAppServerBridge.ts[3445-3492]
- src/server/codexAppServerBridge.ts[3494-3508]
- src/components/sidebar/SidebarThreadTree.vue[2120-2126]
### Implementation direction
- Decide on supported behavior:
1) If multi-cwd cron automations are supported:
- On delete for one cwd: if `automation.cwds.length > 1`, rewrite `automation.toml` with that cwd removed (and only delete the directory if no cwds remain).
- On save/edit: preserve existing `cwds` (or at minimum, preserve all existing cwds and ensure the current cwd remains included).
2) If multi-cwd is *not* supported:
- Enforce it explicitly: reject/ignore cron automations with `cwds.length !== 1` in project-scoped APIs, and avoid deleting directories that are shared across multiple cwds.
- Ensure sidebar “remove project” cleanup does not delete shared automations (same rule as above).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| --- | ||
|
|
||
| ### Pinned threads remain visible during background pagination |
There was a problem hiding this comment.
1. tests.md cases removed 📘 Rule violation ⚙ Maintainability
This PR deletes/replaces existing manual test sections in tests.md, reducing historical regression coverage. Compliance requires preserving existing test cases and only appending or minimally updating what’s needed.
Agent Prompt
## Issue description
This PR removes existing `tests.md` manual test cases (replacing one section and deleting another), which reduces historical regression coverage.
## Issue Context
Per compliance, `tests.md` updates must be additive or minimally edited for the new feature; existing cases should not be removed unless there is a very strong reason and an equivalent replacement is kept.
## Fix Focus Areas
- tests.md[274-301]
- tests.md[1695-1727]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Persistent review updated to latest commit ddd9014 |
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
2 participants
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