Repository navigation
fuzz(restore): diskfull app create probe runs New Workspace as a person - #30531
Conversation
…ction.run newWorkspace) on the full disk must leave no ghost in the app now or after a relaunch, and must show a toast or dialog; a failed probe is a violation
…newWorkspace (the toolbar button's); an unknown action makes the probe 'not run', never fail or pass; the visible-refusal reader reads debug.toast's per-window toasts and refusals
…ab (WorkspaceActionCatalog)
…a CLI create (margin 0 if needed), and is 'not run' when no refusal happens or the app's create is accepted
….toast perform, origin user), so it gets the refusal a person gets
…eeds a typed disk-full refusal of a CLI create, counts only a new refusal notice about the create (multiset diff), is 'not run' when a view cannot be read (also after the relaunch), fails on no app answer instead of crashing, turns an unmounted image into harness_invalid, closes an accepted create, trims the filler back to --disk-margin-mb, checks the lease and writes its record and violations.json for every verdict; tests cover each path
GitHub Actions
Inactive
|
Review (claude-review-subagent): FIX at 0c64846 Static review only (diff read against origin/feat-cmux-next, plus The normal path works: CLI refused,
Non-blocking:
|
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Review (claude-review-subagent): SHIP at 1c3b15b Re-review of the repair delta 0c64846..1c3b15b, with the whole probe read again. Static review only: All six blocking findings from my FIX at 0c64846 are fixed:
The earlier non-blocking findings 7-11 are also fixed: the filler is trimmed back, the probe checks the time left before it starts, an accepted create is closed, every verdict writes I found no path that gives a false pass, crashes the run, or leaves the image mounted or the filler in place. Non-blocking:
|
lawrencecchen
added
the
ready-to-land
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/fuzz/restore/test_restore_fuzz.py">
<violation number="1" location="scripts/fuzz/restore/test_restore_fuzz.py:490">
P2: This fixture does not verify that the probe invokes the person-facing New Workspace action: a regression to the wrong RPC method or payload would still pass every added test. Make the mock assert `method == "debug.toast"` and `params == {"perform": "newTab"}` before returning the configured response.</violation>
</file>
<file name="scripts/fuzz/restore/restore_fuzz.py">
<violation number="1" location="scripts/fuzz/restore/restore_fuzz.py:1449">
P2: `app_view` treats incomplete topology responses as readable, so the probe can judge an empty baseline instead of proving both views were read. Require explicit workspace-list shapes for the app and daemon before setting `readable` true.
(Based on your team's feedback about incomplete snapshot workspace lists.) [1391804a-5457-4c80-8025-dcfde26134cd]</violation>
<violation number="2" location="scripts/fuzz/restore/restore_fuzz.py:1482">
P2: The visible-refusal check counts any new notice mentioning workspace, storage, or create, including informational notices. Require refusal/error wording in addition to the create or disk-full context before treating the person-visible check as passed.</violation>
<violation number="3" location="scripts/fuzz/restore/restore_fuzz.py:1545">
P2: The probe only closes successful creates returned as `workspace_id`; a `created_workspace_id` response leaks the temporary workspace into later iterations. Read `created_workspace_id` first, with `workspace_id` as the compatibility fallback.
(Based on your team's feedback about workspace create response IDs.) [c35e470a-df15-44af-906f-a38221be6485]</violation>
<violation number="4" location="scripts/fuzz/restore/restore_fuzz.py:1578">
P2: This predicate can crash during a normal relaunch gap because it reads `app_state()` twice and indexes the second result. Capture one state per poll before checking its loaded flag.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| return {}, 0.1 | ||
|
|
||
| rf.cli = fake_cli | ||
| rf.rpc = lambda m, p=None, timeout=20: action if action is not None else {"performed": True} |
There was a problem hiding this comment.
P2: This fixture does not verify that the probe invokes the person-facing New Workspace action: a regression to the wrong RPC method or payload would still pass every added test. Make the mock assert method == "debug.toast" and params == {"perform": "newTab"} before returning the configured response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/fuzz/restore/test_restore_fuzz.py, line 490:
<comment>This fixture does not verify that the probe invokes the person-facing New Workspace action: a regression to the wrong RPC method or payload would still pass every added test. Make the mock assert `method == "debug.toast"` and `params == {"perform": "newTab"}` before returning the configured response.</comment>
<file context>
@@ -406,6 +406,162 @@ def test_stall_or_loss_in_not_run_fails(self):
+ return {}, 0.1
+
+ rf.cli = fake_cli
+ rf.rpc = lambda m, p=None, timeout=20: action if action is not None else {"performed": True}
+ rf.app_view = lambda: views.pop(0)
+ rf.stop_build_processes = lambda exclude=(): None
</file context>
| rf.rpc = lambda m, p=None, timeout=20: action if action is not None else {"performed": True} | |
| def fake_rpc(method, params=None, timeout=20): | |
| if method != "debug.toast" or params != {"perform": "newTab"}: | |
| raise AssertionError((method, params)) | |
| return action if action is not None else {"performed": True} | |
| rf.rpc = fake_rpc |
| counts as "no ghost": the probe makes it "not run".""" | ||
| snap = rpc("snapshot.get") | ||
| topo = snap.get("topology") if isinstance(snap, dict) else None | ||
| app_ids = {w.get("id") for w in (topo or {}).get("workspaces", []) if isinstance(w, dict) and w.get("id")} |
There was a problem hiding this comment.
P2: app_view treats incomplete topology responses as readable, so the probe can judge an empty baseline instead of proving both views were read. Require explicit workspace-list shapes for the app and daemon before setting readable true.
(Based on your team's feedback about incomplete snapshot workspace lists.) [1391804a-5457-4c80-8025-dcfde26134cd]
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/fuzz/restore/restore_fuzz.py, line 1449:
<comment>`app_view` treats incomplete topology responses as readable, so the probe can judge an empty baseline instead of proving both views were read. Require explicit workspace-list shapes for the app and daemon before setting `readable` true.
(Based on your team's feedback about incomplete snapshot workspace lists.) [1391804a-5457-4c80-8025-dcfde26134cd]</comment>
<file context>
@@ -1433,6 +1440,154 @@ def recover_after_space(target, old_daemons):
+ counts as "no ghost": the probe makes it "not run"."""
+ snap = rpc("snapshot.get")
+ topo = snap.get("topology") if isinstance(snap, dict) else None
+ app_ids = {w.get("id") for w in (topo or {}).get("workspaces", []) if isinstance(w, dict) and w.get("id")}
+ wsl, _ = cli("workspace", "list")
+ daemon_read = isinstance(value(wsl), list) or (isinstance(wsl, dict) and "error" not in wsl)
</file context>
| for t in after.get("texts") or []: # a multiset difference: the same refusal shown again is new | ||
| if old.get(t): | ||
| old[t] -= 1 | ||
| elif CREATE_REFUSAL_WORDS.search(t): |
There was a problem hiding this comment.
P2: The visible-refusal check counts any new notice mentioning workspace, storage, or create, including informational notices. Require refusal/error wording in addition to the create or disk-full context before treating the person-visible check as passed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/fuzz/restore/restore_fuzz.py, line 1482:
<comment>The visible-refusal check counts any new notice mentioning workspace, storage, or create, including informational notices. Require refusal/error wording in addition to the create or disk-full context before treating the person-visible check as passed.</comment>
<file context>
@@ -1433,6 +1440,154 @@ def recover_after_space(target, old_daemons):
+ for t in after.get("texts") or []: # a multiset difference: the same refusal shown again is new
+ if old.get(t):
+ old[t] -= 1
+ elif CREATE_REFUSAL_WORDS.search(t):
+ fresh.append(t)
+ return fresh
</file context>
| if disk_full_refusal(answer): | ||
| refused = answer | ||
| break | ||
| made = value(answer).get("workspace_id") if isinstance(value(answer), dict) else None |
There was a problem hiding this comment.
P2: The probe only closes successful creates returned as workspace_id; a created_workspace_id response leaks the temporary workspace into later iterations. Read created_workspace_id first, with workspace_id as the compatibility fallback.
(Based on your team's feedback about workspace create response IDs.) [c35e470a-df15-44af-906f-a38221be6485]
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/fuzz/restore/restore_fuzz.py, line 1545:
<comment>The probe only closes successful creates returned as `workspace_id`; a `created_workspace_id` response leaks the temporary workspace into later iterations. Read `created_workspace_id` first, with `workspace_id` as the compatibility fallback.
(Based on your team's feedback about workspace create response IDs.) [c35e470a-df15-44af-906f-a38221be6485]</comment>
<file context>
@@ -1433,6 +1440,154 @@ def recover_after_space(target, old_daemons):
+ if disk_full_refusal(answer):
+ refused = answer
+ break
+ made = value(answer).get("workspace_id") if isinstance(value(answer), dict) else None
+ if made:
+ cli("workspace", made, "close")
</file context>
| made = value(answer).get("workspace_id") if isinstance(value(answer), dict) else None | |
| made = (value(answer).get("created_workspace_id") or value(answer).get("workspace_id")) if isinstance(value(answer), dict) else None |
| "no refusal to judge"], action_answer=json.dumps(answer)[:400]) | ||
| stop_build_processes(exclude=set(APP.daemon_pids()) | set(APP.acpmux_pids())) | ||
| relaunched_ok = APP.relaunch() | ||
| loaded = relaunched_ok and wait(lambda: app_state() and app_state()[1], 30, 0.5) |
There was a problem hiding this comment.
P2: This predicate can crash during a normal relaunch gap because it reads app_state() twice and indexes the second result. Capture one state per poll before checking its loaded flag.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/fuzz/restore/restore_fuzz.py, line 1578:
<comment>This predicate can crash during a normal relaunch gap because it reads `app_state()` twice and indexes the second result. Capture one state per poll before checking its loaded flag.</comment>
<file context>
@@ -1433,6 +1440,154 @@ def recover_after_space(target, old_daemons):
+ "no refusal to judge"], action_answer=json.dumps(answer)[:400])
+ stop_build_processes(exclude=set(APP.daemon_pids()) | set(APP.acpmux_pids()))
+ relaunched_ok = APP.relaunch()
+ loaded = relaunched_ok and wait(lambda: app_state() and app_state()[1], 30, 0.5)
+ relaunched = app_view() if loaded else None
+ if not relaunched_ok:
</file context>
| loaded = relaunched_ok and wait(lambda: app_state() and app_state()[1], 30, 0.5) | |
| loaded = relaunched_ok and wait(lambda: (lambda state: state and state[1])(app_state()), 30, 0.5) |
github-actions
Bot
added
risk:normal
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Shipped in:
|
Summary
The diskfull restore fuzz (#30208) gets an app-path create probe. With the disk full, it runs New Workspace the way a person does (
debug.toast {perform:"newTab"}, origin user) and checks two things:Before the probe runs, the harness fills the disk until the daemon refuses a CLI create (margin 0 if it must). If no refusal comes, or the app's create is accepted, the probe is "not run". It is never a pass or a fail in that case. An unknown action id is also "not run".
Harness only:
scripts/fuzz/restore/. No product code, and no CI job, step or gate.Evidence (scheduler rounds, perf minis)
Changelog
none
Observed / Not run
test_restore_fuzz.pyfor this head (it is a harness unit test; the rounds ran the code paths).