#232 Extend DatastoreSyncService.markDirty() with optional relPath argument

A few things worth tightening before this lands — happy to plan an implementation once they're resolved.

1. The current signature isn't no-argument

markDirty already takes an options bag for AbortSignal symmetry with pullChanged/pushChanged:

// src/domain/datastore/datastore_sync_service.ts:61
markDirty(options?: DatastoreSyncOptions): Promise<void>;

That changes the proposed shape. markDirty(relPath?: string) collides with the existing options-object position. Three landing options, none of them "purely additive" without qualification:

• markDirty(options?: DatastoreSyncOptions & { relPath?: string }) — fully source-compatible but the proposed positional shape is gone.
• markDirty(relPath?: string, options?: DatastoreSyncOptions) — type-breaks any extension implementing markDirty(opts) today (no in-tree caller does, but the canonical interface allowed it).
• Drop the options bag — same break, plus loses the AbortSignal channel.

The PR author needs to commit to one and document the migration.

2. The contract mismatch is bigger than the signature

The JSDoc at datastore_sync_service.ts:46-60 is explicit about when and why markDirty fires:

Must be called before ... any write into the cache directory ... so the next pushChanged fast path cannot short-circuit past the write ... Implementations must be idempotent and cheap ... Called before the write begins so a crash mid-write still leaves the watermark dirty.

markDirty is a watermark invalidator — it flips a clean/dirty flag so the next pushChanged knows it has work to do. The actual per-file sync work happens in pushChanged.

The proposal's claim is that the new arg lets queryable backends "validate or update a single file in O(1)." That doesn't fit the contract: at markDirty-time the bytes haven't been written yet, so a Mongo-style backend has nothing to push or validate. Whatever optimization the new arg enables has to flow through pushChanged, not happen inside markDirty.

3. Pick which design you actually want

The proposal is conflating two distinct designs:

Design A — path-tracking via markDirty. markDirty stays cheap, records relPaths into a Set, and pushChanged consumes that Set instead of walking the cache. This matches "extensions MAY use this to track per-path dirty state" but the proposal's wording suggests something stronger.

Design B — path-level sync. pushChanged itself learns to take a dirty-paths list (pushChanged({ paths })); markDirty is unchanged or repurposed. This is what "validate or update a single file in O(1)" actually requires.

Pick one. The two have different surface areas, different test stories, and different implications for the s3/gcs fingerprint fast path.

4. The "10–15 call sites" undersells the cascade

The internal MarkDirtyHook indirection (type MarkDirtyHook = () => Promise<void> at datastore_sync_service.ts:76) also has to grow the arg, and every notifyDirty() helper in the four repositories needs the path threaded in:

• unified_data_repository.ts — 8 sites
• yaml_output_repository.ts — 2 sites
• yaml_workflow_run_repository.ts — 2 sites
• yaml_evaluated_definition_repository.ts — 3 sites

Plus the wiring at repo_context.ts:437 and :566 that bridges syncService.markDirty() into the hook (and would now own absolute → cache-relative path conversion). Plus the packages/testing/datastore_types.ts mirror, which is shape-checked against the canonical type by testing_package_compat_test.ts. Plus design/datastores.md and the swamp-extension-datastore skill (api.md + examples.md).

Mechanical work, but not "purely additive, optional" — every caller plus the hook type needs to move.

What I'd ask

1. Fix the "no-argument" claim and pick a signature that composes with the existing options bag.
2. Decide explicitly whether you want path-level invalidation tracking (Design A — markDirty records, pushChanged consumes) or path-level sync (Design B — pushChanged takes paths). State the answer in the proposal.
3. Acknowledge the MarkDirtyHook + call-site + wiring cascade so the implementation footprint isn't a surprise.

Once those land I'll finish the plan against whichever design you pick.

Thanks — all three points are valid. Revisions:

1. Signature — extend the existing options bag

Conceding the "no-argument" claim. The current shape markDirty(options?: DatastoreSyncOptions) rules out the positional relPath I proposed. The right landing is:

interface MarkDirtyOptions extends DatastoreSyncOptions {
  relPath?: string;
}
markDirty(options?: MarkDirtyOptions): Promise<void>;

Fully source-compatible with today's markDirty() and markDirty({signal}) calls. The other two landing options the reviewer listed (positional relPath first, drop options bag) both break existing callers and lose AbortSignal symmetry — non-starters.

2. Committing to Design A

markDirty records the relPath into an in-memory dirty set (persisted to a sidecar so it survives process restart); pushChanged consumes that set instead of walking the cache.

This matches what markDirty actually is — a watermark invalidator called before the write, when bytes aren't on disk yet. The path argument turns it into a path-tracking invalidator. Per-key sync work still happens inside pushChanged, just driven by the recorded set instead of a full-cache walk. The reviewer's framing is correct: the win flows through pushChanged; the new arg lets it know which paths to look at.

Design B (pushChanged({ paths })) was tempting but requires swamp-core's coordinator to also maintain a dirty-paths list and pipe it through to the extension — a bigger structural change for the same end result.

The original wording ("validate or update a single file in O(1)") was wrong about timing. Corrected: markDirty records O(1); pushChanged later does the per-file work, sized to what was recorded rather than to the cache.

3. Acknowledging the cascade

The implementation is strictly additive at the behavior level — extensions ignoring the new field keep working — but the mechanical footprint extends past the four repository files I named:

• Canonical DatastoreSyncService interface + MarkDirtyHook type at datastore_sync_service.ts:76 both grow the arg
• notifyDirty() helpers in the four persistence repos (~15 sites total: unified_data_repository.ts 8, yaml_output_repository.ts 2, yaml_workflow_run_repository.ts 2, yaml_evaluated_definition_repository.ts 3)
• Wiring at repo_context.ts:437 and :566 — and that wiring layer takes on the abs→cache-relative path conversion (it's the layer that owns the cache root)
• packages/testing/datastore_types.ts mirror, kept in sync via testing_package_compat_test.ts
• design/datastores.md contract section
• swamp-extension-datastore skill (api.md + examples.md)

Mechanical work, but not "forget about it" — every caller plus the hook type moves.

What I'd ask now

Replan against these three corrections. Once a v2 plan lands I'll review and we can go from there.