Skip to content

code-gen: add the restore-side stash screens and close the workspace edge cases left after #149 #173

Description

@sugat009

Describe the issue

The #149 QA and review found these items in the snapshot and rollback of cht-core (src/layers/code-gen/modules/claude-code-cli/workspace.ts) and in the CLI (src/cli/stash-screen.ts, dev.ts, full.ts). We did not fix them in #149, so that it could merge, and its PR body lists them. Most of them need rare repo states. Items 1, 2 and 6 can lose an operator file, but only in very rare states. Items 9, 12 and 17 can happen on a normal checkout, and none of those three loses a file.

  1. When git cannot read the stash's untracked files, the printed recovery steps put git reset --hard before the step that moves the operator's files aside. This can happen when a git object disappears immediately after the push. In one case (a tracked dir replaced by a file inside a read-only dir), that reset deletes the only copy of the file.
  2. If the operator replaces the dir of a submodule that has ignore=all (or diff.ignoreSubmodules=all) by a file, git status hides the change. Then the reset deletes the file, and the rollback reports ok.
  3. With core.ignoreStat=true, the rollback's reset marks the files that the session edited as assume-unchanged. The next run then refuses them, although the operator never set the flag.
  4. The rollback removes the session's files, but not the dirs that the session created. When a stashed operator file is at such a dir path, the restore fails. The work stays in the stash, but the printed restore step also fails until the operator removes the empty dir.
  5. Two refusal texts name the wrong cause. One text calls an intent-to-add file that replaced a tracked dir "an ignored file". The other text calls a staged file that a symlink then replaced "a staged symbolic link". Nothing changes in the tree.
  6. A tracked dir that the operator made unreadable (chmod 000) hides a tracked delete from git status. The later reset-failure steps can then delete an ignored operator file.
  7. A snapshot and rollback cycle drops an intent-to-add entry whose file was deleted.
  8. Test gaps: 41 of 107 code mutations survive the suite. Most are in four places. These are the error wraps of the reads before the stash, the log prefix of printed lines, the blocker walk order and the read-failure branches.
  9. When the rollback's git stash drop fails after a good restore, the work is back, and our entry stays as an announced spare copy. The rest of the run does not skip it. The next snapshot of the same run refuses because of it. So claude-code-cli stops with exit 1, and cht-agent skips the compile gate for the rest of the run. Before the stash policy, CHT_AGENT_IGNORE_LEAKED_STASH=true let a headless run continue. No work is lost.
  10. The stash screens ("I handled it myself", "Retry", "Abort") exist only where the snapshot's push fails, and other failures still halt with printed steps. These are a failed undo, a stash list that stays unreadable after the push, a failed restore at the rollback and a failed drop. A first version of the restore-side screens is in the reverted commits f5c0e4f, 67316b1, d938a14 and c050440 of fix(#140): make the code-gen workspace cycle safer on a dirty cht-core checkout #149. We reverted it because a choice after a screen wait could write over two kinds of change that the operator made during that wait. git stash pop --index refuses both: an edit to a file that the stash holds, and an untracked file at a path that the stash adds. Also, a screen that named such changes printed a git reset --hard step that discarded them.
  11. With submodule.recurse=true in the git config, the undo of a partial push discards the content of a dirty submodule. Then the undo says that it put the work back.
  12. At the rollback, a read-only dir can make git stash apply --index skip an operator delete and still exit 0. The deleted file comes back, and the rollback drops our stash.
  13. A session file blocks the restore when it is at a stashed path that HEAD's ignore rules match. The run halts, and the work stays in the stash.
  14. In rare restore-side halts, git refuses the printed "Restore it" step until the operator acts on a path that git names. After a failed undo, the operator must move aside the untracked copy of a staged new file or of a rename target. After a partial push with an unreadable stash list, the operator must first reset a partly stashed path. The work stays in the stash (24 of 504 halts in the fix(#140): make the code-gen workspace cycle safer on a dirty cht-core checkout #149 QA sweep).
  15. After a screen accepts a spare copy of ours, a later halt in the same snapshot does not name that spare again. The next run's start check shows it.
  16. At a push-side screen whose cause stays, each "I handled it myself" or Retry can leave one more announced spare copy.
  17. The screen checks only process.stdin.isTTY. With stdout redirected to a file, the menu goes to the file while the terminal waits. With stdout piped (for example through tee), the shutdown handler takes the first Ctrl-C at a screen during the run. The screen does not detect a terminal stdin that already reached its end.
  18. When the Claude session fails and the rollback then halts, dev:run and full print the halt but not the session's error. The error stays on the halt as its cause only.
  19. Code that the revert left: StashFailure.step lists undo, restore and drop, which no code uses. The WorkspaceCallOptions doc does not say that rollbackChtCore reads only logPrefix. Two compile-gate comments describe a restore-side choice. The gate's rollback Abort branch and its test cannot run now.
  20. Test gaps in the stash policy: no test checks the exit code of dev.ts and full.ts (no spec loads the entry points). No test checks the terminal detection (terminalIo) or the compile gate with the real choice loop. 4 older mutations also survive.

Describe the improvement you'd like

  1. Print the move-aside step before the reset step.
  2. Refuse, before the stash, a submodule entry whose path is not a dir.
  3. Add core.ignoreStat=false to the hardened git config of cht-agent's own git calls.
  4. After the clean, remove each parent dir of a cleaned path when that dir is now empty. Stop at the top level, at a baseline entry or at a dir that is not empty. Or name the dir in the recovery steps.
  5. Correct the two texts.
  6. Treat a status whose stderr has "Permission denied" as incomplete, and refuse before the stash.
  7. Refuse intent-to-add entries before the stash, or keep them.
  8. Add one table-driven test for the reads before the stash, and a prefix test for each code path that prints. Also add tests for two-level blocker shapes.
  9. In popStep, call onSpareStash when the drop keeps our entry, as the snapshot's undo does.
  10. Add the restore-side screens back, with one rule: a choice after a screen wait writes nothing wherever git stash pop --index refuses.
  • Then cht-agent shows the screen again and names each such path, up to a limit.
  • Check HEAD before any write.
  • No false refusal: with no change during the wait, the choice writes the work back as it does today. This is also true after the operator ran git stash apply with or without --index.
  • A screen that names such changes must not print a git reset --hard step without a warning.
  • The fix(#140): make the code-gen workspace cycle safer on a dirty cht-core checkout #149 QA tested a rule (W is our stash commit): refuse (A and B and E) or (C and D and F) or G. A, B and E are the paths whose tree differs from W, W^1 and W^2, in that order. C, D and F are the paths whose index differs from W^2, W^1 and W, in that order. G is the set of untracked files at paths that W or W^2 adds, with content other than W's. The rule refused nothing in 187 normal flows and named every case that git refuses.
  1. Set submodule.recurse=false in the hardened git config (one line).
  2. After the restore, check that each path that the stash deleted is absent. If one is back, keep the stash.
  3. Name the ignored path in the recovery steps, or move it aside before the restore.
  4. Add the necessary step to the printed recovery steps, or move the named file aside before the restore.
  5. Name every accepted spare in each later halt of the snapshot.
  6. Keep at most one spare per screen: try the drop of the proven spare again before the snapshot runs again.
  7. Ask only when stdin and stdout are terminals, or write the menu to stderr. Give Abort when stdin is at its end. Let the screen take Ctrl-C when stdout is piped.
  8. Print the session error once, also when it becomes the halt's cause (withCause in claude-code-cli/index.ts). Or, in reportRunHalt, print the cause of a halt when that cause is not a halt.
  9. Narrow StashFailure.step to push, or mark the other values as reserved. Correct the doc and the two comments. Remove the unused Abort branch with its test, or keep them for item 10.
  10. Add the tests. For example, move the body of main() in dev.ts and full.ts into a module that a spec can load.

Acceptance criteria:

  • A real-git test for each of items 1 to 7 and 9 to 16 that fails before the fix.
  • Item 10: for each case that git stash pop --index refuses, a test shows that the choice writes nothing. For each screen, a normal-flow test shows that the choice writes the work back.
  • No mutation from items 8 and 20 survives, except the equivalent mutations.

Describe alternatives you've considered

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority: 3 - LowCan be dropped from the release.Type: BugFix something that isn't working as intended.

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions