Conversation
ad7bfe8 to
76601e7
Compare
There was a problem hiding this comment.
[P2] Recheck the active mode after an asynchronous cleanup confirmation (extensions/workspace-cleanup-guard/workspace-provenance.ts)
In ask mode, before() awaits confirmDelete([]) and then returns allow without checking whether setMode() changed the policy while the dialog was open. The setup-change event applies a new mode immediately. If the user switches from ask to enforce while an opaque cleanup confirmation is pending, accepting that old dialog still admits a command that enforce must block; the later setMode() clear does not invalidate the awaiting call. The same stale decision can occur across the later protected-path confirmation. Capture a mode generation or otherwise re-evaluate after each await that can cross a policy change, and add a deferred-confirmation regression for ask to enforce during an opaque or protected-path confirmation.
Reviewed exact head 76601e7 against main ad68e44. CI is green, but the mode-change tests only switch modes before starting before().
76601e7 to
ff315b0
Compare
|
@tt-a1i Addressed in ff315b0. setMode() now advances a generation, and in-flight cleanup checks fail closed if the mode changes while filesystem inspection or an async confirmation is pending. I added deferred-confirmation regressions for both opaque cleanup (confirmDelete([])) and protected-path confirmation: switching ask → enforce before approving now blocks in both cases. Focused tests passed (149/149), bun run check passed, and GitHub CI passed on Node 22, 24, 26, Web E2E, and Windows Background terminals. The local bun run test did not produce a final summary on Windows after more than two minutes, so I interrupted it; the remote CI run completed successfully. Please re-review. |
2b9c878 to
1af8e51
Compare
1af8e51 to
6290523
Compare
Problem
Issue #625 reports that
workspace-cleanup-guardblocks opaque deletion commands without a configuration path for an explicit decision. The guard currently has no mode setting. Closes #625.Value
Users can choose a confirmation step for opaque cleanup commands or disable this guard, while existing installations keep the current fail-closed behavior by default.
Approach
workspaceCleanupGuardto the existing/openpi-setupconfiguration withenforceas the default andask/offas alternatives.ask, send opaque cleanup requests through Pi's existing confirmation callback with an empty path list and dedicated confirmation text. Literal, verifiable paths retain their existing handling.off, skip this guard. Clear tracked provenance when the mode changes so files observed under another policy are not treated as verified scratch.rmtext classification addressed by PR fix(workspace-cleanup-guard): keep quoted rm text native in composite commands #624 or add a global permission policy.Validation
mainat1d36e00dc1d8d42162c7969f7d6919718d239aad, including merged PR fix(setup): make session_start diagnostics actionable #633. README had one documentation conflict; both the main-branch Web settings text and cleanup-guard documentation were retained. No code conflicts.git diff --check origin/main...HEAD— passed at candidate6290523836ec7ac04da1c054b0116a6b05302303; net PR diff remains 12 files, 301 insertions / 24 deletions.bun run check— passed, including config/docs contracts, discipline checks, Web production build, formatting, lint, and TypeScript. The build emitted the existing large-chunk advisory.bun run test— 2,153 passed, 5 failed, 14 skipped. All five failures involve Windows symlink cases:tests/web/git-review.test.ts,tests/web/transcript-search.test.ts(three cases, including one follow-on assertion after symlink creation failed), andtests/web/turn-changes.test.ts; creation returnedEPERM. The real Pi setup-writer provider integration test passed in isolation (1/1).6290523836ec7ac04da1c054b0116a6b05302303, including Windows runtime/UI, Node 22.19 / 24 / 26, and both Web E2E shards.Impact
Existing configurations without this field resolve to
enforce, preserving current behavior.asklets users approve an opaque command whose deletion targets cannot be verified;offdisables this extension's guard. Mode changes apply in the active session, clear its provenance map, and invalidate pending decisions, which can require a later confirmation for files previously tracked as session-created scratch. No global permission behavior changes.