Skip to content

feat: session-close reports an open parent goal instead of asking - #315

Merged
bborbe merged 4 commits into
masterfrom
fix/session-close-goal-report-only
Oct 19, 2026
Merged

bborbe merged 4 commits into
masterfrom
fix/session-close-goal-report-only

Conversation

@bborbe

@bborbe bborbe commented Oct 19, 2026

Copy link
Copy Markdown
Owner

Why

A worker that finishes its anchor task under a goal that is still in_progress ends session-close on an operator-facing approve: line asking to confirm the goal stays open. Goal status is the managing session's call, so the worker parks on a question nobody needs to answer and keeps its spawn slot.

Measured 2026-10-19 by the Development Pipeline Throughput Manager: three finished workers in ~40 minutes — "Measure Free Capacity on the Nuke Dev, Nuke Prod and Quant Clusters", "Diagnose Whether the Serialized Cluster Agents Can Run More Than One Job", and "Count Only Operator-Typed Prompts as Interactions" (completed/done, 0 open boxes) — each ended on approve: leave [[…]] in_progress. At the time worker-sessions.py --count read 30 against spawn.maxConcurrent 30 soft / 50 hard, with 75 approved-not-started tasks waiting.

What changed

Phase 4.5 previously merged two different cases into one outstanding item. It now splits them:

  • A goal with ≥1 open task — the ordinary steady state (goals span 1–4 weeks, sessions do not), and its status is the manager's own write under the manager contract. It emits one report-only information line and nothing else:
    goal <title>: in_progress, <n>/<m> SC — managed by <manager> (or — no live manager)
    No outstanding item, no approve: / pick: / you run: / review: line, no AskUserQuestion naming the goal.
  • A closable goal (every task terminal, every SC ticked) — a decision is still owed, so it is routed to its live manager by SendMessage, and falls back to today's operator line when no live manager resolves.

Manager resolution: the goal page's manager_session_id, else the manager_session_id of a topic page whose ## Goals lists the goal. The id is a lead, never a verdict — only session-liveness.py --check exit 0 (LIVE) counts; ABSENT / UNKNOWN / AMBIGUOUS / MALFORMED and no id all read as no live manager. A worker never resumes or starts a manager, so SendMessage is the only action available.

The anchor-task block is unchanged.

Verification

  • make precommit — pass.
  • scripts/mode-membership-rule-test.sh — 13 passed, 0 failed.
  • scripts/check-changelog.sh — structure OK; fold guard run after merging origin/master, and the bullet was split back out of the v0.173.0 section the release cut had absorbed it into.

The task's own acceptance criteria need the deployed command (they read a real session-close transcript), so they run after release and local claude plugin update, not against this branch:

  • SC1: a real worker close under an in_progress goal emits no ask naming the goal and exactly one goal … information line.
  • SC2 probe A: a closable goal whose manager_session_id is live → SendMessage, no operator line.
  • SC2 probe B: a closable goal with a present-but-not-live id → operator line, no send (proves the liveness gate runs).
  • SC2 probe C: a closable goal with no id anywhere → operator line, no send.

Results land in the task's # Results section.

A task-anchored session that finishes its anchor no longer ends on an approve: line asking the operator to confirm leaving the parent goal in_progress. Phase 4.5 splits the two cases it used to merge:

- a goal with at least one open task is the ordinary steady state (goals span 1-4 weeks, sessions do not) and its status is the manager's own write, so it emits one report-only information line and never an outstanding item, approve: line, or AskUserQuestion;
- a closable goal (every task terminal, every SC ticked) still owes a decision, so it is SendMessage-routed to its live manager and falls back to today's operator line when no live manager resolves. Only session-liveness.py exit 0 counts as live; a worker never resumes or starts a manager.

The anchor-task block is unchanged.
…0 cut

