feat(roca-firstmate): add session-owned resident watch - #6
Merged
Conversation
Raise watch as a stdin-owned child that catch-up sweeps then listens. Reuse seats for single-flight, log JSONL telemetry, and place the executable in the plugin directory so a session parent can resolve it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Wire the roca-firstmate watcher into the session-residency pattern: when a long-lived parent (roca mcp serve) owns stdin/stdout, it can raise
roca-firstmate watchas a resident child that lives and dies with the session. This closes the hole where the plugin mirror went stale because watch is a foreground process, there is deliberately no daemon or KeepAlive, and nothing re-raises it after the installer terminal closes.All raising logic belongs to this plugin repo. La Roca must gain zero plugin-specific code. Current La Roca only hardcodes a vector resident child and rejects unknown plugin.json fields, so this change does not add a companions key to the manifest and does not edit the La Roca product. The plugin implements the child contract so a generic session-companion seam, if added later, can exec watch with no firstmate mention in kernel code.
Required child contract:
placecopies it into the plugin directory (plugin dir is fine); serve/parent should resolve from plugin registration, not PATHWhat Changed
watchfollow parent stdin lifetime, sweep before live monitoring, retry transient failures with backoff, and emit JSONL lifecycle and sweep telemetry.seatsfor per-home single-flight leases, fenced Scribe commits, lease renewal, release, and standby takeover.placefor plugin-local executable resolution and fabricated-home coverage for shutdown, catch-up, concurrent-holder failover, and ingestion.Risk Assessment
🚨 High: The change still has a breaking multi-home readiness protocol, does not prove the required takeover deadline, and can destructively replace a destination directory.
Testing
After confirming the target commit and clean baseline, targeted automated tests and a real subprocess e2e both passed; persisted database and telemetry artifacts demonstrate session death, missed-write catch-up, live resume, single-flight lease takeover, fail-soft retry coverage, executable placement, and unchanged data-only payload.
Evidence: Watcher JSONL telemetry
Evidence: Persisted watcher version history
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
cmd/roca-firstmate/resident.go:157- The lease is acquired before the synchronous fingerprint sweep, but heartbeats do not start until the sweep finishes. If Backfill exceeds the two-minute lease, another session can acquire the expired row; the first session then activates its watcher without revalidating ownership. Renew during the sweep or fence activation with a final ownership check.cmd/roca-firstmate/resident.go:183- A renewal error leaves the existing source active. If database access fails through the lease deadline, another session can acquire the expired lease while the original watcher continues consuming events until a later heartbeat. Stand down on renewal failure or enforce the locally known lease expiry before processing more events.cmd/roca-firstmate/resident.go:124- The required contract says a watch crash must be “noted in telemetry and retried with backoff,” but an IngestPath failure is only logged; the home remains marked holding and no retry or recovery sweep is scheduled. Polling has already advanced its filesystem baseline, so a transient read/SQLite failure can leave the mirror stale indefinitely. Mark the home retryable and perform a catch-up sweep after backoff.cmd/roca-firstmate/resident.go:141- The watcher lock is inserted as a livemachinesubscription seat. Consequently tick will suppress orphan delivery for all machine wakeups while any watcher is alive, although watch never drains them; after clean stdin shutdown, ReleaseSeat merely expires the row, so silenceClock later creates recurring captainseat-silentwakeups for an intentionally ended watcher. Separate watcher-lock rows from subscription/silence semantics while retaining the existing seats lease machinery.internal/nerve/lease.go:16-watch-<home-id>shares an unreserved namespace with the public arbitrary--seat-id. Registering a follow seat with that valid value overwrites the live watch token and destination, forcing the watcher to stand down. Reserve and reject the internal prefix at the explicit-seat boundary, or otherwise make watcher identities collision-proof.🔧 Fix: Fence watcher leases and retry failed ingestion
1 error still open:
cmd/roca-firstmate/resident.go:100- Lease heartbeats share the event-loop goroutine with synchronous acquisition and ingestion work. If H1 is active while H2's catch-up sweep takes longer than the two-minute lease, H1 cannot renew; another session can acquire H1 while the original source remains active, and a queued H1 event may be consumed before the pending heartbeat detects the lost token. A dropped-event recovery Backfill reaches the same failure. Run renewal/ownership fencing independently so no sweep or ingestion can starve it.🔧 Fix: Decouple lease heartbeats from watcher work
3 errors still open:
internal/nerve/lease.go:86- Lease expiry uses lexical comparison between variable-width RFC3339Nano strings. For example, SQLite considers...02.12Z <= ...02.1Ztrue even though.12is later, allowing a contender to overwrite a still-live lease; the old watcher remains active until its next heartbeat. Use a fixed-width timestamp representation or numeric SQLite time comparison at the shared lease boundary.cmd/roca-firstmate/resident.go:60- The required fail-soft contract says a watch crash must be logged and retried with backoff, but any transientnewIngesterdatabase/filesystem failure exits the child immediately. Becauseraiseis logged only afterward, this path records neither raise nor crash-retry and requires the parent to re-raise the child. Move runtime initialization into the retry lifecycle while keeping permanent configuration errors fatal.cmd/roca-firstmate/resident_test.go:342- The intent explicitly requires a lab e2e that starts a session-owned child, proves it exits with stdin, and verifies process-death takeover.startWatchonly invokesrunContextin a goroutine, so it tests in-process cancellation and clean lease release—not executable resolution, OS child lifetime, or lease-expiry inheritance after abrupt holder death. Add a subprocess-level test using the built/placed CLI and fabricated homes.🔧 Fix: Harden lease timing and subprocess residency
6 issues (5 errors, 1 warning) still open:
cmd/roca-firstmate/resident.go:129- Event processing is not fenced by the lease deadline:watchHomeLeasechecks only local state. If the process pauses past expiry, a standby acquires the seat, and the original resumes with a queued event before its heartbeat observes token loss, the old holder can still callIngestPath. Enforce the renewed deadline locally or verify the holder token in the mirror transaction before committing.cmd/roca-firstmate/resident.go:156-nowis captured beforenewIngester. If initialization takes longer than the lease,TryAcquireSeatwrites an already-expired lease and then activates the source/sweep, allowing an immediate competing acquisition. Re-sample time immediately beforeTryAcquireSeat.internal/nerve/lease.go:88- The new fixed-width timestamps are converted through SQLitejulianday, which cannot preserve all nine fractional digits. A contender just before a sub-millisecond expiry can compare equal and satisfy<=, stealing a live lease. Compare the canonical fixed-width UTC strings directly or use an exact integer timestamp.cmd/roca-firstmate/main.go:123- The required “fail-soft inside the child” retry lifecycle still begins only afteropenDatabase. A transient open, ping, WAL pragma, or migration failure exits immediately with no raise/crash-retry telemetry, requiring the parent to re-raise the child. Move transient database initialization into the watch retry lifecycle while retaining fatal argument/configuration validation.cmd/roca-firstmate/resident.go:73- Required JSONL telemetry is best-effort because everylog.Appenderror is discarded. With an unwritable or full log directory, watch reports itself active while recording none of raise, lease, sweep, or crash-retry and emits no diagnostic. Define and enforce failure handling at a shared logging boundary.cmd/roca-firstmate/resident.go:353- Crash telemetry stores raw error strings even though the logger contract says counts and identities only. For example, a missing data directory fromnewIngesterincludes the absolute home path and is written verbatim. Record a bounded error code/message or redact configured paths before appending.🔧 Fix: Fence watcher ingestion and harden retry telemetry
1 error still open:
cmd/roca-firstmate/resident.go:102- The retry loop treats every database-open failure as transient. A mistyped or missing--dbpath therefore keeps the resident alive indefinitely, emitting only JSONL retries and never reachingwatching, despite the prescribed boundary that missing/invalid configuration remains fatal. Classifyos.ErrNotExistand other permanent database validation errors before entering the retry loop; reserve retry for busy/ping/WAL-style transient failures.🔧 Fix: Classify permanent watcher database failures
1 error still open:
cmd/roca-firstmate/resident.go:223- The ingester is created before lease acquisition and then cached across failed acquisitions and lease loss. Becausescribe.Newsnapshotsingest_file_state, a standby can later acquire using stale state; a timestamp-preserving restore to that cached fingerprint makes Backfill return “unchanged” before consulting SQLite, even when another holder committed intervening content. Refresh/recreate the ingester after each successful acquisition and before the catch-up sweep.🔧 Fix: Refresh Scribe state on watcher takeover
2 errors still open:
cmd/roca-firstmate/main.go:193- The required contract says a watch crash is retried with backoff, but after the initial path validation a transient removal/replacement between lines 99 and 111 makesopenDatabasereturnos.ErrNotExist; this classifier rejects every non-SQLite error, so the already-raised child exits instead of retrying. Keep the initial configuration preflight fatal, but treat subsequent runtime open failures as retryable.cmd/roca-firstmate/resident.go:237- Before acquiring the lease, the discarded factory call runsscribe.New, whosehomesUPSERT updateslabelandkind. A valid standby using the same home ID with different metadata can therefore overwrite the active holder's home row despite failing lease acquisition. Pre-register only the missing FK row before acquisition, and apply mutable metadata only after ownership is established.🔧 Fix: Separate watcher preflight and metadata ownership
2 errors still open:
internal/scribe/scribe.go:189-scribe.Newcommits thehomeslabel/kind UPSERT without applyingCommitFence. If holder A pauses past lease expiry, B can acquire and register B's metadata, then A can resume before its heartbeat runs and overwrite that metadata; A's later backfill fence fails, but the stale metadata remains committed. Fence the metadata registration transaction with the holder token.cmd/roca-firstmate/resident.go:367- A renewal is treated as live using the timestamp captured beforeRenewSeat. If the database call or process is delayed longer than the configured lease, it can successfully write an already-expired deadline and leave the local source active; another session may then acquire immediately. After renewal, verify the deadline is still future or retry using a fresh timestamp before retaining the watcher.🔧 Fix: Fence home metadata and renewal deadlines
4 errors still open:
cmd/roca-firstmate/resident.go:141- The required child contract says it “dies when stdin closes,” but shutdown calls stand-down withcontext.Background;ReleaseSeatthen uses an uncancelled context. If another SQLite writer holds the database, this cleanup can block through the configured five-second busy timeout—or longer on stalled I/O—leaving the child alive after EOF. Bound cleanup or let the lease expire when prompt shutdown is required.cmd/roca-firstmate/resident.go:170- The required takeover must occur “within the lease window,” but standby acquisition runs only on the fixed retry ticker. With defaults, if the holder dies immediately after renewal and a standby attempt occurs just before the two-minute expiry, its next attempt is thirty seconds later, exceeding the lease window. Schedule acquisition from the observed lease deadline or otherwise guarantee an attempt at expiry.cmd/roca-firstmate/resident.go:112- The required lifecycle saysraiseshould be recorded only after the child enters its retry lifecycle. A regular non-SQLite or corrupt file passesvalidateDatabasePath, emitsraise, thenopenDatabasereturns a non-retryable SQLite error and the child exits immediately. Complete permanent database validation before emittingraise, or defer it until open succeeds or is classified retryable.cmd/roca-firstmate/main.go:207- The fail-soft open classifier omits SQLiteCANTOPEN(14). A database that passes the path preflight but becomes temporarily unavailable during open/Ping—such as a transient mount or access race—therefore exits the already-raised child instead of logging crash-retry and backing off. Treat post-preflight availability errors as retryable while retaining fatal handling for corrupt/not-a-database inputs.🔧 Fix: Harden watcher shutdown, takeover, and database retry
1 warning still open:
cmd/roca-firstmate/resident.go:142- On stdin EOF,ctxis already canceled, so deriving the 250 ms cleanup context from it makesReleaseSeatimmediately fail with cancellation. A replacement session therefore remains standby until the production lease expires (up to two minutes), even when SQLite is available. Derive bounded cleanup from a fresh background context.🔧 Fix: Release watcher leases within bounded shutdown cleanup
3 errors still open:
cmd/roca-firstmate/resident.go:116- The required child contract says it “dies when stdin closes,” but startup callsopenDBsynchronously without the session context.openDatabaseusesPingand migration with a five-second SQLite busy timeout, so closing stdin while startup waits on a writer lock cannot interrupt it and stalled I/O can delay exit indefinitely. Make database opening/migration context-aware or otherwise bounded by session cancellation.cmd/roca-firstmate/resident.go:341- Home acquisition is sequential and performs the complete source setup and Backfill before trying the next home. If H1 has a slow sweep while H2’s holder dies, this standby cannot even attempt H2 until H1 finishes, exceeding H2’s lease window. Acquire and begin heartbeating all eligible seats before blocking sweeps, or run independent per-home acquisition lifecycles.cmd/roca-firstmate/resident.go:341- The required child contract says it “dies when stdin closes,” but cancellation during Backfill is ineffective for an unchanged corpus: Scribe’s WalkDir callback never checks the context, and unchanged files return before any context-aware database operation. A large restart sweep can therefore continue after EOF and block shutdown. Enforce cancellation inside the shared Backfill walk, before fingerprint work.🔧 Fix: Decouple home activation and enforce cancellation
4 issues (3 errors, 1 warning) still open:
cmd/roca-firstmate/resident.go:604- The resident reuses one Ingester's process-local fingerprint cache indefinitely. Concrete sequence: watch caches F1; an authorized attach/scribe process ingests F2; a timestamp-preserving restore returns the file to F1; the queued watch event returns “unchanged” from the stale cache before consulting SQLite, leaving F2 current while disk holds F1. Validate persisted cursor/current state at Scribe's shared unchanged boundary instead of relying solely on the cached map.cmd/roca-firstmate/resident.go:213- Only the first successful activation is announced, and the renderer receives singleton slices. With two free homes, stdout reportswatchingfor whichever sweep finishes first and never reports the second home's readiness, backend, or sweep counts, regressing the existing multi-homehomes[]contract. Define per-home readiness events or aggregate all configured-home outcomes before emitting child-level readiness.cmd/roca-firstmate/resident.go:300- A transient TryAcquireSeat error clears the previously observed lease deadline. If SQLITE_BUSY occurs at that deadline, the default retry moves the next attempt 30 seconds later, so a holder killed just after renewal is not inherited within the two-minute lease window. Preserve the observed deadline and retry promptly when it is due.cmd/roca-firstmate/place.go:100-os.Renamedoes not replace an existing destination on Windows, so a subsequentplaceinvocation cannot update an already placed executable. Use a platform-appropriate replacement operation or a recoverable remove/rename sequence.🔧 Fix: Harden watcher takeover, readiness, cursor, and placement
2 errors still open:
cmd/roca-firstmate/resident.go:223-activateWatchHomestarts the event pump before the main loop accepts its success. A queued event can fail and stand the home down, yet the later success message still passes this source-only check and emitswatchingfor a closed watcher. Carry the generation and revalidate the lease here, or delay pumping until activation is accepted.cmd/roca-firstmate/resident.go:228- The requested multi-home contract says to “report all configured homes.” If one home's initial activation fails transiently while another succeeds,pendingActivationsreaches zero, the successful subset is announced, andannouncedpermanently prevents the retried home from appearing. Either wait for every configured home or define per-home readiness updates.🔧 Fix: Revalidate and refresh multi-home watcher readiness
4 errors still open:
cmd/roca-firstmate/verbs.go:584- With multiple configured homes, if only one currently activates, this branch emits the single-homebackfillenvelope rather thanhomes[]; when another home later activates, stdout changes schema mid-session. Select the envelope from the configured-home count, not the active-summary count.cmd/roca-firstmate/resident.go:290- After an announced home encounters a stream or ingestion failure, stand-down clears its active state but readiness is not recomputed because updates occur only on activation messages. The lastwatchingenvelope can therefore indefinitely claim a failed home remains active. Notify readiness from the shared stand-down transition.cmd/roca-firstmate/resident_test.go:811- The required criterion says “kill the holder, the other takes over within the lease window,” but this test configures a 500ms lease and accepts takeover through the helper's eight-second timeout. A multi-second takeover regression would pass; assert elapsed takeover time against the lease with a small scheduling margin.cmd/roca-firstmate/place.go:116- The replacement fallback treats any existing destination as replaceable. Ifroca-firstmateis a directory, it is renamed aside, replaced by the executable, and either deleted when empty or left under a random backup while the command reports failure. Refuse non-regular destinations before renaming.✅ **Test** - passed
✅ No issues found.
git status --short --branchandgit diff --stat/--name-status <base>..<target>go test ./cmd/roca-firstmate -run '^(TestWatchSubprocessSessionLifecycleAndAbruptTakeover|TestWatchResidentRetriesFailedIngestWithCatchUp|TestWatchResidentRetriesRuntimeInitialization|TestPlaceCopiesExecutableIntoPluginDirectory)$' -count=1 -vgo test ./internal/nerve -run '^(TestTryAcquireSeatSingleFlightAndInherit|TestConcurrentTryAcquireSeatYieldsOneHolder|TestReleaseSeatLetsTheNextHolderTakeOverImmediately)$' -count=1 -vgo test ./internal/watchlog -run '^TestAppendWritesRaiseLeaseAndSweepEvents$' -count=1 -vgo test ./schema -run '^(TestChecksumsCoverExactlyTheInstallPayload|TestShippedDatabaseHasNoMirroredRows|TestTelemetryTablesRemainFreeForLaterAdditiveSchema)$' -count=1 -vBuilt and ranroca-firstmate watch --db <fabricated-db> --json --poll-interval 50ms --home <fabricated-home> --home-id northwind-harborwith session-owned stdin; verified live ingest, EOF exit, idle non-ingest, restart catch-up, concurrent standby, abrupt holder death, takeover, and resumed ingestQueried fabricated SQLite state to confirm one active watch seat during concurrency and exactly one persisted row per observed versionRanroca-firstmate place --dir <fabricated-plugin-dir>and verified the placed file was executable and byte-identicalExported final persisted history and JSONL telemetry to the dedicated evidence directory, then removed all worktree scratch artifacts🔧 **Document** - 1 issue found → auto-fixed ✅
plugin.json:187- Semantic metadata still describesseatsonly as workspace subscriptions, but resident watch holders now use the same table. Correcting this requires changingplugin.jsonand its package checksum, which this documentation-only phase cannot modify.🔧 Fix: Document resident watch seat leases
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.