Add local --back step for the workshop engine - #21
Merged
Conversation
Local learners who advance without committing each step had no way to move back. Add `advance_step.py --back` (plus a `make back` alias, README docs, and a `back-workshop.yml` CI pipeline) that restores the learner's saved work from the previous step's snapshot, or rebuilds the canonical step files when no snapshot exists. Namespace the backup layout so the agent snapshot and repo-root overlay targets never collide: `step-<N>/travel_assistant/`, `step-<N>/_root/`, and a `backup.json` completion manifest written last. This makes root-name collisions structurally impossible (#1) and lets `--back` restore structurally rather than by name. Snapshots are now published atomically (stage -> copy -> manifest -> swap) and fully replace any prior snapshot, so files deleted on a revisit are never resurrected (#3). Also: gate `--back` restore on a valid manifest (legacy/partial backups degrade to a canonical rebuild with a clear warning), reject `_root` overlay targets that shadow protected repo paths, and make reset/back/step backup dirs unique within the same second. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fea011c8-1399-4f8a-a3bd-277408cae6c2
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+1028
to
+1029
| "Would back up travel_assistant/ (and workshop root files) to " | ||
| ".workshop_instance/workshop_backups/back-<timestamp>/.", |
Resolve conflicts between the local --back feature and main's reset-current / sync-template additions: - advance_step.py: keep both _back/_plan_back (this branch) and _relay_current/_plan_relay_current (main) as sibling functions; wire both --back and --reset-current into main(). Align main's reset-current backup with this branch's namespaced backup layout and _reserve_backup_dir uniqueness (reset-current-<NN>-<timestamp>, deduped within the same second). - Makefile / 00-intro.md / _push_to_advance.md: keep all local-flow commands (advance, back, reset, reset-current, preflight, sync-template). - test_advance_step.py: keep both test suites; update main's reset-current backup assertion to the namespaced travel_assistant/ path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fea011c8-1399-4f8a-a3bd-277408cae6c2
Address rubber-duck findings on the local --back backup engine: - Atomic publish: snapshot now renames the prior step-<N>/ aside, swaps the new snapshot in, and only deletes the old copy once the swap succeeds, with rollback on failure and staging cleanup on every failure path — so a mid-swap crash never leaves a step without a valid backup (#1). - Always capture the travel_assistant/ namespace, even a lone .gitkeep, so a root-only snapshot restores the placeholder instead of an empty dir (#2). - Protect repo machinery: _root overlay targets that resolve to MACHINERY_PATHS or .git are rejected up front, compared casefolded so case variants can't slip past on case-insensitive filesystems (#3). - Exact restore: clear each destination before copying so an undeclared stale target (e.g. left after a template sync) is replaced, not merged (#4). - Validate the backup manifest step is an integer before trusting it (#5). Add regression tests for each fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fea011c8-1399-4f8a-a3bd-277408cae6c2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Local learners who advance through the workshop without committing each step had no built-in way to move back a step. This adds a first-class
--backcommand to the local step engine, plus supporting docs, amake backalias, and a CI pipeline.Changes
New
advance_step.py --backworkshop_backups/back-<timestamp>/); errors at step 0.Namespaced backup layout (format v2) — addresses two deferred review findings:
travel_assistant/named after a root overlay target round-trips correctly._snapshot_current_steppublishes atomically (stage → copy → manifest → swap) and fully replaces any priorstep-<N>/snapshot, so files deleted on a revisit are never resurrected. The old snapshot is only removed once the replacement is complete (no destructive gap).Hardening (from the rubber-duck review)
--backrestore is gated on a valid completion manifest; legacy flattened / partial-write backups degrade gracefully to a canonical rebuild._rootoverlay targets that would shadow protected repo paths (travel_assistant,README.md,.workshop,.workshop_instance,.git) are rejected up front.reset/back/stepbackup dirs are reserved via exclusive mkdir with a-Nsuffix, so two backups within the same UTC second never merge.Docs & tooling
--backand adds themake backalias.back-workshop.ymlCI pipeline.Validation
pytest .workshop/scripts/tests→ 96 passed (8 new regression tests covering Add MIT License to the project #1 misroute, Create SECURITY.md for security reporting #3 revisit + empty-drop, legacy fallback, file root-target, protected-target rejection, reset uniqueness).python .workshop/scripts/lint_steps.py→ 0 failures (2 pre-existing warnings).