feat(gsd): enable safety mechanisms by default (snapshots, pre-merge checks) (#2678)
Flip two safety mechanisms from opt-in to opt-out so all users benefit from rollback protection and merge regression checks without manual configuration. - git.snapshots: false → true (creates recovery refs before destructive ops) - git.pre_merge_check: false → "auto" in solo mode (auto-detects test runner) Both remain configurable; users can explicitly disable with snapshots: false or pre_merge_check: false. Closes #2677
This commit is contained in:
parent
de600c1db0
commit
ff2c2605f3
7 changed files with 51 additions and 17 deletions
|
|
@ -374,8 +374,8 @@ git:
|
|||
auto_push: false # push commits to remote after committing
|
||||
push_branches: false # push milestone branch to remote
|
||||
remote: origin # git remote name
|
||||
snapshots: false # WIP snapshot commits during long tasks
|
||||
pre_merge_check: false # run checks before worktree merge (true/false/"auto")
|
||||
snapshots: true # WIP snapshot commits during long tasks
|
||||
pre_merge_check: auto # run checks before worktree merge (true/false/"auto")
|
||||
commit_type: feat # override conventional commit prefix
|
||||
main_branch: main # primary branch name
|
||||
merge_strategy: squash # how worktree branches merge: "squash" or "merge"
|
||||
|
|
@ -392,8 +392,8 @@ git:
|
|||
| `auto_push` | boolean | `false` | Push commits to remote after committing |
|
||||
| `push_branches` | boolean | `false` | Push milestone branch to remote |
|
||||
| `remote` | string | `"origin"` | Git remote name |
|
||||
| `snapshots` | boolean | `false` | WIP snapshot commits during long tasks |
|
||||
| `pre_merge_check` | bool/string | `false` | Run checks before merge (`true`/`false`/`"auto"`) |
|
||||
| `snapshots` | boolean | `true` | WIP snapshot commits during long tasks |
|
||||
| `pre_merge_check` | bool/string | `"auto"` | Run checks before merge (`true`/`false`/`"auto"`) |
|
||||
| `commit_type` | string | (inferred) | Override conventional commit prefix (`feat`, `fix`, `refactor`, `docs`, `test`, `chore`, `perf`, `ci`, `build`, `style`) |
|
||||
| `main_branch` | string | `"main"` | Primary branch name |
|
||||
| `merge_strategy` | string | `"squash"` | How worktree branches merge: `"squash"` (combine all commits) or `"merge"` (preserve individual commits) |
|
||||
|
|
|
|||
|
|
@ -390,7 +390,7 @@ async function configureGit(ctx: ExtensionCommandContext, prefs: Record<string,
|
|||
const gitBooleanFields = [
|
||||
{ key: "auto_push", label: "Auto-push commits after committing", defaultVal: false },
|
||||
{ key: "push_branches", label: "Push milestone branches to remote", defaultVal: false },
|
||||
{ key: "snapshots", label: "Create WIP snapshot commits during long tasks", defaultVal: false },
|
||||
{ key: "snapshots", label: "Create WIP snapshot commits during long tasks", defaultVal: true },
|
||||
] as const;
|
||||
|
||||
for (const field of gitBooleanFields) {
|
||||
|
|
@ -423,7 +423,7 @@ async function configureGit(ctx: ExtensionCommandContext, prefs: Record<string,
|
|||
// pre_merge_check
|
||||
const currentPreMerge = git.pre_merge_check !== undefined ? String(git.pre_merge_check) : "";
|
||||
const preMergeChoice = await ctx.ui.select(
|
||||
`Pre-merge check${currentPreMerge ? ` (current: ${currentPreMerge})` : " (default: false)"}:`,
|
||||
`Pre-merge check${currentPreMerge ? ` (current: ${currentPreMerge})` : " (default: auto)"}:`,
|
||||
["true", "false", "auto", "(keep current)"],
|
||||
);
|
||||
if (preMergeChoice && preMergeChoice !== "(keep current)") {
|
||||
|
|
@ -588,7 +588,7 @@ export async function configureMode(ctx: ExtensionCommandContext, prefs: Record<
|
|||
if (modeStr.startsWith("solo")) {
|
||||
prefs.mode = "solo";
|
||||
ctx.ui.notify(
|
||||
"Mode: solo — defaults: auto_push=true, push_branches=false, pre_merge_check=false, merge_strategy=squash, isolation=worktree, unique_milestone_ids=false",
|
||||
"Mode: solo — defaults: auto_push=true, push_branches=false, pre_merge_check=auto, merge_strategy=squash, isolation=worktree, unique_milestone_ids=false",
|
||||
"info",
|
||||
);
|
||||
} else if (modeStr.startsWith("team")) {
|
||||
|
|
|
|||
|
|
@ -126,8 +126,8 @@ Setting `prefer_skills: []` does **not** disable skill discovery — it just mea
|
|||
- `auto_push`: boolean — automatically push commits to the remote after committing. Default: `false`.
|
||||
- `push_branches`: boolean — push the milestone branch to the remote after commits. Default: `false`.
|
||||
- `remote`: string — git remote name to push to. Default: `"origin"`.
|
||||
- `snapshots`: boolean — create snapshot commits (WIP saves) during long-running tasks. Default: `false`.
|
||||
- `pre_merge_check`: boolean or `"auto"` — run pre-merge checks before merging a worktree back to the integration branch. `true` always runs, `false` never runs, `"auto"` runs when CI is detected. Default: `false`.
|
||||
- `snapshots`: boolean — create snapshot commits (WIP saves) during long-running tasks. Default: `true`.
|
||||
- `pre_merge_check`: boolean or `"auto"` — run pre-merge checks before merging a worktree back to the integration branch. `true` always runs, `false` never runs, `"auto"` runs when CI is detected. Default: `"auto"`.
|
||||
- `commit_type`: string — override the conventional commit type prefix. Must be one of: `feat`, `fix`, `refactor`, `docs`, `test`, `chore`, `perf`, `ci`, `build`, `style`. Default: inferred from diff content.
|
||||
- `main_branch`: string — the primary branch name for new git repos (e.g., `"main"`, `"master"`, `"trunk"`). Also used by `getMainBranch()` as the preferred branch when auto-detection is ambiguous. Default: `"main"`.
|
||||
- `merge_strategy`: `"squash"` or `"merge"` — controls how worktree branches are merged back. `"squash"` combines all commits into one; `"merge"` preserves individual commits. Default: `"squash"`.
|
||||
|
|
|
|||
|
|
@ -605,11 +605,12 @@ export class GitServiceImpl {
|
|||
|
||||
/**
|
||||
* Create a snapshot ref for the given label (typically a slice branch name).
|
||||
* Gated on prefs.snapshots === true. Ref path: refs/gsd/snapshots/<label>/<timestamp>
|
||||
* Enabled by default; opt out with prefs.snapshots === false.
|
||||
* Ref path: refs/gsd/snapshots/<label>/<timestamp>
|
||||
* The ref points at HEAD, capturing the current commit before destructive operations.
|
||||
*/
|
||||
createSnapshot(label: string): void {
|
||||
if (this.prefs.snapshots !== true) return;
|
||||
if (this.prefs.snapshots === false) return;
|
||||
|
||||
const now = new Date();
|
||||
const ts = now.getFullYear().toString()
|
||||
|
|
@ -631,7 +632,7 @@ export class GitServiceImpl {
|
|||
* Stub: to be implemented in T03.
|
||||
*/
|
||||
runPreMergeCheck(): PreMergeCheckResult {
|
||||
if (this.prefs.pre_merge_check === false || this.prefs.pre_merge_check === undefined) {
|
||||
if (this.prefs.pre_merge_check === false) {
|
||||
return { passed: true, skipped: true };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ export const MODE_DEFAULTS: Record<WorkflowMode, Partial<GSDPreferences>> = {
|
|||
git: {
|
||||
auto_push: true,
|
||||
push_branches: false,
|
||||
pre_merge_check: false,
|
||||
pre_merge_check: "auto",
|
||||
merge_strategy: "squash",
|
||||
isolation: "none",
|
||||
},
|
||||
|
|
|
|||
|
|
@ -635,11 +635,11 @@ describe('git-service', async () => {
|
|||
// S05: Enhanced features — snapshots, pre-merge checks
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
// ─── createSnapshot: prefs enabled ─────────────────────────────────────
|
||||
// ─── createSnapshot: default (enabled) ─────────────────────────────────
|
||||
|
||||
test('createSnapshot: enabled', () => {
|
||||
test('createSnapshot: enabled by default when prefs omitted', () => {
|
||||
const repo = initBranchTestRepo();
|
||||
const svc = new GitServiceImpl(repo, { snapshots: true });
|
||||
const svc = new GitServiceImpl(repo);
|
||||
|
||||
// Create a branch with a commit
|
||||
run("git checkout -b gsd/M001/S01", repo);
|
||||
|
|
@ -675,6 +675,39 @@ describe('git-service', async () => {
|
|||
rmSync(repo, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// ─── runPreMergeCheck: default (auto-detect) ──────────────────────────
|
||||
|
||||
test('runPreMergeCheck: auto-detects when prefs omitted', () => {
|
||||
const repo = initBranchTestRepo();
|
||||
createFile(repo, "package.json", JSON.stringify({
|
||||
name: "test-default",
|
||||
scripts: { test: 'node -e "process.exit(0)"' },
|
||||
}));
|
||||
run("git add -A", repo);
|
||||
run('git commit -m "add package.json"', repo);
|
||||
|
||||
// No pre_merge_check pref set — should auto-detect and run
|
||||
const svc = new GitServiceImpl(repo);
|
||||
const result: PreMergeCheckResult = svc.runPreMergeCheck();
|
||||
|
||||
assert.deepStrictEqual(result.passed, true, "runPreMergeCheck auto-detects and passes when prefs omitted");
|
||||
assert.ok(!result.skipped, "runPreMergeCheck is not skipped when prefs omitted and package.json exists");
|
||||
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('runPreMergeCheck: gracefully skips when prefs omitted and no package.json', () => {
|
||||
const repo = initBranchTestRepo();
|
||||
// No package.json — auto-detect should skip gracefully
|
||||
const svc = new GitServiceImpl(repo);
|
||||
const result: PreMergeCheckResult = svc.runPreMergeCheck();
|
||||
|
||||
assert.deepStrictEqual(result.passed, true, "runPreMergeCheck passes when no package.json (skip)");
|
||||
assert.deepStrictEqual(result.skipped, true, "runPreMergeCheck skips when no test runner detected");
|
||||
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// ─── runPreMergeCheck: pass ────────────────────────────────────────────
|
||||
|
||||
test('runPreMergeCheck: pass', () => {
|
||||
|
|
|
|||
|
|
@ -59,7 +59,7 @@ test("solo mode applies correct defaults", () => {
|
|||
const result = applyModeDefaults("solo", { mode: "solo" });
|
||||
assert.equal(result.git?.auto_push, true);
|
||||
assert.equal(result.git?.push_branches, false);
|
||||
assert.equal(result.git?.pre_merge_check, false);
|
||||
assert.equal(result.git?.pre_merge_check, "auto");
|
||||
assert.equal(result.git?.merge_strategy, "squash");
|
||||
assert.equal(result.git?.isolation, "none");
|
||||
assert.equal(result.unique_milestone_ids, false);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue