microsoft / microsoft/skill-recorder
[Medium] deleteSession can race in-flight background post-processing
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(deleteSessionhandler) validatessessionIdand 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 afterstop()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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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