Skip to content

Commit 91986d6

Browse files
acmoreclaude
andauthored
feat(sync): allow nested local roots via a managed primary-folder ignore block (#157)
* feat(sync): allow nested local roots via a managed primary-folder ignore block An additional mapping's local root may now nest inside the primary mapping's local root (./results inside the repo) — the natural place for a results directory — instead of forcing an awkward sibling placement. Remote roots stay disjoint-only. okdev maintains a managed block in the primary root's .stignore ("// okdev:begin/end managed sync excludes"; syncthing comments are "//", not "#") holding one /rel-path pattern per nested child, so the subtree travels only through its own folder. Ordering is a hard guarantee, not a race: adding a mapping changes the sync config hash, which restarts the sync process chain, and the block is merged into the file BEFORE the local syncthing daemon starts — its first scan already honors the exclusion. Removing a mapping never silently widens the primary folder: the child folder is pruned from both syncthing instances and local files are kept, but the ignore entry is retained as a tombstone with an explanatory comment; re-inclusion is the user's explicit one-line deletion. Sync start prints a notice for newly retained entries, and status --details lists active and retained excludes. Other local overlaps (equal roots, nesting between additional mappings, an extra containing the primary) remain rejected. Kind e2e now runs the nested layout end-to-end: exclusion effective from first scan (no leak into /workspace), results flow through the child folder, and removal leaves a tombstone with the subtree still excluded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(sync): skill and troubleshooting coverage for directions and nested mappings - Skill: SKILL.md gets a sync-capabilities line; workflows.md gains the result-collection-via-sync recipe; troubleshooting.md explains the two by-design "not syncing" cases (direction contract, managed excludes / tombstones) before fault-hunting. - docs/troubleshooting.md: drop the stale single-mapping claim, add the same by-design guidance, and qualify the versioning section (directional mappings are structurally immune to cross-side overwrites). - command-reference: nested-local + tombstone semantics under `okdev sync`; the already-active no-op wording follows the new default-mode semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 36985e5 commit 91986d6

14 files changed

Lines changed: 485 additions & 42 deletions

docs/command-reference.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,8 @@ agents can react without launching a diagnostic chain on every blip:
306306
- Advanced command. Starts detached background sync by default; use `--foreground` for sync debugging, or explicit one-way sync (`up`/`down`).
307307
- By default each configured mapping syncs in its own `direction` (`spec.sync.paths[].direction`, falling back to `bi`); an explicit `--mode` forces that mode onto every mapping for the invocation. See the config manifest for choosing persistent directions — `down` makes the pod the authority so local writes can never clobber pod-generated results.
308308
- With multiple mappings, each becomes its own syncthing folder; the first (primary) mapping is the one shared to mesh receivers, and `sync reset-remote` clears only the primary remote.
309-
- For default `--mode bi`, no-op when background sync is already active for the session.
309+
- A mapping's local root may nest inside the primary root: okdev maintains a managed block in the primary root's `.stignore` excluding it from the primary folder (written before the sync daemon starts). Removing a mapping retains the entry as a tombstone so the subtree never silently joins the primary folder; sync start prints a notice and `status --details` lists active/retained excludes.
310+
- Without an explicit `--mode`, no-op when background sync is already active for the session.
310311
- `--background`: explicitly request detached background mode.
311312
- `--reset`: check local-to-hub sync and mesh receiver health, then reset only what is broken. Skips the local sync teardown when the primary sync is already healthy. For sessions with mesh receivers, probes each receiver and re-runs mesh setup only when broken or disconnected receivers are found.
312313
- `--reset --force` / `--reset -f`: unconditionally reset without health checks.

docs/config-manifest.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,15 +250,17 @@ spec:
250250
```yaml
251251
paths:
252252
- .:/workspace # primary: code, bidirectional
253-
- local: ../collected # sibling of the repo — local roots must
254-
remote: /data/results # not nest inside the primary mapping
253+
- local: ./collected # nested inside the repo — allowed:
254+
remote: /data/results # okdev excludes it from the primary folder
255255
direction: down # pod-generated results, pod is authority
256-
- local: ../datasets
256+
- local: ../datasets # disjoint roots work too
257257
remote: /data/datasets
258258
direction: up # dataset push, local is authority
259259
```
260260

261-
Note the disjoint rule applies to the **local** side too: a results directory *inside* the synced repo (e.g. `./collected` under `.`) is rejected — place it next to the repo instead, or narrow the primary mapping to a subdirectory.
261+
**Nested local roots.** An additional mapping's local root may live *inside* the primary mapping's local root (`./collected` under `.`). okdev maintains a managed block in the primary root's `.stignore` (`// okdev:begin/end managed sync excludes`) so the subtree travels only through its own folder — the block is written before the sync daemon starts, so the exclusion is in force from the first scan. Any other local overlap (equal roots, nesting between additional mappings, an additional root containing the primary) is rejected, and **remote roots must always be disjoint**.
262+
263+
**Removing a mapping never widens the primary folder.** When a nested mapping is removed, its folder is pruned from both syncthing instances and local files are kept, but the `.stignore` entry is *retained* as a tombstone (with an explanatory comment) — otherwise the subtree would suddenly bulk-sync through the code channel. To fold the directory back into the primary folder, delete its line from `.stignore`. `status --details` lists both active and retained excludes.
262264

263265
**Volumes are provisioned automatically.** An additional mapping's remote root must be visible to the okdev sidecar (its syncthing serves the remote side), which means it must live on a volume — a root on the container's own filesystem cannot sync. You don't need to write any volume YAML: okdev auto-provisions an emptyDir at any extra remote root not already covered by a target-container volume mount, and mirrors the target container's mounts into the sidecar. Adding or removing a mapping changes the pod spec, so the next `okdev up` asks for `--reconcile` (pod recreate). Auto-provisioned emptyDirs don't survive pod recreation — fine for both directions (`up` re-pushes from local, `down` results are mirrored locally); mount your own PVC at the root if the data must persist, and okdev will use it instead. Sync startup still probes each extra root and fails with a clear error if it is not shared with the sidecar.
264266

docs/troubleshooting.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,23 @@
2222
## Sync Failures
2323

2424
- Validate workspace path exists and is writable in the container.
25-
- Isolate directionality with one-way runs: `okdev sync --mode up`, `okdev sync --mode down`.
25+
- Isolate directionality with one-way runs: `okdev sync --mode up`, `okdev sync --mode down` (an explicit `--mode` forces every mapping for that invocation).
2626
- `sync.engine=syncthing` requires local Syncthing bootstrap (handled by `okdev`) and sidecar image pull success.
27-
- Current Syncthing implementation supports a single `local:remote` mapping.
27+
- Multiple mappings are supported (each is its own syncthing folder with its own `direction`); remote roots must be disjoint, and a local root nested inside the primary root is excluded from the primary folder via the managed block in its `.stignore`.
2828
- If you see `ErrImagePull` / GHCR `403`, use a publicly pullable `spec.sidecar.image` or adjust registry permissions.
2929
- If `okdev status --details` shows stale local sync state or a stale PID file, run `okdev sync --reset`. For mesh sessions, `--reset` also probes receiver health and repairs any broken mesh connections.
3030
- `okdev status --details` shows live mesh health when receivers exist, including per-receiver connection and sync status.
3131
- If the session pod was recreated, rerun `okdev up` to re-bootstrap local sync against the new pod.
3232
- Re-run `okdev ports` if the sync connection depends on managed SSH forwarding that may have been interrupted.
3333

34+
Before treating "changes are not syncing" as a fault, rule out two by-design behaviors:
35+
36+
- **Direction contract**: with `direction: up` pod-side writes never reach local; with `down` local writes never reach the pod (syncthing marks them as local additions). `okdev status --details` shows each mapping's direction arrow.
37+
- **Managed excludes**: a directory that belongs to (or belonged to) its own nested mapping is excluded from the primary folder. `status --details` lists active excludes and tombstones retained after a mapping was removed; delete the entry from the primary root's `.stignore` to fold the directory back into the primary folder.
38+
3439
## Recovering A File Overwritten By Sync
3540

36-
The managed folder is bidirectional in steady state, so a bad write on one side (e.g. an empty file from a failed `okdev exec ... > result.txt` redirect) can propagate and overwrite the real file on the other side. Versioning (on by default, `spec.sync.syncthing.versioningDays`) archives the previous version on the side that applied the incoming change:
41+
With the default `direction: bi` a folder is bidirectional in steady state, so a bad write on one side (e.g. an empty file from a failed `okdev exec ... > result.txt` redirect) can propagate and overwrite the real file on the other side. (Directional mappings — `up`/`down` — are structurally immune: the non-authoritative side's writes are never propagated.) Versioning (on by default, `spec.sync.syncthing.versioningDays`) archives the previous version on the side that applied the incoming change:
3742

3843
- On the pod: `<remote workspace>/.stversions/` (e.g. `/workspace/.stversions/`)
3944
- Locally: `<local sync root>/.stversions/`

internal/cli/status_details.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,14 @@ type detailedStatusSSH struct {
113113
}
114114

115115
type detailedStatusSync struct {
116-
Engine string `json:"engine"`
117-
Direction string `json:"direction,omitempty"`
116+
Engine string `json:"engine"`
117+
Direction string `json:"direction,omitempty"`
118+
// ManagedExcludes are nested child mappings excluded from the primary
119+
// folder; RetainedExcludes are tombstones kept after a mapping was
120+
// removed (the subtree stays out of the primary folder until the user
121+
// deletes the entry from .stignore).
122+
ManagedExcludes []string `json:"managedExcludes,omitempty"`
123+
RetainedExcludes []string `json:"retainedExcludes,omitempty"`
118124
Health string `json:"health,omitempty"`
119125
HealthDetail string `json:"healthDetail,omitempty"`
120126
ConfiguredPaths []string `json:"configuredPaths,omitempty"`
@@ -422,13 +428,19 @@ func buildDetailedSync(sessionName string, cfg *config.DevEnvironment, cfgPath s
422428
engine = "syncthing"
423429
}
424430
direction := ""
431+
var managedExcludes, retainedExcludes []string
425432
if pairs, err := syncengine.ParsePairs(cfg.Spec.Sync.Paths, cfg.EffectiveWorkspaceMountPath(cfgPath)); err == nil && len(pairs) > 0 {
426433
direction = pairs[0].Direction
434+
if absPrimary, absErr := filepath.Abs(pairs[0].Local); absErr == nil {
435+
managedExcludes, retainedExcludes = readManagedSTIgnoreBlock(absPrimary)
436+
}
427437
}
428438
detail := detailedStatusSync{
429-
Engine: engine,
430-
Direction: direction,
431-
ConfiguredPaths: summarizeConfiguredSyncPaths(cfg, cfgPath),
439+
Engine: engine,
440+
Direction: direction,
441+
ManagedExcludes: managedExcludes,
442+
RetainedExcludes: retainedExcludes,
443+
ConfiguredPaths: summarizeConfiguredSyncPaths(cfg, cfgPath),
432444
}
433445
if engine != "syncthing" {
434446
return detail
@@ -602,6 +614,12 @@ func printDetailedStatus(w io.Writer, detail detailedStatus) {
602614
if detail.Sync.Direction != "" {
603615
fmt.Fprintf(w, "- direction: %s (%s)\n", detail.Sync.Direction, modeSymbol(detail.Sync.Direction))
604616
}
617+
if len(detail.Sync.ManagedExcludes) > 0 {
618+
fmt.Fprintf(w, "- excluded from primary folder (own mappings): %s\n", strings.Join(detail.Sync.ManagedExcludes, ", "))
619+
}
620+
if len(detail.Sync.RetainedExcludes) > 0 {
621+
fmt.Fprintf(w, "- excluded from primary folder (retained after mapping removal): %s\n", strings.Join(detail.Sync.RetainedExcludes, ", "))
622+
}
605623
if detail.Sync.BackgroundStatus != "" {
606624
status := detail.Sync.BackgroundStatus
607625
if detail.Sync.BackgroundPID > 0 {

internal/cli/status_details_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,8 @@ func TestPrintDetailedStatusIncludesSections(t *testing.T) {
348348
Sync: detailedStatusSync{
349349
Engine: "syncthing",
350350
Direction: "down",
351+
ManagedExcludes: []string{"/results"},
352+
RetainedExcludes: []string{"/old-datasets"},
351353
BackgroundStatus: "running",
352354
BackgroundPID: 123,
353355
ConfiguredPaths: []string{"/tmp/repo -> /workspace"},
@@ -393,6 +395,8 @@ func TestPrintDetailedStatusIncludesSections(t *testing.T) {
393395
"managed forward: running",
394396
"Sync:",
395397
"direction: down (<-)",
398+
"excluded from primary folder (own mappings): /results",
399+
"excluded from primary folder (retained after mapping removal): /old-datasets",
396400
"background: running (pid 123)",
397401
"conflicts: 2",
398402
"./a.sync-conflict-1",
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package cli
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestMergeManagedSTIgnoreBlock(t *testing.T) {
11+
dir := t.TempDir()
12+
path := filepath.Join(dir, ".stignore")
13+
14+
// Creating the block appends after existing user content.
15+
if err := os.WriteFile(path, []byte("node_modules\n// user comment\n"), 0o644); err != nil {
16+
t.Fatal(err)
17+
}
18+
retained, err := mergeManagedSTIgnoreBlock(dir, []string{"/results"})
19+
if err != nil || len(retained) != 0 {
20+
t.Fatalf("merge: retained=%v err=%v", retained, err)
21+
}
22+
content, _ := os.ReadFile(path)
23+
text := string(content)
24+
if !strings.Contains(text, "node_modules\n") || !strings.Contains(text, "// user comment") {
25+
t.Fatalf("user content must be preserved, got %q", text)
26+
}
27+
if !strings.Contains(text, managedSTIgnoreBegin+"\n/results\n"+managedSTIgnoreEnd) {
28+
t.Fatalf("expected managed block with /results, got %q", text)
29+
}
30+
31+
// Idempotent: same input, same output.
32+
if _, err := mergeManagedSTIgnoreBlock(dir, []string{"/results"}); err != nil {
33+
t.Fatal(err)
34+
}
35+
again, _ := os.ReadFile(path)
36+
if string(again) != text {
37+
t.Fatalf("merge must be idempotent:\n%q\nvs\n%q", text, again)
38+
}
39+
40+
// Growing the active set keeps sorted patterns inside the block.
41+
if _, err := mergeManagedSTIgnoreBlock(dir, []string{"/datasets", "/results"}); err != nil {
42+
t.Fatal(err)
43+
}
44+
active, tombs := readManagedSTIgnoreBlock(dir)
45+
if strings.Join(active, ",") != "/datasets,/results" || len(tombs) != 0 {
46+
t.Fatalf("unexpected block state: active=%v tombs=%v", active, tombs)
47+
}
48+
49+
// Dropping a mapping tombstones its pattern instead of releasing the
50+
// subtree into the primary folder.
51+
retained, err = mergeManagedSTIgnoreBlock(dir, []string{"/datasets"})
52+
if err != nil {
53+
t.Fatal(err)
54+
}
55+
if len(retained) != 1 || retained[0] != "/results" {
56+
t.Fatalf("expected /results newly retained, got %v", retained)
57+
}
58+
active, tombs = readManagedSTIgnoreBlock(dir)
59+
if strings.Join(active, ",") != "/datasets" || strings.Join(tombs, ",") != "/results" {
60+
t.Fatalf("unexpected block state: active=%v tombs=%v", active, tombs)
61+
}
62+
content, _ = os.ReadFile(path)
63+
if !strings.Contains(string(content), managedSTIgnoreTombstone+"\n/results\n") {
64+
t.Fatalf("tombstone comment must precede the retained pattern, got %q", content)
65+
}
66+
67+
// A tombstoned pattern re-added to the config becomes active again and
68+
// is reported as retained exactly once.
69+
retained, err = mergeManagedSTIgnoreBlock(dir, []string{"/datasets", "/results"})
70+
if err != nil || len(retained) != 0 {
71+
t.Fatalf("re-adding must not re-report retention: retained=%v err=%v", retained, err)
72+
}
73+
active, tombs = readManagedSTIgnoreBlock(dir)
74+
if strings.Join(active, ",") != "/datasets,/results" || len(tombs) != 0 {
75+
t.Fatalf("re-added pattern must leave tombstones, got active=%v tombs=%v", active, tombs)
76+
}
77+
78+
// User content after the block is preserved across rewrites.
79+
if err := os.WriteFile(path, []byte(string(content)+"trailing-user-pattern\n"), 0o644); err != nil {
80+
t.Fatal(err)
81+
}
82+
if _, err := mergeManagedSTIgnoreBlock(dir, []string{"/datasets"}); err != nil {
83+
t.Fatal(err)
84+
}
85+
final, _ := os.ReadFile(path)
86+
if !strings.Contains(string(final), "trailing-user-pattern") {
87+
t.Fatalf("content after block must survive, got %q", final)
88+
}
89+
}
90+
91+
func TestMergeManagedSTIgnoreBlockNoopWithoutFileOrPatterns(t *testing.T) {
92+
dir := t.TempDir()
93+
if retained, err := mergeManagedSTIgnoreBlock(dir, nil); err != nil || retained != nil {
94+
t.Fatalf("no file + no patterns must be a no-op, got %v %v", retained, err)
95+
}
96+
if _, err := os.Stat(filepath.Join(dir, ".stignore")); !os.IsNotExist(err) {
97+
t.Fatal("no-op must not create .stignore")
98+
}
99+
}

0 commit comments

Comments
 (0)