stacklok / stacklok/mecatl

teams: per-transition fidelity for the team.tasks/team.findings snapshot stream (capture-at-emit)

Open
#59 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Go
Stars
152
Forks
16
Avg merge
14h 48m
Merged PRs (30d)
536

Description

Context

Deferred follow-up surfaced while deflaking TestTeamToolStreamsTaskSnapshots (PR #58, merge 0d2b20b). Documented as the "team-snapshot fidelity note" in docs/design/IMPLEMENTATION-NOTES.md.

The first-class team.tasks / team.findings snapshot stream is change-driven and eventually-consistent, NOT per-transition-guaranteed. The Team-tool sink projects each snapshot from a live tm.Tasks() / tm.Findings() read at the moment the single buffered forwarder goroutine (evCh, depth 64) drains the event — decoupled in time from the member/supervisor goroutines that mutate the list (engine/agent/teamtool.go ~386-389).

Consequence: under scheduler load the forwarder can lag until a task is already completed; every drained event then reads the same terminal state, and the intermediate pending / in_progress snapshots coalesce away (the de-dup collapses them). A client (the mecatui task panel) watching for an in_progress flicker may never see it.

Severity

Minor / observability-fidelity only. The terminal state is always correct (the last live snapshot converges and EvTeamEnd re-reads the settled list), and the stream never shows a WRONG state — it can only skip an intermediate one under load. This is a documented best-effort contract, not a correctness bug. No action is required unless per-transition visibility becomes a product requirement for the task panel.

The fix (when picked up)

Capture the frozen snapshot at the mutation rather than sampling live state at forward-time, so each transition rides evCh as its own frozen value (causally ordered: a task cannot complete before it is claimed). Two shapes were scoped:

  • B1 (cleanest): an optional change-callback on the team.Team aggregate (engine/team/team.go), fired under t.mu at the tail of every mutator (CreateTask/ClaimTask/ClaimNext/CompleteTask/ReleaseTasks); the supervisor wires it to push a frozen projectTeamTasksSnapshot onto evCh. Captures every edge incl. the supervisor's silent auto-claim (planRound ClaimNext, which emits nothing today).
  • B2: plumb an emit hook into MemberTools + an explicit supervisor-side push after the auto-claim/release edges. Leaves engine/team pure but spreads the emit responsibility and changes the MemberTools signature.

Proto/wire: noneTeamPayload.tasks (proto field 15) and the mecatui routing are already fully wired; no task generate.

The hazard (why it was deferred, not done inline)

This touches the historically deadlock-prone team-emit path. Pushing a snapshot synchronously onto the bounded evCh while holding t.mu is a lock-ordering inversion: if evCh is full (consumer backpressure / a stalled forwarder), the mutating op blocks on the send while holding the team lock, wedging every other team operation — exactly the class the seal / hardAbort / try-send-first machinery exists to prevent. The safe form is capture-under-lock, send-after-unlock through an abort-aware enqueue, with a new send-vs-seal ordering invariant to reason about. That deserves its own design + review + stress arc, not a bundled change.

Acceptance sketch

  • Each pending → in_progress → completed transition surfaces as a distinct streamed team.tasks snapshot, deterministically, under GOMAXPROCS=1 stress (the original TestTeamToolStreamsTaskSnapshots per-transition assertion — snapshots[0] non-completed — can be restored and passes -count=5000).
  • No deadlock/backpressure regression: the team aggregate never blocks on evCh while holding t.mu; the seal/hardAbort discipline holds (a goleak + cancel-while-parked stress pass stays green).
  • De-dup stays the wire-volume guard; the findings stream gets the same treatment for consistency.

References

  • docs/design/IMPLEMENTATION-NOTES.md — "team-snapshot fidelity note"
  • engine/agent/teamtool.go (the sink, ~371-417), engine/agent/teamsupervisor.go (the forwarder + planRound auto-claim), engine/team/team.go (the aggregate mutators)
  • Deflake: PR #58 (0d2b20b)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with engine/agent/teamtool.go, engine/agent/teamsupervisor.go, engine/team/team.go, and the team-snapshot fidelity note in docs/design/IMPLEMENTATION-NOTES.md; run TestTeamToolStreamsTaskSnapshots to reproduce the deferred behavior. Done means distinct pending, in_progress, and completed snapshots under stress, with no team-lock blocking on evCh and the seal, cancellation, and backpressure tests remaining safe.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, distributed-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.