Wire real tracker/notifier adapters into os-sdlc escalation #342
Labels
No labels
P0
P1
P2
P3
bug
create
delete
enhancement
filed-by/agent
filed-by/user
frozen
lint-rule
needs-info
needs-triage
next
plugin/cc-architect
plugin/os
plugin/os-adr
plugin/os-aidd-lint
plugin/os-backlog
plugin/os-context
plugin/os-doc-hygiene
plugin/os-sdlc
plugin/os-vault
project/cc-os
ready-for-agent
ready-for-human
recurring
review
update
waiting
wayfinder:grilling
wayfinder:map
wayfinder:map
wayfinder:research
wayfinder:task
wayfinder:task
wontfix
worklist/deviations
worklist/lint-rule
worklist/new-implement-build
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
jared/cc-os#342
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
Deferred from #341 (ADR-0113 park-and-notify): EngineFactory's production call sites (plugins/os-sdlc/bin/os-sdlc-runner, hooks/post_tool_use.rb) still inject NullTracker/NullNotifier, so a parked ticket adds no waiting label, no blocker comment, and no push notification. Build a tea-based tracker port (waiting label + blocker comment) and a push-notification notifier port, and inject them at those call sites. Also retire the table-identity legacy branch (SkeletonTable .equal? checks in Resumer/StageBriefWriter) before phase 3 closes — flagged non-blocking in the #341 review.
Decomposition (2026-08-12)
Evidence gathered via
os-sdlc:code-probe(ast-grep, read-only) before decomposing:plugins/os-sdlc/lib/os_sdlc/runner/null_tracker.rb/null_notifier.rb):Tracker#add_waiting_label(ticket_id),Tracker#blocker_comment(ticket_id, message),Notifier#push(message).plugins/os-sdlc/lib/os_sdlc/runner/engine_factory.rb,Build#tracker/Build#notifier):@options[:tracker] || NullTracker.new,@options[:notifier] || NullNotifier.new. Both production call sites never pass these options:hooks/post_tool_use.rb:56(EngineFactory.build(root), no options at all) andbin/os-sdlc-runner:15(EngineFactory.build(root, **options)— CLI-parsedoptionshas no tracker/notifier flag today).plugins/os-sdlc/lib/os_sdlc/runner/stage_brief_writer.rb:22-24):legacy? => table.equal?(SkeletonTable::TABLE), gatingBrief.write(old flat path) vsBriefAssembler(map-based path).plugins/os-sdlc/lib/os_sdlc/runner/resumer.rb,abort!):Ticket.update_state(@context.db, @ticket[:id], "failed")— literal string, no table lookup.TransitionTablealready exposesterminal_states/terminal?(state)(transition_table.rb:28,42-43), validated at load time via existingvalidate!.plugins/osandplugins/os-sdlcfor notify-send/osascript/ntfy/pushover — none found); this is new surface, scoped minimally below.Gate/sequencing logic stays out of scope for every subtask below (ADR-0097): none of these change routing in
transition_table.rb, they add adapters and fix a data lookup.Subtask 1 — Tea-based Tracker adapter
Add
OsSdlc::Runner::TeaTracker(new file, sibling tonull_tracker.rb) implementing theTrackerport:add_waiting_label(ticket_id)shells out totea issues edit <ticket_id> --add-labels waiting --repo <configured-repo>;blocker_comment(ticket_id, message)shells out totea comment <ticket_id> --repo <configured-repo>(or the equivalent non-hanging path — MEMORY notestea commenthangs on large bodies; keep blocker-comment bodies short/single-line, or write via a temp file the same way this task updates #342, and verify non-interactively before landing).plugins/os-sdlc/lib/os_sdlc/issue_source.rborplugins/os-backlog/lib/backlog/issues.rbalready resolves for repo config — do not invent a second config path).ghsupport, retry/backoff on tea failures (a failed tracker call should not crash escalation — see Subtask 3 for the injection contract, not this subtask for resiliency policy).add_waiting_labelinvokestea issues edit <id> --add-labels waitingwith the correct repo.blocker_commentinvokes the tea comment path with ticket_id and message, non-interactively (test must not actually hang or shell out to a real network call).TeaTrackeris a duck-type match forNullTracker: same two public method signatures, verifiable by a shared contract test module run against both.Subtask 2 — Push notifier adapter
Add
OsSdlc::Runner::PushNotifier(or similarly named, sibling tonull_notifier.rb) implementingpush(message). Given no existing push mechanism in this repo and the dev machine is Linux/Fedora, scope to anotify-send-based desktop notification (matches the local environment; no external service dependency).notify-sendwith the message, guarded so a missingnotify-sendbinary degrades to a no-op-with-warning rather than raising (escalation must never crash on a notification failure — matches ADR-0113's "end the turn" framing, which assumes escalation persistence in SQLite succeeds regardless of notification delivery).osascript, mobile push, ntfy/Pushover integration) — no requirement or precedent for those surfaced during the probe; deferred until a concrete need appears.push(message)invokesnotify-sendwith the message text.pushswallows it without raising and the ticket's escalation persistence (already covered by existing SQLite Escalation tests) is unaffected — i.e. this subtask only needs to provepushitself doesn't raise, not re-test Escalation.PushNotifieris a duck-type match forNullNotifier: same one public method signature.Subtask 3 — Inject real adapters at both production call sites
Wire
TeaTracker/PushNotifierintohooks/post_tool_use.rb:56andplugins/os-sdlc/bin/os-sdlc-runner:15, replacing the implicitNullTracker/NullNotifierfallback for real runs.EngineFactory::Build#tracker/#notifieralready read@options[:tracker]/@options[:notifier]— this subtask supplies those options at the two call sites, it does not changeengine_factory.rb.TeaTracker.new/PushNotifier.new(with whatever config each needs, e.g. repo slug) at both call sites and passing them asEngineFactory.build(root, tracker: ..., notifier: ...)..sdlc/project.yaml) — no requirement surfaced for that; both call sites get the real adapters unconditionally. If a future ticket needs a stub mode for local dev, that's a separate concern (CLI flag or env var), not this ticket.plugins/os-sdlc/tests/runner/post_tool_use_hook_test.rbandgate_wiring_test.rb) assertsEngineFactory.buildis invoked (or the hook/CLI path is exercised) with non-Null tracker/notifier instances at both sites — via dependency injection/stub, not a real tea/notify-send call.NullTracker/NullNotifierdefaults (e.g.retry_counter_reset_test.rb) continue to pass unmodified — this subtask must not changeEngineFactory's default-injection behavior, only what the two call sites explicitly supply.Subtask 4 — Retire the SkeletonTable
.equal?legacy identity branchRemove
StageBriefWriter#legacy?'stable.equal?(SkeletonTable::TABLE)check (plugins/os-sdlc/lib/os_sdlc/runner/stage_brief_writer.rb:22-24) and theBrief.writelegacy path it guards, once no live call site still constructs a legacy (non-map) engine — confirm viaEngineFactory::Build#legacy_engine(engine_factory.rb) still being reachable; iflegacy_engineis still a live path (project has no.sdlc/project.yaml/map), this subtask is blocked until that path itself is retired or given its own non-identity signal.Brief.writecall it guards, iff investigation confirmslegacy_engineis unreachable in current production flows (all projects onboarded to maps). If it's still reachable, this subtask becomes "replace.equal?with an explicitlegacy:flag threaded throughEngine::Context" instead of an outright deletion — the probe did not resolve which of these two shapes applies; that's a judgment call for whoever picks up this subtask, informed by whetherSkeletonTableis still referenced byEngineFactory::Build#legacy_engine.SkeletonTableitself orEngine::Context'smap_loadershape — only the identity-check branch and (if provably dead) its guarded legacy path.grep -rn "SkeletonTable::TABLE" plugins/os-sdlc/libreturns no.equal?comparison;StageBriefWriter#writealways calls@assembler.write; existing StageBriefWriter tests updated/pass without a legacy-path branch.Engine::Contextwithlegacy: trueand assertsStageBriefWriter#writecallsBrief.write, and withlegacy: false(or unset) asserts it calls@assembler.write— no.equal?/identity comparison remains in the diff.Subtask 5 — Resumer#abort! reads the terminal target instead of hardcoding it
Replace the literal
"failed"inResumer#abort!(plugins/os-sdlc/lib/os_sdlc/runner/resumer.rb) with a lookup against the ticket's table.TransitionTablealready hasterminal_states/terminal?(state)and validates the table at construction time (transition_table.rb), so the fix is either (a) a load-time assertion that every map'sterminal_statesincludes"failed"(raiseInvalidTableotherwise, at the same pointvalidate!already runs), keepingabort!'s literal but now backed by a guarantee, or (b) haveabort!read the target off the table (e.g. a designated "abort target" or the first/only terminal state) rather than assume the string"failed"at all. Pick (a) unless a map legitimately wants a different abort target than "failed" — the probe found no such map, so (a) is the smaller, lower-risk change; record the choice made."failed"interminal_statesfails loudly at map-load time (option a) or thatabort!correctly resolves to whatever the table designates (option b) — not both; pick one per the paragraph above.TransitionTablebuilt from YAML whoseterminal_statesomits"failed"raisesInvalidTableat load time (if option a), ORResumer#abort!against such a table still lands the ticket on the table's actual terminal state rather than an unknown/invalid one (if option b).plugins/os-sdlc/tests/runner/escalation_test.rband anyResumerabort test) continue to pass, and a new test documents the failure mode this closes: "unknown non-terminal state + deactivated escalation = unrecoverable ticket" (per the ticket's round-4 review comment) can no longer happen silently.Ordering / dependencies
1 and 2 are independent of each other and of 4/5; do them first (each is a clean, independently commit-able adapter). 3 depends on 1 and 2 landing. 4 and 5 are independent of 1/2/3 and of each other — either can land any time, in any order, on their own commits.
Global out-of-scope (whole ticket)
gh-based tracker adapter — this repo's tracker is Forgejo/teaonly (perdocs/issue-workflow.md).notify-sendcovers the current dev environment.transition_table.rb— none of these subtasks touch routing (ADR-0097).Additional line item from #341 round-4 review: Resumer#abort! hardcodes terminal 'failed' — add a load-time check that 'failed' is terminal in every map, or have abort! read its target off the table. Failure mode otherwise: unknown non-terminal state + deactivated escalation = unrecoverable ticket.
Work started on this ticket (subtasks 1-5, in decomposition order: 1,2 then 3, then 4,5).
All 5 subtasks done, one commit each (
3f33c63,3b8964c,8598f1d,ddec63a,688528d) + style cleanupee91639. TeaTracker + PushNotifier adapters TDD'd against the Null ports; both production call sites inject them. Probe findings: legacy_engine was a live silent fallback, so (user-approved) subtask 4 deleted it too — missing project.yaml now raises HarnessError loudly. Subtask 5 = option (a): YamlLoader raises InvalidTable unless a map's terminal list includes "failed". Suite 823 runs green, rubocop clean, opus review approved with no blocking findings. Left open for sign-off.