Repository navigation
Let a run start from a workspace with only the given files - #1159
TomNaessens wants to merge 1 commit into
Conversation
db641b5 to
f2e5585
Compare
There was a problem hiding this comment.
馃煛 Changes recommended
The unresolved critical cancellation and moderate large-file recovery issues block approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds support for starting runs with a workspace containing only explicitly supplied files.
Changes:
- Stages text and binary files before execution.
- Adds workspace clearing through the backend and Python worker.
- Adds tests for isolation, recovery, stopping, and write failures.
Review findings include a critical cancellation issue after preparation exceeds the stop timeout and a moderate recovery issue affecting files larger than 1 MiB.
File summaries
| File | Summary |
|---|---|
test/__tests__/state/RuntimeRecovery.test.ts |
Tests recovery, stopping, and file preparation. |
test/__tests__/state/Files.test.ts |
Tests isolated workspaces and text/binary files. |
src/frontend/state/Runner.ts |
Stages supplied files; contains the reported cancellation and recovery issues. |
src/backend/workers/python/PythonWorker.ts |
Exposes workspace clearing to Python. |
src/backend/workers/python/papyros/papyros.py |
Resets and clears the workspace. |
src/backend/Backend.ts |
Adds the workspace-clearing API. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
f2e5585 to
045e3eb
Compare
Runner.start takes an optional list of files. When given, the run clears the workspace and writes those files during its Loading phase, before the code runs, and forgets the files handed to provideFiles, so a runtime recovery does not write them back. Dodona's code playgrounds use this to start every run from the same filesystem: writing the files before start() left a window in which the runner was still Ready. start() also skips the code when Stop was pressed while it was still loading, instead of running the program anyway. The worker's clear_workspace moves the working directory back into the workspace before removing every entry, because a program may have changed into a subdirectory that is about to be removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
045e3eb to
c2d97af
Compare
|
馃 Folded into #1168 so the playground stack reads as one series: |
This pull request lets a run start from a workspace that holds only given files:
runner.start(mode, files).For more context & screenshots, see https://github.com/dodona-edu/dodona/pull/9211.
Details
Dodona's code playgrounds get a
filesattribute that loads data files (for examplegrades.txt) into the workspace, and every Run has to start from the same filesystem. Doing that with separate calls beforestart()left a window in which the runner was still Ready, so the playground had to track its own preparation state and a second block could still start in between. Writing the files inside the run's Loading phase closes that window: the existingstate !== Readychecks and the Stopping path cover it.Runner.start(mode?, files?): whenfilesis given, it forgets the files handed toprovideFiles(a runtime recovery must not write them back), clears the workspace and writes each file withupdateFile, all inside the run'stry, so a failed write is reported like a failed run. Withoutfiles, the workspace is left as is.start()now skips the code when Stop was pressed while it was still loading. Before, the program ran anyway, becauseinterrupt()does nothing while the client is idle.Backend.clearWorkspace/PythonWorker.clearWorkspace/clear_workspaceinpapyros.py: moves the working directory back to/home/pyodide/workspace(a program may haveos.chdired into a subdirectory that is about to be removed) and removes every entry.io.filesis updated by the files event at the end of the run.Manual testing instructions
PATH="$(mise where python)/bin:$PATH" yarn setupto rebuild the Python package.yarn vitest run test/__tests__/state/Files.test.ts test/__tests__/state/RuntimeRecovery.test.ts --exclude '**/.claude/**': the new cases check that a run given files sees only those files (text and binary), starts in the workspace directory after anos.chdir, lists exactly those files afterwards, writes them before the code runs, does not run when stopped during the writes, reports a file that cannot be written, and that a recovery does not replay earlier provided files.Checklist
Files.test.ts, four inRuntimeRecovery.test.ts馃 Generated with Claude Code