Skip to content

fix: NER-382 — attempt attach workspace drift guard - #123

Merged
freezscholte merged 2 commits into
mainfrom
feat/ner-382-workspace-drift-guard
Jul 6, 2026
Merged

freezscholte merged 2 commits into
mainfrom
feat/ner-382-workspace-drift-guard

Conversation

@freezscholte

@freezscholte freezscholte commented Jul 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

forge attempt attach silently discarded edits made inside a not-yet-attached attempt's workspace dir (.forge/worktrees/<id>) — the workspace path in attempt start's payload looked editable but was only a materialization target. Found dogfooding (ccx T2 spike). Fixes NER-382.

Commit 1 — the guard:

  • workspace_path is now qualified by an additive workspace_role: "materialization_target" field (start/attempt-start payloads + replay) with matching help/schema text.
  • attempt attach refuses with new typed WORKSPACE_DRIFT (drifted paths secret-redacted like DIRTY_WORKTREE) when the workspace dir no longer equals its recorded materialized_content_ref, unless --discard-workspace-changes is passed. The override discards workspace drift only — it can never bypass DIRTY_WORKTREE (tested).
  • Read-only equality check: refusal happens before any materialization write.
  • Integration tests incl. the original silent-loss repro now failing loudly.

Commit 2 — multi-persona review fixes (run 20260706-145749-963e80e5; 15 findings, 8 independently validated, 1 live-reproduced):

  • Root cause of both P1s: the drift walk used weaker exclusion semantics than the scanner that built the recorded tree — gitignored build artifacts became permanent, unclearable drift (reproduced against the binary). Fixed with a new read-only forge-content-native::workspace_equality primitive built on walk_worktree + tree_fingerprints; the hand-rolled store-side walk is deleted (−199 lines).
  • Private-labeled paths excluded from both sides of the equality (previously the advertised --discard flag would delete private files; validated composition).
  • Machine surface: attempt attach schema entry, structured override_flag/recovery_hint details keys, workspace_role on attempt list/show, replay injection for pre-upgrade rows.
  • Test hardening: gitignore-parity, deletion drift, symlink drift, chmod no-drift, override-vs-dirty-root, private-label exclusion, pre-upgrade replay.

Deferred by design → NER-383: four refusal-semantics decisions (crash-retry hinge, deleted-dir semantics, lock ordering, non-Unix symlink arm) with proposed defaults.

Provenance

Implementation produced in the contract-pilot experiment (branch experiment/ccx-spikes, experiments/ccx/RESULTS.md): frozen task contracts, fresh brief-only sessions, independently re-run acceptance gates, two blinded scorers, then the full /ce-code-review gate with per-finding validators. scripts/ci.sh green at both commits.

Verification

  • cargo fmt --all --check / cargo test --workspace (621 passed) / cargo clippy --workspace --all-targets -- -D warnings / scripts/ci.sh incl. e2e eval — all green, run independently after the fix round.
  • New-feature scenarios per CLAUDE.md: silent-loss repro → loud refusal; discard flag → documented discard; no-drift attach byte-identical; never-materialized skip; gitignore parity (the reproduced P1, now pinned); override cannot bypass DIRTY_WORKTREE.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GTeDdEu9Q4a96DdXK4tfju


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

freezscholte and others added 2 commits July 6, 2026 14:53
…E_DRIFT refusal, tests

attempt attach silently discarded pre-attach edits inside
.forge/worktrees/<attempt>/. This change (1) qualifies workspace_path as
a materialization target in start/attempt-start payloads (additive
workspace_role field + help text), (2) makes attach refuse with typed
WORKSPACE_DRIFT (drifted paths in details, secret-filtered) unless
--discard-workspace-changes is passed, via a read-only per-file equality
check against the recorded materialized_content_ref, and (3) adds
integration tests incl. the original silent-loss repro now failing loudly.

Produced in the ccx contract pilot (experiment/ccx-spikes,
experiments/ccx/RESULTS.md), Arm A stack; gates re-verified independently.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTeDdEu9Q4a96DdXK4tfju
…y primitive, private-label exclusion, machine-surface polish, test hardening

Multi-persona review (run 20260706-145749-963e80e5) found two validated
P1s sharing one root cause: the drift walk used weaker exclusion
semantics than the scanner that built the recorded tree (gitignored
build artifacts became permanent unclearable WORKSPACE_DRIFT — live
reproduced), and the store hand-parsed native tree JSON. Both fixed by
a new read-only forge-content-native workspace_equality primitive built
on walk_worktree + tree_fingerprints; the store-side walk is deleted.
Private-labeled paths are excluded from both sides of the equality (the
advertised --discard flag could previously delete private files).
Additive: attach schema entry, override_flag/recovery_hint details
keys, workspace_role on list/show, replay injection for pre-upgrade
rows. Seven new integration tests incl. gitignore-parity and
override-cannot-bypass-DIRTY_WORKTREE. Design decisions deferred to
NER-383.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTeDdEu9Q4a96DdXK4tfju
@freezscholte
freezscholte merged commit b82f244 into main Jul 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant