Skip to content

feat(gs): sync pulls the dotfiles clone and installs 10-core.fish - #77

Merged
barbatdev merged 2 commits into
mainfrom
feat/gs-sync-dotfiles
Oct 21, 2026
Merged

barbatdev merged 2 commits into
mainfrom
feat/gs-sync-dotfiles

Conversation

@barbatdev

Copy link
Copy Markdown
Owner

Hosts kept hand-edited copies of 10-core.fish, and the Mac's Home Manager link was stale since 2026-10-08.

New dotfiles step in gs sync (after gentle-ai sync):

  • clone at $CORTEX_DOTFILES_DIR (default ~/dev/barbatdev/cortex/cortex-dotfiles); missing → skip line
  • local changes or not on main → skip line, clone untouched
  • clean on main → git pull --ff-only; a moved HEAD adds dotfiles (a -> b) to the restart hint
  • 10-core.fish: Home Manager link → home-manager switch --flake <clone>#<cfg> -b pre-gs (GS_HM_CONFIG, default jbarbat/jbarbat-linux); otherwise symlink to the clone, host copy backed up to .pre-gs
  • GS_SKIP_DOTFILES=1 skips it; never fails the sync

Tests: scripts/test-gs-sync.sh with a real local clone + bare origin (skip, no clone, pull + hint, backup + link, no-op rerun, dirty/branch untouched, Home Manager switch + GS_HM_CONFIG). shellcheck clean.

Refs #69

Hosts kept hand-edited copies of 10-core.fish and the Mac's Home Manager
link went stale for weeks. gs sync now fast-forwards a clean dotfiles clone
on main, then links 10-core.fish to it (backing up a host copy to .pre-gs)
or runs home-manager switch when Home Manager owns the file. Dirty clones,
other branches and missing clones are skipped with a status line.

Refs #69
@barbatdev
barbatdev merged commit c7c4992 into main Oct 21, 2026
1 check passed
@barbatdev
barbatdev deleted the feat/gs-sync-dotfiles branch October 21, 2026 17:55
@barbatdev

Copy link
Copy Markdown
Owner Author

Fleet review (glm)

Lens: requirements + adversarial. The diff matches the behavior described in the PR body: skip paths (GS_SKIP_DOTFILES, missing clone, dirty tree, non-main branch), pull --ff-only with FAILED line and early return, restart hint "dotfiles (a -> b)", Home Manager switch vs symlink with .pre-gs backup, and it never fails the sync (every path returns 0; only pull failure prints FAILED). It references #69 without closing it, and the issue's numbered proposals (1-5) are not claimed by this PR, so no acceptance-criterion contradiction. Tests cover all stated paths. Two non-blocking observations: untracked files count as "local changes" and GS_HM_STORE_PREFIX is undocumented in the example configs.

Suggestion (1)

  • GS_HM_STORE_PREFIX is used by the tests but not documented (fish/conf.d/99-local.fish.example:17): sync_dotfiles reads GS_HM_STORE_PREFIX (default /nix/store) to detect a Home Manager-owned link, and the new tests rely on overriding it, but neither 99-local.fish.example nor local/env.zsh.example mentions it (they document CORTEX_DOTFILES_DIR, GS_HM_CONFIG, and GS_SKIP_DOTFILES). A host whose Nix store is not at /nix/store would be silently relinked instead of switched.

Question (1)

  • Untracked files permanently skip the dotfiles step (bin/gs:288): git status --porcelain includes untracked entries, so a single .DS_Store or editor artifact dropped in the clone makes every future sync print "skipped (… has local changes)" until the user cleans it. This matches the PR body's "local changes … skip line", so it may be intended, but on a macOS host it is a common way for the step to silently stop materializing configs. Is treating untracked files as work in progress the intent, or should only tracked modifications count?

Verdict: ready to merge

@barbatdev

Copy link
Copy Markdown
Owner Author

Post-merge QA (glm)

New dotfiles step in gs sync (issue #69 scope item): pulls the clone only when clean and on main, installs 10-core.fish via symlink with a byte-preserving backup, or home-manager switch when the target links into the Nix store. Every failure path is a status line (never fails the sync), the skip env vars are documented in both example files, and the test suite covers skip/no-clone/pull+hint/backup+link/no-op/dirty/branch/HM-switch including GS_HM_CONFIG. Conservative choices verified: dirty or non-main clones are left untouched, a failed pull never relinks, and an existing pre-gs backup gets a timestamped name. No defects found.

No findings.

Verdict: clean

@barbatdev

Copy link
Copy Markdown
Owner Author

Post-merge QA (deepseek)

Lens: evidence + test design.

The new dotfiles step is the right shape: it refuses to touch a dirty clone or a clone off main, uses git pull --ff-only, distinguishes the Home Manager case by the /nix/store symlink before relinking, backs the host copy up to .pre-gs (second time to .pre-gs.<epoch>), and the scripts/test-gs-sync.sh additions are real evidence rather than descriptions — a local clone with a bare origin, an upstream move that proves the pull and the dotfiles (a -> b) hint, a byte-for-byte backup check, a no-op rerun, dirty and off-branch refusals, and both Home Manager switch invocations with GS_HM_CONFIG.

One defect and two evidence gaps. bin/gs runs under set -euo pipefail, but sync_dotfiles calls mkdir -p, mv, and ln -s unguarded and cmd_sync invokes it without || true, so any filesystem error in that step aborts the whole gs sync — the opposite of the step's advertised "never fails the sync" contract, and it skips the version report and the legacy-script cleanup that follow. The tests cannot see this because every shim exits 0. The Home Manager default-config assertion is also unanchored, so on the Linux runner it passes without ever checking which flake config was chosen.

Blocking (1)

  • Unguarded filesystem calls abort gs sync under set -euo pipefail (bin/gs:314): sync_dotfiles guards the git calls but not the installation half: mkdir -p "$(dirname "$target")" (line 314), mv "$target" "$backup" (line 318) and ln -s "$src" "$target" (line 320) all run bare, and cmd_sync calls sync_dotfiles at line 423 with no || true. With set -euo pipefail in force at the top of the file, the first failing call terminates the script, contradicting the PR's "never fails the sync" and the step comment, and skipping everything after it — the legacy upgrade-script cleanup, the version report, and the restart hint.
    • Failure scenario: A host whose ~/.config/fish is owned by root (created by an earlier sudo run) or whose ~/.config is on a read-only mount. gs sync completes the gentle-ai step, then mkdir -p "$HOME/.config/fish/conf.d" fails with EACCES, and gs exits non-zero with no dotfiles status line and no closing report, even though the dotfiles step is documented as best-effort. The same happens when mv/ln -s fail (target directory not writable by the user), where the target has already been moved aside.

Suggestion (2)

  • Home Manager default-config assertion is unanchored and passes on Linux (scripts/test-gs-sync.sh:246): The runner's uname is Linux, so the code selects jbarbat-linux. The assertion greps "^home-manager|switch --flake $dot#jbarbat" with no trailing anchor, so it is satisfied by the #jbarbat-linux prefix and would equally pass if the platform default were wrong. Anchor it (#jbarbat-linux$, or assert the exact logged line as the GS_HM_CONFIG case below already does), and add a case where home-manager exits non-zero — no shim ever fails, so the "never fails the sync" half of that branch is untested too.
  • Pull result and link result are reported separately from what actually happened (bin/gs:305): dot_change is set from the HEAD move before the fish install runs, and the 10-core.fish line prints linked unconditionally after ln -s. When the clone pulls new commits but $dir/fish/conf.d/10-core.fish is missing, the function returns without a 10-core.fish line at all, and when the link is correct but upstream moved, the hint correctly names dotfiles — but a run where the clone moved and the file was relinked reports only the commit pair. Reporting the link action independently of the pull would make the status line an accurate record of both steps.

Verdict: blocking

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