microsoft / microsoft/skill-recorder

[Medium] deleteSession can race in-flight background post-processing

Open
#13 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
4k
Forks
413
Avg merge
3h 16m
Merged PRs (30d)
10

Description

Severity

Medium — race between deletion and background work; can corrupt/partially write into a deleted session dir.

Location

  • electron/ipc.ts:196 (deleteSession handler) validates sessionId and refuses deletion only while the session is actively recording (lines 198–199) — it does not account for post-processing.
  • electron/recorder/controller.ts:517 (schedulePostProcess) runs describer/frame work after stop() returns, in the background.

Mechanism

Right after Stop, post-processing (analysis, frame extraction) is still running for the session. Nothing prevents the user from deleting that same session in the same window, so background tasks can write frames/analysis into a directory that deleteSession is concurrently removing.

Impact

Orphaned/partial files, write-after-delete errors, or a resurrected half-session directory.

Suggested fix

Track in-flight post-processing per session id; have deleteSession cancel (or await) it before removing the directory — or refuse deletion with a clear message while post-processing is active (mirroring the active-recording guard).

Regression test to add (npm test)

Unit test: start a fake post-process task for a session, call deleteSession while it's pending, and assert the delete either cancels/awaits it and that no write lands after the directory is removed. Deterministic; not an evals/ concern.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the deleteSession handler in electron/ipc.ts around line 196 and schedulePostProcess in electron/recorder/controller.ts around line 517. Run npm test and add the described deterministic unit test with a pending fake post-processing task. Done means deletion safely cancels or awaits post-processing, with no write occurring after the session directory is removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
electron, typescript
Domain
desktop, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.