fix: Operator drives the rk cron clock explicitly — mute/unmute on tracked-set transitions, lease as bounded snooze - #660
Conversation
…, lease as bounded snooze run-kit removed both suppress_while guards (nothing-tracked, operator-loop-fresh) from the operator-tick cron entry; quiescence is now an explicit mute issued edge-triggered from mutateOperatorState via rk cron mute <id> / --off (fail-silent, 5s-bounded, LookPath-gated), with lightweight reconcile in tick-start --diff and the skill's Init step 4. Skill, memory, spec, and _cli-fab/_cli-external realigned to the guard-free entry (no anchor, respawn argv); slugify/StatePath documented as a cross-repo contract with run-kit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
rk cron list --json parsing can silently fail when muted_until is null, which would disable clock resolution/mute logic in the common “muted” state.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR realigns fab-kit’s operator cadence with run-kit’s guard-free cron entry by moving “quiescence” from rk-side inference to explicit, deterministic rk cron mute/--off calls driven by fab’s tracked-set state mutations, and updates the skills/docs to match that contract.
Changes:
- Add Go-side tracked predicate + edge-triggered clock sync (mute/unmute on tracked↔untracked transitions) with a lightweight tick-start reconcile for drift.
- Introduce a context-bounded subprocess helper (
pane.RunCmdContext) and wire clock calls through it with timeout + fail-silent behavior. - Update operator skill text and docs/specs/memory to reflect mute/lease semantics and the new ownership model.
File summaries
| File | Description |
|---|---|
| src/kit/skills/fab-operator.md | Updates operator skill to describe mute/unmute ownership, lease semantics, and ready-line clock status. |
| src/kit/skills/_cli-fab.md | Documents the shared “clock side effect” across operator state verbs and tick-start diff reconcile. |
| src/kit/skills/_cli-external.md | Adds a pointer to the fab-owned operator clock mute/lease rules. |
| src/go/fab/internal/pane/pane.go | Adds RunCmdContext and routes RunCmd through it to preserve behavior while enabling timeouts. |
| src/go/fab/internal/pane/pane_test.go | Adds tests covering deadline cancellation and stdout/stderr capture for RunCmdContext. |
| src/go/fab/cmd/fab/operator.go | Adds cross-repo contract comments for state-file slug derivation and ownership. |
| src/go/fab/cmd/fab/operator_tick_start.go | Disables edge-triggered sync in tick-start diff path and adds reconcile mute-if-untracked after mutation. |
| src/go/fab/cmd/fab/operator_test.go | Updates slugify test comments to note the cross-repo consumer contract. |
| src/go/fab/cmd/fab/operator_state.go | Introduces mutateOperatorStateClock and wires edge-triggered sync post-save. |
| src/go/fab/cmd/fab/operator_clock.go | Implements tracked predicate, rk cron runner seam, entry resolution, and mute/unmute behavior. |
| src/go/fab/cmd/fab/operator_clock_test.go | Adds comprehensive tests for tracked predicate, flips, reconcile, and fail-silent degradation. |
| fab/changes/260909-qvek-operator-cron-mute-lease/plan.md | Captures the implementation plan, requirements, and acceptance checklist for this change. |
| fab/changes/260909-qvek-operator-cron-mute-lease/intake.md | Records clarified decisions and updated requirements (placement + reconcile strategy). |
| fab/changes/260909-qvek-operator-cron-mute-lease/.status.yaml | Updates fab change status/progress metadata and computed impact summary. |
| fab/changes/260909-qvek-operator-cron-mute-lease/.history.jsonl | Logs stage transitions and confidence scoring history for the change. |
| docs/specs/skills.md | Updates /fab-operator spec lines to reflect mute/lease posture and tools usage. |
| docs/memory/runtime/operator.md | Realigns runtime/operator memory to the new explicit mute/lease clock contract and design decisions. |
| docs/memory/runtime/index.md | Updates runtime index description for operator memory to include tracked-set-verb mute/lease quiescence. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Name string `json:"name"` | ||
| Target string `json:"target"` | ||
| Muted bool `json:"muted"` | ||
| MutedUntil int64 `json:"muted_until"` | ||
| } |
There was a problem hiding this comment.
Skipped — Go's encoding/json unmarshals a JSON null into a non-pointer numeric type as a no-op, never an error; the existing test fixture (cronListJSONMuted) already exercises "muted_until":null through resolveOperatorCronRow and passes, so clock resolution does not no-op here.
| ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) | ||
| defer cancel() | ||
| start := time.Now() | ||
| _, _, err := RunCmdContext(ctx, "sleep", "30") | ||
| if err == nil { | ||
| t.Fatal("RunCmdContext past-deadline sleep: err = nil, want non-nil") | ||
| } | ||
| if elapsed := time.Since(start); elapsed > 5*time.Second { | ||
| t.Fatalf("RunCmdContext returned after %v, want within the deadline", elapsed) | ||
| } |
There was a problem hiding this comment.
Fixed — tightened the elapsed bound in TestRunCmdContext_DeadlineKillsChild from 5s to 2s so the test actually validates the 100ms deadline kills the child. (68adb1e)
Tighten TestRunCmdContext_DeadlineKillsChild elapsed bound 5s -> 2s so the test actually validates that the 100ms context deadline kills the child. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Meta
qvekexcludes
fab/,docs/· generated by fab-kit v2.24.5Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr
Summary
run-kit's operator-tick cron entry lost both
suppress_whileguards (nothing-tracked,operator-loop-fresh) in sahil87/run-kit#889 after rk's state-file slug mismatch silently suppressed every operator tick for days. fab-kit's documented clock was then wrong in three places, and nothing on the fab side stopped ticks on an empty operator. This change moves quiescence from rk-side inference to explicit, deterministic mute/unmute issued by fab's tracked-set verbs, realigning skill text, memory, and specs to the guard-free entry.Changes
mutateOperatorStateissuesrk cron mute <id>when the tracked set (monitored / watches / active autopilot / open coordination note) empties andrk cron mute <id> --offwhen it first fills; every call isexec.LookPath-gated, argv exec, 5s-bounded via a new context-boundpane.RunCmdvariant, and fail-silenttick-start --diffmutes on a post-diff untracked state; skill §2 Init step 4 unmutes when the entry ismutedwhile work is trackedrk cron mute <id> --for <dur>for user-requested bounded quiet; never leave an indefinite mute behind while work is trackedfab-operator.md) — §2 ready line gains· muted/· muted until <t>; §4 quotes the guard-free, anchor-less entry withrespawn: ["rk","operator","-L","{server}"]; ownership reworded to tracked-set-verb mute/unmute; Tick Behavior step 7, §6 known-gap paragraph, and §9 Cadence row updateddocs/memory/runtime/operator.mdclock block + Design Decisions (add "Explicit Mute Over Inferred Quiescence", "Lease Is a Bounded Snooze, Not a Heartbeat"; supersede the nothing-tracked gap entry),docs/specs/skills.mdtwo-line update,_cli-fab.mdshared clock-side-effect paragraph,_cli-external.md§ rk pointerslugify/serverSlug/StatePathinoperator.godocumented as owned by fab-kit and mirrored by run-kit for display; renaming either requires a coordinated run-kit change🤖 Generated with Claude Code