The release cut renamed Unreleased to v0.173.0 between the branch point and the master merge, absorbing this branch's bullet into a released section that no tag contains. Split it back out per the fold guard.
fleet-surface.md's task-level chain ends at the Fleet Manager; this goal-level chain stops at no live manager, because a worker messages only a manager it can prove is live. Stating the difference keeps the two rules from reading as one.
@bborbe
bborbe marked this pull request as ready for review October 19, 2026 21:52

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now let me do the manual review of the diff, addressing each concern from the Plan.

Manual Review

Concern 1: Manager resolution exit code correctness

The diff correctly implements the exit code interpretation:

  • "Only exit 0 LIVE counts as a live manager; exit 1 ABSENT, 2 UNKNOWN, 3 AMBIGUOUS, 4 MALFORMED, and no id anywhere all read as no live manager."
  • ✅ Correct

Concern 2: Open goal vs. closable goal split

The diff correctly implements the split at lines 345-358:

  • "A goal with ≥1 open task — report only, never ask" → information line in Phase 9 summary
  • "A closable goal (every task terminal, every SC ticked) — a decision is owed" → SendMessage to live manager or operator line
  • ✅ Correct

Concern 3: SendMessage id-to-name mapping

The diff uses session-liveness.py --list --json which carries both sessionId and name — this is the correct source for the mapping. The ListAgents roster limitation (showing only registry ref) is explicitly noted.

  • ✅ Correct

Concern 4: Within-one-session repeat-run carry-forward

The carry-forward clause at line 362 correctly:

  • Suppresses re-flagging when a resolution already exists
  • Carries forward in the summary block, not the numbered list
  • Re-flags only if status or # Tasks changed
  • ✅ Correct

Concern 5: Goal-anchored exemption scope

The exemption correctly covers only the goal half of the gate (tasks still gated). The Phase 9 summary line (goal … information line) replaces the outstanding item for open goals.

  • ✅ Correct

Additional observation: The warning "⚠️ **manager-liveness.py --state --topic is not a substitute" is a good addition that prevents a plausible mis-use of the wrong tool.


{
  "verdict": "approve",
  "summary": "The session-close command gains a Phase 4.5 split that routes open goals (≥1 open task) to a report-only information line and closable goals to SendMessage/live manager or operator line. All five Plan concerns are correctly addressed: exit code semantics are precise, the open/closable split is well-implemented, id-to-name mapping uses the correct source, the within-one-session carry-forward logic is sound, and the goal-anchored exemption scope is correct. No blocking issues found.",
  "comments": [],
  "concerns_addressed": [
    {
      "concern": "correctness: Manager resolution uses session-liveness.py --check exit code",
      "disposition": "addressed",
      "detail": "Lines 341-342 correctly interpret exit 0 as LIVE only; exits 1/2/3/4 and no id all map to 'no live manager'"
    },
    {
      "concern": "correctness: The split between open goal and closable goal",
      "disposition": "addressed",
      "detail": "Lines 345-358 implement the split correctly: ≥1 open task → report-only info line; all terminal + all SC ticked → SendMessage or operator line"
    },
    {
      "concern": "security: SendMessage id-to-name mapping",
      "disposition": "addressed",
      "detail": "Lines 354-355 use session-liveness.py --list --json (which carries sessionId AND name) rather than ListAgents roster"
    },
    {
      "concern": "correctness: Within-one-session repeat-run carry-forward",
      "disposition": "addressed",
      "detail": "Lines 362-363 implement carry-forward correctly: summary block only, re-flag on status/Tasks change, suppression of approve: line"
    },
    {
      "concern": "correctness: Goal-anchored exemption scope",
      "disposition": "addressed",
      "detail": "Lines 378-381 correctly scope exemption to goal half only; task half still gates; Phase 9 info line replaces outstanding item"
    }
  ]
}

@bborbe
bborbe merged commit 5cdeac4 into master Oct 19, 2026
5 checks passed
@bborbe
bborbe deleted the fix/session-close-goal-report-only branch October 19, 2026 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant