close
Skip to content

feat: add meeting deletion - #2268

Merged
kyleseaman merged 1 commit into
kirodotdev:mainfrom
Frxnesvo:feat/meetings-delete
Aug 9, 2026
Merged

feat: add meeting deletion#2268
kyleseaman merged 1 commit into
kirodotdev:mainfrom
Frxnesvo:feat/meetings-delete

Conversation

@Frxnesvo

@Frxnesvo Frxnesvo commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Meetings that have local notes and artifacts cannot be removed from the dashboard. Users have no way to clean up completed test, duplicate, or no-longer-needed meeting data without editing the data directory manually.

Why it matters

Meeting data can include transcripts, notes, diagrams, and action items. A clear, guarded deletion flow gives users control over that local data while preventing an in-progress meeting from being removed accidentally.

What changed (motivation → approach → change)

  • Adds a danger IconButton with the dashboard's established Trash2 delete icon to the far right of each locally stored meeting row, after the status and notes icon.
  • Keeps the action visible but disabled for active, paused, and reviewing meetings. Calendar-only rows do not expose a local-data delete action.
  • Shows an irreversible-action confirmation before deletion, uses a neutral pending loader, refreshes the list on success, and clears meeting-scoped React Query data.
  • Reports failures both through dashboard notifications and inline beside the affected meeting, preserving an actionable backend message such as the stale-list 409 response.
  • Adds DELETE /api/apps/meetings/meetings/{meeting_id}. The backend validates containment, refuses linked meeting directories, rejects live sessions with 409 meeting_active, and returns 404 meeting_not_found for missing meetings.
  • Uses literal HTTP status values for the non-2xx responses so the static error-code contract can verify both responses.
  • Serializes deletion with initialization, agent toggles, task mutations, and task filing. This prevents concurrent requests from recreating partial meeting data or creating an external task between the filing read and local deletion.
  • Localizes the new labels and messages across every shipped catalog and the pseudolocale.
  • Documents the lifecycle and deletion contract in the Meetings system specification.

State behavior

Meeting state Delete action Result
Ended/local Enabled Confirmation, deletion, cache/list refresh, notification
Active Disabled No request; backend also rejects a live session
Paused Disabled No request; backend also rejects a live session
Reviewing Disabled No request; backend also rejects a live session
Calendar-only Hidden External calendar event remains untouched

Tests

  • python -m pytest -q test/test_error_code_contract.py test/test_meetings_routes.py test/test_meetings_store.py — 242 passed; covers the HTTP error-code contract, deletion success/missing/live cases, path safety, and deterministic delete races with initialization, Quick Add, and agent toggles.
  • npx vitest run src/test/MeetingsPage.test.tsx src/test/MeetingsApiClient.test.ts src/i18n/catalogParity.test.ts --coverage=false — 92 passed; covers UI states, semantic delete and pending icons, inline backend failures, confirmation, API behavior, cache cleanup, and catalog parity.
  • Stabilized test_first_empty_response_requeues_message by awaiting its cancelled background tasks and scoping the flush assertion to its own slot; it passes 5 consecutive focused runs, the 10-test class, and all 540 dashboard-chat tests in both serial and parallel execution.
  • Stabilized the agent template tab reset by moving selection cleanup to the layout phase; both previously failing skill-tab tests pass 5 consecutive coverage runs and the full coverage suite.
  • npm run test:website — 11,408 passed, 2 expected failures, 3 skipped across 858 files.
  • npm run test:electron — 844 passed.
  • npm run build.
  • npm run lint — 0 errors (repository baseline warnings only).
  • I18N_BASE_REF=upstream/main npm run lint:i18n and I18N_BASE_REF=upstream/main npm run i18n:check — passed all diff-scoped and enforced checks.
  • isort --check-only, flake8, and mypy on all modified Meetings backend modules — passed.
  • scripts/docs-lint.sh, scripts/scrub-lint.sh --no-history, the brand-name gate, and git diff --check — passed.
  • Rebased onto current main, which includes the merged Korean catalog parity fix from fix(i18n): restore ko catalog parity broken on main #2244; the duplicate follow-up fix(i18n): add missing ko translations breaking catalogParity on main #2256 was closed without merge.

Manual verification

website/scripts/capture-meetings-delete.mjs runs the production SPA in Chromium with deterministic API fixtures and verifies:

  • the ended-meeting action uses the established trash icon and is optically centered;
  • active, paused, and reviewing actions are disabled;
  • calendar-only rows have no delete action;
  • the confirmation names the meeting and warns that deletion cannot be undone;
  • accepting removes the row and shows the success notification;
  • a failed deletion keeps the row and shows the backend message inline;
  • both dark and light themes render correctly.

Screenshots / video

Meeting list states — dark

Meeting deletion controls in dark mode

Successful deletion and notification

Meeting removed with success notification

Failed deletion with inline feedback

Failed meeting deletion with an inline backend message

Meeting list states — light

Meeting deletion controls in light mode

Related Issues

N/A — no open issue or pull request specifically covering meeting deletion was found.

Checklist

  • Single commit with a Conventional Commits title (feat: add meeting deletion)
  • Existing relevant tests pass and new tests cover the new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

N/A — the repository template does not currently require contributor-entered CLA text.

@Frxnesvo
Frxnesvo requested a review from a team August 8, 2026 20:02
@Frxnesvo
Frxnesvo requested a review from a team as a code owner August 8, 2026 20:02
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 8, 2026
@Frxnesvo
Frxnesvo force-pushed the feat/meetings-delete branch from 93e9eef to ae1179e Compare August 9, 2026 09:33
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 9, 2026
@Frxnesvo
Frxnesvo force-pushed the feat/meetings-delete branch from ae1179e to 9365a52 Compare August 9, 2026 10:21
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Advisory design-level review of 190f2730d3e21202db37c3c55f460f9570608211 via the fork AI-review pipeline — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

Sound deletion design; the risk is two unrelated production/test changes riding along, including a timing change to AgentsPage's delete-confirmation guard.

Watch

  • AgentsPage.tsx's useEffectuseLayoutEffect swap changes when the armed delete-confirm disarms — the exact guard whose comment warns "a click there deletes B's config file." It's justified only as test stabilization ("moving selection cleanup to the layout phase") and will get zero scrutiny under a "meeting deletion" headline. Move it and the test_dashboard_chat.py flake fix to their own PR.
  • Deletion pulls /init and agent toggles under the global START_LOCK, the same lock handle_start_meeting holds across its long awaited agent-initialization sequence — any init, toggle, or delete issued during a start now stalls for that whole sequence, across all meetings. Acceptable single-user, but the four-lock nesting (START_LOCK_FILING_LOCK_TASKS_LOCKmeta_transaction) now spans three modules with no written ordering rule; document it where START_LOCK is defined before the next path acquires them differently.

Suggestions

  • The UI disables delete purely from persisted status, so a meeting left active by a crash (backend ACTIVE empty, delete would succeed) shows a permanently disabled button — consider trusting the backend's 409 as the arbiter instead of pre-blocking on stale status.

[DESIGN-REVIEWED] 190f273

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 190f2730d3e21202db37c3c55f460f9570608211 via the fork AI-review pipeline; updated in place on each push.

Review details

I've reviewed the full diff and the surrounding lock/containment machinery. Let me do a final check on lock ordering to rule out a deadlock in the new delete path.

The delete path acquires locks in order: START_LOCK_FILING_LOCK → (worker thread) _TASKS_LOCK_META_LOCK. I verified no other path acquires these in a conflicting order (handle_file_task/handle_delete_task: _FILING_LOCK_TASKS_LOCK; handle_start_meeting: START_LOCK_META_LOCK; no path takes _META_LOCK_TASKS_LOCK), so no lock-ordering inversion exists. The shutil.rmtree runs via asyncio.to_thread, off the event loop. The containment barrier (contain + is_link_or_junction rejection before rmtree) blocks both out-of-root symlink escape and in-root link identity confusion. The _append_task 404 guard correctly serializes against deletion under _TASKS_LOCK.

No findings.

[OPUS-REVIEWED] 190f273

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 190f2730d3e21202db37c3c55f460f9570608211 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 190f273

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — 🟡 CONCERNS

Advisory UX-level review of 190f2730d3e21202db37c3c55f460f9570608211 via the fork AI-review pipeline — updated in place on each push; does not block merge.

I've reviewed the full patch, the surrounding MeetingsPage/Clickable/IconButton code, sibling confirm patterns, and the backend route strings. Here is the review.

UX-Verdict: CONCERNS

Solid guarded delete flow, but calendar-synced rows survive their own "Deleted" toast, and failures render the backend's raw lowercase English in all 12 locales.

Watch

  • Deleting a calendar-backed meeting removes only local data, so after invalidateQueries({queryKey: ['meetings','list']}) the row re-renders as a plain scheduled event while the notification says Deleted “{{title}}” — the user sees the "deleted" meeting still in the list and reads it as a failed delete. Frequent (calendar is the primary meeting source) × confusion/retry friction × every delete. Smallest fix: add one clause to deleteConfirm/deleted copy ("the calendar event stays").
  • deleteFailureMessage prefers raw error.message, so a real 409 shows the backend's unlocalized, lowercase "end the meeting before deleting it" inline in every non-English UI — while a translated deleteUnavailable string already exists and the response carries code: "meeting_active". Rare (stale-list race) × jarring, half-translated error × every occurrence. Map the code to the catalog key; note the capture stub capitalizes the message, so screenshot 04 overstates the shipped copy.

Suggestions

  • For live rows the button's aria-label/title becomes the instruction "End this meeting before deleting it", which no longer names the control; keep Delete {{title}} as the accessible name and put the reason in title only.
  • The success/error notifications land behind the header's Notifications button; the inline row alert is the real feedback — consider dropping the duplicate notify on error to avoid the same sentence appearing twice.

[UX-REVIEWED] 190f273

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 9, 2026
@Frxnesvo
Frxnesvo force-pushed the feat/meetings-delete branch from 9365a52 to 2f7e813 Compare August 9, 2026 11:22
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 9, 2026
@Frxnesvo

Frxnesvo commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the Fable UX review in 2f7e8131c47cffd3a62f7d9e6f3b0e2ee9609285:

  • replaced the ambiguous X with the dashboard's established Trash2 delete icon;
  • replaced the sync-shaped pending indicator with Loader2;
  • added row-local inline error feedback and preserved the actionable backend message, while retaining the notification-feed entry;
  • added tests for the semantic icons and backend-failure state;
  • added the failure-state screenshot to the PR description and refreshed all UI captures.

Focused frontend coverage: 92 tests passed. The full frontend and Electron test command also passes (844 Electron tests).

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 9, 2026
@Frxnesvo
Frxnesvo force-pushed the feat/meetings-delete branch from 2f7e813 to cdbdc28 Compare August 9, 2026 13:23
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
let users remove inactive meetings and their local notes from the meetings list while protecting live sessions from destructive races.
@Frxnesvo
Frxnesvo force-pushed the feat/meetings-delete branch from cdbdc28 to 190f273 Compare August 9, 2026 17:29
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@kyleseaman
kyleseaman merged commit 8c85a4d into kirodotdev:main Aug 9, 2026
52 checks passed
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 9, 2026
bolichen97 added a commit that referenced this pull request Aug 9, 2026
)

The 0.2.0 section was written in #2305, the commit that became v0.2.0-rc.4.
Seventy-one commits have landed on main since, nineteen of them feat:, and
the section was never revisited. It therefore both omitted shipped features
and described one that no longer exists as written.

The wrong entry mattered most: the Webhooks bullet told the reader to manage
inbound automation "from Settings", but #2343 moved that page behind a
per-device Preview pages toggle under Developer and hides it by default. A
0.2.0 user following the release notes would have gone looking for a page
that is not there.

Added, all from the rc.6 range: opt-in Slack setup and the multi-channel
repositioning (#2340), Telegram multi-account (#2203) and inbound
attachments (#2201), sub-agent completions reaching non-Slack parents
(#2352), Discord reply continuation (#2326), Slack OPTIONS as a control
(#1467), the Agent Templates two-pane inspector, project-local agent
discovery (#2167), send-a-copy-to-another-instance, Jira and setting link
chips (#2019, #1907), CJK emphasis rendering, the MCP Apps switch (#2293,
#2337), the Connections provider registry (#2285), GitHub Enterprise
Server support in Code Review Sage (#2154), operator notes on user deny
patterns (#2341), the locked git-publish floor rules (#2369), the
persist-or-refuse boot guard (#2279), and the turn-ceiling bounds on the
approval and stall windows (#2372, #2373).

Scope is exactly 5fe4bd5..ab20b4e, the range v0.2.0-rc.6 ships. The
three commits main carries beyond rc.6, including meeting deletion (#2268),
belong to the next release and are deliberately not described here.
bolichen97 added a commit that referenced this pull request Aug 9, 2026
…c.7 delta (#2413)

#2412 reconciled the 0.2.0 section against what rc.6 ships. Cutting rc.7 from
main moves the target, so this closes the remaining gap and surfaces a larger
omission found while checking it.

Four built-in apps ship today and appear in NO release's notes at all:
Meetings (#1080), Papyrus (#1077), PPTX Maker (#1079), and Mochi (#1258). All
four landed in the 0.1.2-to-0.2.0 window, 0.1.2's list did not include them,
and 0.2.0's heading claimed "Four new built-in apps" while eight had in fact
arrived. A user can install and enable any of them from the App Store, so the
notes were hiding shipped functionality rather than merely lagging. Since
0.1.2's notes are already published and immutable, 0.2.0 is the only place
they can surface. The heading now reads eight, and the entry states plainly
that all of them are opt-in.

Also documents the three user-visible changes main carries beyond rc.6, which
rc.7 will therefore ship: meeting deletion (#2268), the wait tool's live
countdown and early-end button (#2331), and the MCP OAuth fix that emits
oauthScopes and oauth.clientId in the fields kiro-cli reads.

Note the structural problem this is the second pass on: a periodic
reconciliation drifts again the moment another user-visible PR merges. The
durable fix is requiring a changelog line in the PR that introduces the
change, not a sweep before each release.
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
let users remove inactive meetings and their local notes from the meetings list while protecting live sessions from destructive races.
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…rodotdev#2412)

The 0.2.0 section was written in kirodotdev#2305, the commit that became v0.2.0-rc.4.
Seventy-one commits have landed on main since, nineteen of them feat:, and
the section was never revisited. It therefore both omitted shipped features
and described one that no longer exists as written.

The wrong entry mattered most: the Webhooks bullet told the reader to manage
inbound automation "from Settings", but kirodotdev#2343 moved that page behind a
per-device Preview pages toggle under Developer and hides it by default. A
0.2.0 user following the release notes would have gone looking for a page
that is not there.

Added, all from the rc.6 range: opt-in Slack setup and the multi-channel
repositioning (kirodotdev#2340), Telegram multi-account (kirodotdev#2203) and inbound
attachments (kirodotdev#2201), sub-agent completions reaching non-Slack parents
(kirodotdev#2352), Discord reply continuation (kirodotdev#2326), Slack OPTIONS as a control
(kirodotdev#1467), the Agent Templates two-pane inspector, project-local agent
discovery (kirodotdev#2167), send-a-copy-to-another-instance, Jira and setting link
chips (kirodotdev#2019, kirodotdev#1907), CJK emphasis rendering, the MCP Apps switch (kirodotdev#2293,
kirodotdev#2337), the Connections provider registry (kirodotdev#2285), GitHub Enterprise
Server support in Code Review Sage (kirodotdev#2154), operator notes on user deny
patterns (kirodotdev#2341), the locked git-publish floor rules (kirodotdev#2369), the
persist-or-refuse boot guard (kirodotdev#2279), and the turn-ceiling bounds on the
approval and stall windows (kirodotdev#2372, kirodotdev#2373).

Scope is exactly 5fe4bd5..ab20b4e, the range v0.2.0-rc.6 ships. The
three commits main carries beyond rc.6, including meeting deletion (kirodotdev#2268),
belong to the next release and are deliberately not described here.
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…c.7 delta (kirodotdev#2413)

kirodotdev#2412 reconciled the 0.2.0 section against what rc.6 ships. Cutting rc.7 from
main moves the target, so this closes the remaining gap and surfaces a larger
omission found while checking it.

Four built-in apps ship today and appear in NO release's notes at all:
Meetings (kirodotdev#1080), Papyrus (kirodotdev#1077), PPTX Maker (kirodotdev#1079), and Mochi (kirodotdev#1258). All
four landed in the 0.1.2-to-0.2.0 window, 0.1.2's list did not include them,
and 0.2.0's heading claimed "Four new built-in apps" while eight had in fact
arrived. A user can install and enable any of them from the App Store, so the
notes were hiding shipped functionality rather than merely lagging. Since
0.1.2's notes are already published and immutable, 0.2.0 is the only place
they can surface. The heading now reads eight, and the entry states plainly
that all of them are opt-in.

Also documents the three user-visible changes main carries beyond rc.6, which
rc.7 will therefore ship: meeting deletion (kirodotdev#2268), the wait tool's live
countdown and early-end button (kirodotdev#2331), and the MCP OAuth fix that emits
oauthScopes and oauth.clientId in the fields kiro-cli reads.

Note the structural problem this is the second pass on: a periodic
reconciliation drifts again the moment another user-visible PR merges. The
durable fix is requiring a changelog line in the PR that introduces the
change, not a sweep before each release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants