feat(client): sync --force to escape the locally-modified skip
Managed items with local modifications were kept (skip + warn) with no escape hatch other than deleting the deployed file by hand — after an upstream change the deployed content silently stayed old, which reads exactly like a stale cache. --force turns the two skip cases into update (repository version wins) and remove (upstream deleted + modified). Unmanaged items and never- managed path occupancies stay untouched even with --force; the default behavior is unchanged.
This commit is contained in:
1 parent
2017329015
commit
1ab97bff43
6 files changed
+164
-14
No files matched your search
@@ -196,6 +196,11 @@ repository, the local manifest and the on-disk content:
|
||||
- upstream deleted & unmodified → **remove**
|
||||
- upstream deleted & modified → **skip + warn** — your version stays
|
||||
|
||||
With `--force` the two skip cases turn into **update** and **remove** —
|
||||
the repository version wins over local modifications. Unmanaged items
|
||||
(leave + warn) are never touched, even with `--force`; neither are paths
|
||||
occupied by items agent-lib never deployed.
|
||||
|
||||
Sync is idempotent (a second run is a byte-level no-op) and fails cleanly:
|
||||
an unreachable repository never leaves a half-deployed machine. The default
|
||||
branch is resolved from the remote's advertised `HEAD`, so a sync without
|
||||
@@ -275,7 +280,7 @@ flag that drives the non-zero exit code — suitable for headless drift checks.
|
||||
| `error: … include entry "x" not found upstream` | Typo, or the item was renamed/removed upstream. Fix the lockfile include list, or re-add with a corrected `--include`. |
|
||||
| `error: … collides with own skills "x"` | Deployed-name collision. Add `--rename <upstream>=<deployed>` (add) or fix the rename map (update). |
|
||||
| `validate` reports tree/lockfile divergence | Someone edited `external/` by hand. Re-run `vendor update <source>` to restore the pinned state. |
|
||||
| Skill didn't update, `skipped (local modifications kept): 1` | Working as designed — you edited the deployed item. Restore the company version by deleting the local copy and re-syncing. |
|
||||
| Skill didn't update, `skipped (local modifications kept): 1` | Working as designed — you edited the deployed item. Run `agent-lib sync --force` to overwrite local modifications with the repository version, or delete the local copy and re-sync. |
|
||||
| `status` exits 1 | Local modifications block repository updates (or occupy names the repo needs). See `status` output for the item list. |
|
||||
| Sync fails with clone/fetch error | Work repository unreachable, or token expired. Previous local state remains intact; fix config/network and re-run. |
|
||||
| No toast on Windows | Toasts need an interactive user session — the scheduled task must run as the logged-in user, not SYSTEM. |
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
func newSyncCmd() *cobra.Command {
|
||||
var configPath, repoURL, ref string
|
||||
var force bool
|
||||
cmd := &cobra.Command{
|
||||
Use: "sync",
|
||||
Short: "Pull the work repository and deploy its content to OpenCode",
|
||||
@@ -17,10 +18,13 @@ func newSyncCmd() *cobra.Command {
|
||||
"into the OpenCode runtime paths. The repository comes from --repo, " +
|
||||
"or from the config chain: --config, " + client.ConfigEnv + " env, the " +
|
||||
"user-level config, or the admin-protected platform default. " +
|
||||
"Idempotent: re-running changes nothing when content and disk match.",
|
||||
"Idempotent: re-running changes nothing when content and disk match. " +
|
||||
"Locally modified managed items are kept by default (skip + warn); " +
|
||||
"--force overwrites them with the repository version and removes " +
|
||||
"them after upstream deletion. Unmanaged items are never touched.",
|
||||
Args: cobra.NoArgs,
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
opts := client.SyncOptions{ConfigPath: configPath, RepoURL: repoURL, Ref: ref}
|
||||
opts := client.SyncOptions{ConfigPath: configPath, RepoURL: repoURL, Ref: ref, Force: force}
|
||||
if _, err := client.Sync(opts, cmd.OutOrStdout()); err != nil {
|
||||
return fmt.Errorf("sync: %w", err)
|
||||
}
|
||||
@@ -30,5 +34,6 @@ func newSyncCmd() *cobra.Command {
|
||||
cmd.Flags().StringVar(&configPath, "config", "", "client config path (default: "+client.ConfigEnv+" env, user config, or platform default)")
|
||||
cmd.Flags().StringVar(&repoURL, "repo", "", "work repository URL or path — overrides any config, works without one")
|
||||
cmd.Flags().StringVar(&ref, "ref", "", "branch or tag to pull (requires --repo)")
|
||||
cmd.Flags().BoolVar(&force, "force", false, "overwrite locally modified managed items with the repository version; unmanaged items are never touched")
|
||||
return cmd
|
||||
}
|
||||
@@ -47,8 +47,11 @@ type PlanEntry struct {
|
||||
// Plan is a pure function from (inventory, manifest, on-disk facts) to an
|
||||
// action plan. pathFor injects the target path mapping so the planner never
|
||||
// touches the filesystem or the environment. The plan is sorted by
|
||||
// (type, name) for deterministic execution and reporting.
|
||||
func Plan(items []PlannedItem, manifest *Manifest, disk map[string]DiskFact, pathFor func(typ, name string) string) []PlanEntry {
|
||||
// (type, name) for deterministic execution and reporting. With force set,
|
||||
// locally modified managed items are overwritten with the upstream version
|
||||
// (and removed after upstream deletion) instead of being skipped; items
|
||||
// agent-lib never managed are never touched, even with force.
|
||||
func Plan(items []PlannedItem, manifest *Manifest, disk map[string]DiskFact, pathFor func(typ, name string) string, force bool) []PlanEntry {
|
||||
invByKey := map[string]PlannedItem{}
|
||||
for _, it := range items {
|
||||
invByKey[manifestKey(it.Type, it.Name)] = it
|
||||
@@ -93,13 +96,15 @@ func Plan(items []PlannedItem, manifest *Manifest, disk map[string]DiskFact, pat
|
||||
emit(ActDeploy, it, "")
|
||||
}
|
||||
case inInv && it.Hash == rec.Hash:
|
||||
decideAgainstDisk(it, rec, fact, emit, "unchanged")
|
||||
decideAgainstDisk(it, rec, fact, emit, "unchanged", force)
|
||||
case inInv:
|
||||
decideAgainstDisk(it, rec, fact, emit, "changed")
|
||||
decideAgainstDisk(it, rec, fact, emit, "changed", force)
|
||||
case rec != nil && !fact.Exists:
|
||||
emit(ActRemove, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin}, "already gone from disk; dropping manifest record")
|
||||
case rec != nil && fact.Hash == rec.Hash:
|
||||
emit(ActRemove, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin}, "upstream deleted")
|
||||
case rec != nil && force:
|
||||
emit(ActRemove, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin}, "upstream deleted; forced removal of the locally modified copy")
|
||||
case rec != nil:
|
||||
emit(ActSkip, PlannedItem{Type: rec.Type, Name: rec.Name, Origin: rec.Origin},
|
||||
"upstream deleted but the local copy was modified; keeping it")
|
||||
@@ -111,7 +116,7 @@ func Plan(items []PlannedItem, manifest *Manifest, disk map[string]DiskFact, pat
|
||||
return plan
|
||||
}
|
||||
|
||||
func decideAgainstDisk(it PlannedItem, rec *ManifestItem, fact DiskFact, emit func(Action, PlannedItem, string), upstreamState string) {
|
||||
func decideAgainstDisk(it PlannedItem, rec *ManifestItem, fact DiskFact, emit func(Action, PlannedItem, string), upstreamState string, force bool) {
|
||||
switch {
|
||||
case !fact.Exists:
|
||||
emit(ActUpdate, it, "managed item missing on disk; restoring")
|
||||
@@ -121,6 +126,8 @@ func decideAgainstDisk(it PlannedItem, rec *ManifestItem, fact DiskFact, emit fu
|
||||
} else {
|
||||
emit(ActUpdate, it, "")
|
||||
}
|
||||
case force:
|
||||
emit(ActUpdate, it, "forced: locally modified copy overwritten")
|
||||
default:
|
||||
emit(ActSkip, it, "locally modified; keeping the local version (upstream "+upstreamState+")")
|
||||
}
|
||||
|
||||
@@ -98,7 +98,7 @@ func TestPlanMatrix(t *testing.T) {
|
||||
diskFacts[key] = DiskFact{Exists: true, Hash: hashMod}
|
||||
}
|
||||
|
||||
plan := Plan(items, manifest, diskFacts, testPathFor)
|
||||
plan := Plan(items, manifest, diskFacts, testPathFor, false)
|
||||
if tc.disk == diskNone && tc.up == upAbsent {
|
||||
if len(plan) != 0 {
|
||||
t.Fatalf("a key present nowhere must produce no entry, got %+v", plan)
|
||||
@@ -123,6 +123,54 @@ func TestPlanMatrix(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestPlanForceMatrix asserts how --force changes the skip cases: locally
|
||||
// modified managed items become updates (or removes after upstream
|
||||
// deletion), while never-managed occupancies stay skipped even with force.
|
||||
func TestPlanForceMatrix(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
up string // "changed", "unchanged", "gone"
|
||||
want Action
|
||||
}{
|
||||
{"modified/changed", "changed", ActUpdate},
|
||||
{"modified/unchanged", "unchanged", ActUpdate},
|
||||
{"modified/gone", "gone", ActRemove},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
key := manifestKey("skills", "alpha")
|
||||
var items []PlannedItem
|
||||
manifest := NewManifest()
|
||||
manifest.Items[key] = &ManifestItem{Name: "alpha", Type: "skills", Hash: hashV1}
|
||||
if tc.up != "gone" {
|
||||
hash := hashV2
|
||||
if tc.up == "unchanged" {
|
||||
hash = hashV1
|
||||
}
|
||||
items = append(items, PlannedItem{Type: "skills", Name: "alpha", RelPath: "skills/alpha", Hash: hash})
|
||||
}
|
||||
disk := map[string]DiskFact{key: {Exists: true, Hash: hashMod}}
|
||||
|
||||
plan := Plan(items, manifest, disk, testPathFor, true)
|
||||
if len(plan) != 1 {
|
||||
t.Fatalf("want exactly 1 entry, got %d: %+v", len(plan), plan)
|
||||
}
|
||||
if plan[0].Action != tc.want {
|
||||
t.Errorf("action = %s, want %s (reason: %s)", plan[0].Action, tc.want, plan[0].Reason)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("never-managed occupancy stays skipped", func(t *testing.T) {
|
||||
items := []PlannedItem{{Type: "skills", Name: "alpha", RelPath: "skills/alpha", Hash: hashV1}}
|
||||
disk := map[string]DiskFact{manifestKey("skills", "alpha"): {Exists: true, Hash: hashMod}}
|
||||
plan := Plan(items, NewManifest(), disk, testPathFor, true)
|
||||
if len(plan) != 1 || plan[0].Action != ActSkip {
|
||||
t.Fatalf("force must not deploy over a never-managed item: %+v", plan)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestPlanMixedFixture combines several states in one plan and asserts the
|
||||
// full deterministic output.
|
||||
func TestPlanMixedFixture(t *testing.T) {
|
||||
@@ -151,7 +199,7 @@ func TestPlanMixedFixture(t *testing.T) {
|
||||
"agents/helper": {Exists: true, Hash: hashV1}, // unmodified × unchanged
|
||||
}
|
||||
|
||||
plan := Plan(items, manifest, disk, testPathFor)
|
||||
plan := Plan(items, manifest, disk, testPathFor, false)
|
||||
got := map[string]Action{}
|
||||
for _, e := range plan {
|
||||
got[e.Type+"/"+e.Name] = e.Action
|
||||
@@ -195,8 +243,8 @@ func TestPlanPureGuarantee(t *testing.T) {
|
||||
manifest := manifestWith(map[string]*ManifestItem{"skills/b": {Name: "b", Type: "skills", Hash: hashV1}})
|
||||
disk := map[string]DiskFact{"skills/b": {Exists: true, Hash: hashV1}}
|
||||
|
||||
p1 := Plan(items, manifest, disk, testPathFor)
|
||||
p2 := Plan(items, manifest, disk, testPathFor)
|
||||
p1 := Plan(items, manifest, disk, testPathFor, false)
|
||||
p2 := Plan(items, manifest, disk, testPathFor, false)
|
||||
if len(p1) != len(p2) {
|
||||
t.Fatal("same inputs must yield same plan length")
|
||||
}
|
||||
|
||||
@@ -12,11 +12,13 @@ import (
|
||||
|
||||
// SyncOptions parameterize a client sync run. RepoURL (with optional Ref)
|
||||
// is the inline --repo override: it works without any config file and beats
|
||||
// a repo_url discovered in the config chain.
|
||||
// a repo_url discovered in the config chain. Force switches the planner
|
||||
// from skip+warn to overwrite for locally modified managed items.
|
||||
type SyncOptions struct {
|
||||
ConfigPath string
|
||||
RepoURL string
|
||||
Ref string
|
||||
Force bool
|
||||
}
|
||||
|
||||
// prepared is everything sync and status share: the pulled repository, the
|
||||
@@ -80,7 +82,7 @@ func prepare(opts SyncOptions) (*prepared, error) {
|
||||
path, _ := oc.DeployPath(typ, name)
|
||||
return path
|
||||
}
|
||||
plan := Plan(items, manifest, diskFacts(oc, manifest, items), pathFor)
|
||||
plan := Plan(items, manifest, diskFacts(oc, manifest, items), pathFor, opts.Force)
|
||||
for _, e := range plan {
|
||||
if !safePath(e.DstPath) {
|
||||
return nil, fmt.Errorf("refusing unsafe deploy path %q", e.DstPath)
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
package e2e
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestSyncForceOverwritesLocallyModified pins the --force escape hatch:
|
||||
// locally modified managed items are overwritten with the repository
|
||||
// version, upstream-deleted modified items are removed, and unmanaged
|
||||
// local items are never touched.
|
||||
func TestSyncForceOverwritesLocallyModified(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
env := newSyncEnv(t, h)
|
||||
buildWorkRepo(t, h, env.workRepo)
|
||||
env.writeConfig(t, env.workRepo, "")
|
||||
env.run(t)
|
||||
|
||||
// local drift on a managed item + upstream change to the same item
|
||||
deployed := filepath.Join(env.home, ".config/opencode/commands/deploy.md")
|
||||
os.WriteFile(deployed, []byte("---\nname: Deploy\n---\nlocal drift\n"), 0o644)
|
||||
upstream := filepath.Join(env.workRepo, "commands/deploy.md")
|
||||
os.WriteFile(upstream, []byte("---\nname: Deploy\ndescription: updated\n---\nnew body\n"), 0o644)
|
||||
|
||||
// an unmanaged local item that must survive --force untouched
|
||||
unmanaged := filepath.Join(env.home, ".config/opencode/commands/mine.md")
|
||||
os.MkdirAll(filepath.Dir(unmanaged), 0o755)
|
||||
os.WriteFile(unmanaged, []byte("---\nname: Mine\n---\nprecious\n"), 0o644)
|
||||
|
||||
h.git(t, env.workRepo, "add", "-A")
|
||||
h.git(t, env.workRepo, "commit", "-m", "update deploy command")
|
||||
|
||||
out, err := env.run(t)
|
||||
if err != nil {
|
||||
t.Fatalf("sync failed: %v\n%s", err, out)
|
||||
}
|
||||
if !contains(out, "skipped (local modifications kept): 1") {
|
||||
t.Errorf("default sync must keep the local modification:\n%s", out)
|
||||
}
|
||||
|
||||
out, err = env.run(t, "--force")
|
||||
if err != nil {
|
||||
t.Fatalf("sync --force failed: %v\n%s", err, out)
|
||||
}
|
||||
if !contains(out, "updated: 1") {
|
||||
t.Errorf("force must update the locally modified item:\n%s", out)
|
||||
}
|
||||
if got := readFile(t, deployed); !contains(got, "new body") {
|
||||
t.Error("force must overwrite the local drift with the repository version")
|
||||
}
|
||||
if got := readFile(t, unmanaged); !contains(got, "precious") {
|
||||
t.Error("force must never touch unmanaged local items")
|
||||
}
|
||||
|
||||
// upstream deletion of a locally modified item: force removes it
|
||||
os.WriteFile(deployed, []byte("---\nname: Deploy\n---\nmore drift\n"), 0o644)
|
||||
os.Remove(upstream)
|
||||
h.git(t, env.workRepo, "add", "-A")
|
||||
h.git(t, env.workRepo, "commit", "-m", "delete deploy command")
|
||||
|
||||
out, err = env.run(t)
|
||||
if err != nil {
|
||||
t.Fatalf("sync failed: %v\n%s", err, out)
|
||||
}
|
||||
if !contains(out, "skipped (local modifications kept): 1") {
|
||||
t.Errorf("default sync must keep the modified item after upstream deletion:\n%s", out)
|
||||
}
|
||||
if _, err := os.Stat(deployed); err != nil {
|
||||
t.Fatal("item must still exist before force")
|
||||
}
|
||||
|
||||
out, err = env.run(t, "--force")
|
||||
if err != nil {
|
||||
t.Fatalf("sync --force failed: %v\n%s", err, out)
|
||||
}
|
||||
if !contains(out, "removed: 1") {
|
||||
t.Errorf("force must remove the locally modified item after upstream deletion:\n%s", out)
|
||||
}
|
||||
if _, err := os.Stat(deployed); !os.IsNotExist(err) {
|
||||
t.Error("forced removal must delete the deployed file")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user