Skip to content

overseer: advisor-gated escalation + auto-issue filing (#1962) - #2096

Merged
jwbron merged 44 commits into
mainfrom
egg/issue-1962
Apr 26, 2026
Merged

overseer: advisor-gated escalation + auto-issue filing (#1962)#2096
jwbron merged 44 commits into
mainfrom
egg/issue-1962

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

Closes #1962. The deployed agent-side overseer is "pretty good" at
escalating to humans via OVERSEER_ALERT but does not always do so when
appropriate, and it cannot file GitHub issues even when the underlying
plumbing exists. At the same time, the host /sdlc skill is doing
mid-pipeline debugging work (stall detection, NACK escalation, agent
nudging, long-running-phase rescue) that the issue says belongs in the
overseer so the host can be a thin reporter / HITL surface.

This PR ships three coupled threads under the
advisor-strategy framing:

  1. Advisor-gated escalation tuning. Haiku 4.5 keeps driving the
    overseer cycle (max_turns=1); when it flags an anomaly AND a
    Tier-1 health alert has tripped, an Opus 4.6 advisor is invoked
    (via a new orchestrator-exposed MCP tool — see "Implementation
    choices" below) to decide alert composition / priority / whether
    to recommend a GitHub issue. New config knobs
    (overseer_advisor_model, overseer_auto_file_issues_mode)
    expose the advisor; no advisor cap is added in this PR
    per decision-19's "no cap for now" resolution — the
    existing max_llm_cost_per_hour=$5 envelope remains the
    budget control, with a follow-up issue tracking advisor-
    specific budget. The gate matches the precedent shipped in
    Overseer agent-heartbeat-stall alert fires prematurely on every refine phase #2012. The advisor
    itself lives in shared/overseer/advisor.py so it is importable
    from both the orchestrator-side MCP-tool handler and any future
    orchestrator-side caller without crossing the sandbox/orchestrator
    package boundary.
  2. Auto-issue filing via a new egg-orch overseer file-issue
    CLI verb.
    The CLI verb runs gh issue create itself, inside
    the sandbox
    , mediated by the gateway — there is no
    orchestrator-side endpoint that runs gh (this is decision-9
    opt-1; the previously sketched orchestrator REST endpoint that
    would have invoked gh is dropped). The dead-code template at
    orchestrator/overseer/issue_filer.py:86-107 is preserved as
    the canonical literal source and marked DEAD; the live body
    composition lives sandbox-side in
    sandbox/egg_lib/overseer_issue_body.py. Issues land with the
    existing agent:overseer label plus the matching priority
    (p0/p1/p2/p3) — no new labels are created. The advisor
    returns recommendation=file_issue inside an OVERSEER_ALERT
    so the human gates the actual filing via the existing
    pending_decisions HITL flow (shadow-mode rollout). Dedup
    searches per-repo by agent:overseer + an anomaly signature
    computed from (anomaly_type, agent_role, repo) and caches its
    findings in .egg-state/oversight/filed-issues.jsonl
    (append-only JSON Lines, header line
    {_kind: "header", schema_version: 1}). The title carries the
    8-char signature prefix so the gh issue list --search …
    fallback is reliable. Bodies are run through a new
    shared/overseer/scrubbing.py pass that removes
    ghp_* / ghs_* / gho_* / ghu_* / ghr_* PATs, AKIA*
    AWS keys, Slack webhook URLs, and GITHUB_TOKEN=* exports
    before the advisor returns its verdict; the gateway does the
    same scan as defense-in-depth.
  3. Host → overseer migration with a side-by-side calibration
    window.
    Stall / silent-agent / NACK / long-running-phase /
    stuck-pipeline-rescue detection moves out of
    skills/sdlc/SKILL.md:489-639 into the overseer. The
    in-memory {role: {phase, phase_entered_at, …}} state map
    moves to .egg-state/oversight/agent-timing.json (overseer-
    owned, no orchestrator schema change). Thresholds become
    per-pipeline configurable via PipelineConfig defaulting to
    current values, with stuck-phase-transition bumped from
    ~60 s to 180 s per the human's calibration feedback. A new
    overseer_owns_host_detection: bool = False knob keeps the
    host's detection logic live by default for one release; the
    follow-up PR flips the default and deletes the dormant code.
    /sdlc keeps surfacing alerts, handling HITL decisions, and
    running the Phase-5 final-handoff rescue prompts.

Impact. The host becomes a thin reporter the way the issue
asks for; the overseer escalates more reliably and on a tighter
cost gate; auto-issue filing is gated through HITL while the
policy is calibrated. Existing OVERSEER_ALERT consumers see no
schema change beyond a new optional recommendation field; the
backwards-compat regression test asserts pre-#1962 alert payloads
still parse and render. Cost stays inside the existing
max_llm_cost_per_hour=$5 envelope; a follow-up issue tracks
dedicated advisor budgeting. /sdlc regression behavior stays
identical at first because (a) threshold defaults match today's
hard-coded values and (b) overseer_owns_host_detection=False
keeps the host detectors firing during the calibration window.

Test Plan

Automated:

  • Orchestrator tests:
    • test_overseer_issue_filer.py — assertion that the existing
      template literal at lines 86-107 is byte-for-byte preserved
      (the canonical-literal-source contract); no other behavior is
      exercised since the module is marked DEAD.
    • test_overseer_advisor_tool.py (NEW) — orchestrator-side MCP
      tool surface for the advisor (handler returns valid
      AdvisorVerdict JSON; auth-gates to the overseer role; the
      executor → advisor prompt-contract from decision-20 opt-3
      is enforced).
    • Existing tests (test_overseer_monitor.py,
      test_overseer_alert_isolation.py,
      test_overseer_hitl_integration.py,
      test_overseer_decision_maker.py) updated for the new
      action vocabulary, the optional recommendation field on
      OVERSEER_ALERT, and the new agent-timing schema.
    • test_overseer_alert_isolation.py — backwards-compat
      regression assertion: a serialized pre-Improve overseer escalation/issue opening behavior #1962
      OVERSEER_ALERT payload (no recommendation field) round-
      trips through the message store and renders in the /sdlc
      alert-display path verbatim.
  • Sandbox tests:
    • test_egg_orch_overseer_file_issue.py (NEW) — CLI verb
      argparse coverage; required-arg validation; happy path
      through gateway-mocked subprocess.run(["gh", "issue", "create", …]); failure path on gh non-zero exit; dedup-hit
      path returns dedup_match without invoking gh.
    • test_overseer_issue_body.py (NEW) — body composition
      helper produces the template body with all five Pipeline
      Links sub-block fields; title format embeds the 8-char
      signature substring; running scrub_secrets over a body
      with embedded ghp_… / AKIA… / hooks.slack.com/… /
      GITHUB_TOKEN=… substitutes them with [REDACTED:<kind>].
    • test_overseer_advisor_invocation.py (NEW) — sandbox-side
      invocation of the orchestrator MCP advisor tool produces a
      populated AdvisorVerdict; trigger gate must require Haiku
      flag AND Tier-1 alert before the tool is called;
      max_uses_per_phase cap respected.
  • Shared-package tests:
    • shared/tests/test_overseer_advisor.py (NEW) — pure unit
      coverage of the consult_advisor function with mocked
      run_agent_async for each decision outcome.
    • shared/tests/test_overseer_scrubbing.py (NEW) — secret
      patterns covered with positive + negative cases.
  • Gateway tests:
    • test_gh_overseer_file_issue.py (NEW) — gh issue create
      from the overseer role allowed; non-overseer role denied;
      labels auto-injected; title >120 chars rejected; body

      50 KB rejected; cross-repo target rejected; defense-in-
      depth secret-pattern rejection on body.

  • Integration tests:
    • integration_tests/test_overseer_auto_issue_filing.py (NEW)
      — end-to-end: synthetic anomaly → Haiku flag + Tier-1 alert
      → advisor recommends file_issue → OVERSEER_ALERT carries
      recommendation → HITL decision created → human approves
      (script-driven provide_input) → sandbox CLI invokes
      gateway-mocked gh issue create with agent:overseer +
      priority labels and the title contains the 8-char
      anomaly-signature substring → re-trigger the same anomaly →
      dedup skips the second gh call.
  • Skill regression:
    • skills/sdlc/tests/test_host_migration.py (NEW) — under
      overseer_owns_host_detection=False the host's detectors
      still fire (calibration mode); under =True they no longer
      fire and the overseer's OVERSEER_ALERT arrives at the
      host's surfacing logic with the same content as today.

Manual:

  • Run /sdlc submit-task <synthetic issue>; under
    overseer_owns_host_detection=True, inject a long synthetic
    stall (>180 s) and verify the overseer (not /sdlc) emits the
    agent-stall alert.
  • Inject a repeated-error anomaly that meets both the
    Haiku-confidence threshold AND a Tier-1 health alert; verify
    OVERSEER_ALERT.recommendation == "file_issue" arrives;
    approve the surfaced HITL decision; verify a GitHub issue
    lands with agent:overseer + the matching priority label,
    the title contains the 8-char anomaly-signature substring,
    and the body has the extended Pipeline Links sub-block.
  • Re-trigger the same anomaly; verify the overseer logs a dedup
    hit and does NOT re-recommend filing.
  • Smoke-test /sdlc thinness: confirm that during a healthy
    pipeline the host emits no stall / NACK / long-run alerts of
    its own when overseer_owns_host_detection=True (those
    should now come from the overseer if at all).

Manual Steps

Pre-merge:

  1. Mark decision-15 (overseer file-boundary policy for
    .egg-state/oversight/) formally resolved with the
    no-op rationale: OVERSEER_PATTERNS at
    shared/egg_restrictions/patterns.py:526-527 already
    permits writes under .egg-state/oversight/. No new
    allowlist entry needed. (Reviewer non-blocking note:
    contract decisions linger as "Defer" forever unless a
    human or planner closes them; this one belongs to the
    planner.)

Post-merge:

  1. File the SDK-pin-bump follow-up issue (advisor Option A
    swap) once Anthropic ships advisor_20260301 inside the
    >=0.1.65,<0.2 window — nothing in this PR blocks the
    follow-up, but the SDK pin currently doesn't expose the
    capability.
  2. File the advisor-budget follow-up issue that
    decision-19 carved out — track an
    overseer_advisor_max_uses_per_phase (or equivalent
    cost-bound) knob and decide a default after measuring
    advisor-call distribution from production data. This
    follow-up is the only sanctioned reintroduction path for
    the cap.
  3. File the metrics-instrumentation follow-up issue for
    feedback-1.Q6.(d) (auto-filed issue acceptance rate —
    closed-as-invalid vs. accepted-and-fixed). TASK-7-5 in
    this PR ships the four log-line shapes (a/b/c) but the
    (d) disposition metric needs GitHub-issue-state polling
    which is out of scope here.
  4. Run the production calibration window (≥2 weeks) with
    overseer_owns_host_detection=False (default). Once
    overseer-side detectors are showing parity or improvement
    relative to /sdlc host-side detectors, file the
    host-migration cleanup follow-up PR that flips the default
    to True and deletes the now-dormant /sdlc detection code
    blocks (TASK-6-2 keeps them gated, not deleted, in this PR).

Pipeline Context

Pipeline: issue-1962
Issue: #1962

Per-phase BRC transcripts: refine, plan, implement.

Authored-by: egg

egg-orchestrator and others added 28 commits April 25, 2026 15:35
Surfaces three threads in the issue — escalation tuning, autonomous
issue filing, and host→overseer migration — and recommends Option B
(escalation tuning + auto-issue in this pipeline, host migration as
a follow-up).

Key finding: `orchestrator/overseer/issue_filer.py::file_diagnostic_issue`
and the full OverseerMonitor decision ladder already exist but are
dead code — only referenced in tests. The deployed overseer is the
sandbox LLM agent under `sandbox/agent-config/rules/overseer.md`,
which explicitly forbids filing issues. Auto-issue is therefore a
wiring problem, not a greenfield capability.

Registered 16 choice decisions and 7 open-ended feedback questions
covering scope, dedup, labels, rollout, thresholds, and cross-issue
coordination (#1722, #1727, #1786, #1806, #1902).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Blocking fixes:
1. Insert inline `<!-- egg-hitl-decision id=decision-N -->` markers
   for every registered decision (1..16). Contract had the decisions
   but draft was missing the per-question markers; fixed by
   restructuring "Open Questions" to reproduce each decision inline
   with its registered options plus recommended-option tags.
2. Remove the duplicate decision-15 cross-reference (it lived in
   both auto-issue-filing and interaction-with-existing-issues
   sections). Single authoritative block now.
3. Resolve decision-11 vs decision-1 redundancy: explicitly tag
   decision-11 as conditional on decision-1 ≠ Option B/D, with a
   dependency note inside the decision body.

Non-blocking improvements:
- Annotate decision-9 opt-4 ("reuse dead-code OverseerMonitor")
  with explicit "not recommended" caveat (re-introduces non-agent
  decision pipeline).
- Clarify decision-4 "Sonnet-gated" means in-loop reasoning, not a
  separate orchestrator-side classifier service.
- Cross-reference #1932 (closed) alongside #1971 in the interacting-
  issues list.
- Add Tests/regression risk subsection to Complexity Assessment
  (test_overseer_*.py, test_overseer_issue_filer.py, gateway tests,
  integration_tests gap).
- Tighten SKILL.md line citation to 1359-1383 (stall + NACK block).
- Tag plan-phase-candidate decisions (12, 13, 14, 16, feedback-1
  Q5/Q6/Q7) so the human can leave them unanswered at the refine
  gate.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reframe the overseer escalation/issue-filing/host-migration work
under Anthropic's advisor strategy (Haiku executor, Opus advisor).
Carry forward pre-refine resolved preferences (scope, labels, dedup,
sub-agent scope, related-bugs scope) as constraints. Surface the
remaining load-bearing HITL items as new decisions (17-23) covering
advisor calibration, budget, prompt contract, gate placement,
host-migration sequencing, label confirmation, and
native-tool-vs-two-call implementation choice.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address all 4 blocking findings from reviewer_refine NACK 93cf8334-699a-46:

1. Decisions 17-23 verified present on contract via mcp__sdlc__show_contract
   (the reviewer's snapshot was stale; re-verification before this re-propose
   confirms all 7 are registered).
2. Add `<!-- egg-hitl-decision id=decision-N -->` markers above every open
   decision (carry-overs 6, 8, 9, 10, 12-16; new 17-23). Add
   `<!-- egg-hitl-feedback id=feedback-1.QN -->` markers for Q1-Q7.
   Reproduce each decision inline with question + options + (Recommended) tags.
3. Fix off-by-one: 7 new decisions (17-23), not 6. Status table, mapping table,
   prose, and registration calls now agree.
4. Untangle decision-10 vs decision-22: rollout (decision-10) and host-migration
   sequencing (decision-22) are distinct questions; remove "folded into" claim;
   mapping table cites them separately.

Non-blocking nits also addressed:
- egg-orch CLI citation: 2549-2597 (subparser at 2553, alert parser at 2556)
- file_diagnostic_issue caller: monitor.py:624 (inside the dead class itself)
- #1902 cross-ref: concrete — opt-2 needs zero file-boundary work
- Authored-by trailer moved to end-of-file
- pip show claude-agent-sdk breadcrumb added for SDK spike
- BrowseComp claims cited to advisor-strategy blog post URL

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…to-issue filing, host migration

Decomposes the refine analysis into 8 phases / 18 tasks landing in one
PR per decision-22:
  Phase 1: SDK spike + config knobs + state schemas
  Phase 2: Gateway + egg-orch overseer file-issue CLI verb
  Phase 3: Issue body template revival + dedup
  Phase 4: Advisor wiring (two-call pattern, Option B per spike)
  Phase 5: Rule-doc rewrite (lift the issue-filing prohibition)
  Phase 6: Host -> overseer migration of /sdlc detectors
  Phase 7: Tests (orchestrator + gateway + integration + skill regression)
  Phase 8: Docs

Includes the SDK capability-spike result: claude-agent-sdk 0.1.65 does
not expose advisor_20260301 / max_uses, so the implement phase will
ship Option B (two-call advisor pattern) with Option A unlocking as a
clean follow-up swap if the SDK upgrades within the >=0.1.65,<0.2 pin.

Honors all 23 resolved refine decisions and the seven feedback-1
answers (Tier-1 intersection gate, shadow-mode rollout, agent:overseer
+ priority labels only, .egg-state/oversight/filed-issues.json dedup,
180s stuck-phase-transition default, no per-pipeline issue cap).
…st migration

Deliver the plan-phase architect output for issue #1962 covering all three
in-scope threads (escalation tuning, autonomous issue filing, host->overseer
migration) under the advisor-strategy framing locked in during refine.

Resolves all 23 HITL decisions to a concrete implementation site:
- decision-23 SDK capability spike: claude-agent-sdk 0.1.65 does NOT expose
  advisor_20260301 / max_uses; Option C resolves deterministically to
  Option B (two-call advisor pattern)
- decision-18 intersection gate: advisor invoked iff Haiku confidence > 0.8
  AND >= 1 Tier-1 health alert present (#2012 generalization)
- decision-21 + decision-10: advisor recommends, human gates via existing
  pending_decisions HITL surface; shadow-mode rollout default
- decision-22: single PR with all three threads
- decision-12: PipelineConfig-driven thresholds; bump
  stuck-phase-transition default to 180s per feedback-1.Q2

Output written to .egg-state/agent-outputs/1962-architect-output.json
covering: 11 components, 12 technical decisions, 7 regression risks for
the risk_analyst, test strategy outline (unit + integration + regression),
and an explicit decision-to-resolution mapping covering all 23 HITL items.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Risk assessment for the overseer escalation/auto-issue/host-migration
work (issue-1962, advisor-strategy framing). Key findings:

- R-COMPAT-01 (verified): vendored claude-agent-sdk 0.1.65 does NOT
  expose advisor_20260301 / max_uses. Decision-23 Option A requires
  an SDK upgrade; default to Option B (two-call pattern).
- R-COST-01: decision-19 deferred the advisor budget cap; recommend
  shipping a defensive per-phase soft cap (max_uses_per_phase=5) to
  bound runaway-pipeline cost.
- R-COMPAT-03: PR #2011/#2016 calibration is recent and easy to
  regress; recommend feature-flagging host-migration so /sdlc and
  overseer can run side-by-side for calibration.
- R-OP-02: dedup signature is unspecified — recommend
  (anomaly_type + agent_role + repo), explicitly excluding pipeline_id.
- R-SEC-01/02: gateway must enforce label injection, size caps,
  repo pinning, and secret scrubbing on gh issue create from overseer.

5 human-review areas flagged. Rollback plan based on layered feature
flags so each thread (advisor / auto-issue / host-migration) can be
disabled at runtime without revert.

Authored-by: egg
…allback, mode

Resolve all 4 blocking + 7 non-blocking issues raised by reviewer_plan:

Blocking:
1. decision-9 opt-1 fix: agent-side CLI runs `gh issue create` ITSELF
   inside the sandbox via the gateway; no orchestrator-side endpoint
   invokes gh. New sandbox/egg_lib/overseer_issue_body.py helper for
   body-building. orchestrator/overseer/issue_filer.py kept ONLY as
   the canonical template literal source (marked DEAD).
2. filed-issues schema: switched to JSON Lines (.jsonl) per the
   append-only semantics; agent-timing.json stays single-object.
   Header line `{_kind: "header", schema_version: 1}` for format
   detection without a sidecar.
3. /sdlc overseer-absent fallback promoted from risk-mitigation prose
   to a concrete component_breakdown deliverable with explicit trigger
   conditions.
4. Dropped 'off' mode from overseer_auto_file_issues_mode Literal
   (decision-10 sanctioned shadow/live only; full disable uses
   existing overseer_enabled=False).

Non-blocking:
- Infra-error fast-path lives in NEW shared/egg_overseer_helpers/
  infra_error.py (single source of truth; agent + dead orchestrator
  code both import).
- Gateway allow-rule wording corrected: gh issue create is blocked
  by default-deny pattern (#1494), not _OVERSEER_BLOCKED_GH_OPS.
- EGG_PIPELINE_REPO env injection sized as a verify-and-inject
  component for the planner.
- PipelineConfig delivery via /status sized as a verify component.
- overseer_advisor_model PINNED to claude-opus-4-20250514.
- Token-budget framed as architect estimate; risk_analyst sizes.
- Test pruning footprint enumerated in regression risks; sized via
  pytest case count instructions for the planner.
- open_questions resolved by architect (mark-unused, no new endpoint,
  pinned opus); section renamed architect_resolved_questions.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…, scrubbing

Resolve all 5 BLOCKING + 11 NON-BLOCKING items raised by reviewer_plan
on the original 8-phase plan.

Blocking fixes:
1. decision-9 opt-1 architectural fix: agent CLI in TASK-2-1 runs
   `gh issue create` ITSELF inside the sandbox via the gateway. The
   orchestrator REST endpoint that previously composed and ran gh is
   DROPPED. Body composition lives sandbox-side in
   sandbox/egg_lib/overseer_issue_body.py (TASK-3-1); orchestrator/
   overseer/issue_filer.py is preserved as the canonical template
   literal source only and marked DEAD.
2. Advisor module relocated from orchestrator/overseer/advisor.py to
   shared/overseer/advisor.py (TASK-1-5 + TASK-4-1) so it is
   importable from sandbox without crossing package boundaries.
3. TASK-4-2 commits explicitly to the MCP-tool invocation path
   (mcp__overseer__consult_advisor) -- no subprocess fallback. Tool
   surface, schema, and auth-gating to overseer role are spec'd.
4. New TASK-3-2 adds shared/egg_overseer_helpers/scrubbing.py with
   patterns for ghp_*/ghs_*/gho_*/ghu_*/ghr_* PATs, AKIA* AWS keys,
   Slack webhooks, and GITHUB_TOKEN=*/GH_TOKEN=*/ANTHROPIC_API_KEY=*
   exports. Advisor scrubs at return time; gateway scrubs as
   defense-in-depth.
5. compute_anomaly_signature in TASK-1-3 now uses
   (anomaly_type, agent_role, repo) -- repo from EGG_PIPELINE_REPO
   per decision-5 + R-OP-02. error_class field is dropped.

Non-blocking fixes:
- AgentTimingEntry adds last_alerted_at + alerted_anomalies fields
  for per-anomaly suppression in TASK-6-1 detectors.
- Title format becomes
  `[Pipeline Diagnostic] {anomaly_type} - {agent_role}
   [{anomaly_signature[:8]}]` so gh issue list --search is reliable.
- Body composition: advisor populates issue_title + issue_body
  (option a); CLI passes them through.
- overseer_auto_file_issues becomes a Literal["shadow","live"] mode
  (drops 'off'); HITL flow runs in both modes -- mode only controls
  whether gh is called once approval lands.
- New overseer_owns_host_detection: bool = False knob keeps /sdlc's
  host-side detectors live during a calibration window; Phase 6
  gates the deletions on this flag (mitigates R-COMPAT-03 / HR-03).
- TASK-3-3 (alert schema change) precedes TASK-4-2 (advisor wiring);
  Phase 3 precedes Phase 2 since CLI imports from issue_body helper
  and find_existing_issue.
- New TASK-7-5 metric instrumentation task per R-OP-05 emits
  structured "overseer_event" log lines at four sites.
- Backwards-compat regression test added in TASK-7-1 for legacy
  OVERSEER_ALERT payloads (R-COMPAT-05).
- TASK-7-6 explicitly asserts title contains signature substring.
- New TASK-1-4 verifies-and-injects EGG_PIPELINE_REPO env var.
- Gateway line numbers cited: agent_restrictions.py:153 (blocked
  ops list) and :193-218 (check_agent_gh_operation). Notes that
  `issue create` is not in the blocked list -- the change is
  adding an allow-rule, not removing from a deny-list.
- /sdlc overseer-absent fallback promoted to a concrete component
  with a single AskUserQuestion (4 options).
- Phase 5 rule doc has verbatim phrases for grep-based acceptance:
  "Dedup before recommend", "Flag-vs-HITL gating", "Secret scrubbing".

Total tasks: 23 across 8 phases (was 14 across 8). All tasks have
role + files + acceptance criteria.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… Opus model pin

Fresh risk assessment for the architect's revised analysis (decision-9
opt-1 sandbox-side gh; JSONL filed-issues; /sdlc overseer-absent
fallback as concrete deliverable; pinned advisor model; no 'off' mode).

Key findings vs the v1 risk pass:
- NEW BLOCKER R-COMPAT-01: architect's overseer_advisor_model pin
  'claude-opus-4-20250514' is the OLDER Opus 4 generation, NOT the
  'claude-opus-4-6' the egg codebase canonicalizes (shared/egg_harness/
  config.py:17) AND not the only model the public advisor tool currently
  supports. shared/egg_harness/cost.py has no row for that ID so cost
  telemetry will fail. Recommend changing default to claude-opus-4-6.
- NEW R-COMPAT-04: EGG_PIPELINE_REPO env var the gateway --repo
  restriction depends on does NOT exist anywhere today (verified by
  grep across sandbox/, orchestrator/, gateway/, shared/). Implement
  phase must add to kubernetes_spawner.py + sandbox/entrypoint.py;
  gateway must default-DENY when unset.
- NEW R-COMPAT-05: architect dropped 'off' value from
  overseer_auto_file_issues_mode per reviewer NACK; only escape is
  overseer_enabled=False which kills three features. Recommend re-add
  'off' value or sibling bool.
- NEW R-COMPAT-08/09: cross-cutting import path concerns for the
  template literal + infra-error helper extraction.
- NEW R-OP-06: /sdlc overseer-absent fallback "fires AT MOST ONCE per
  phase" needs explicit per-phase memory mechanism.
- NEW R-PERF-02: filed-issues.jsonl periodic compaction needs flock to
  avoid lost records on concurrent writes.
- NEW R-COMPAT-11: PipelineConfig delivery via status endpoint is
  unverified -- the new threshold knobs may be inert without endpoint
  changes.

Carry-over (still relevant from v1, updated for v2 architect):
- R-COST-01 advisor budget unbounded (decision-19 deferred)
- R-OP-01 shadow-mode HITL noise overwhelming operator
- R-OP-02 dedup_signature shape ambiguous (architect proposes
  pipeline_id+phase; risk pass recommends excluding both)
- R-OP-04 host-migration regression vs PR #2011/#2016 calibration
- R-SEC-01 secret leakage in public issue body
- R-SEC-02 gateway label injection must STRIP user labels first
- R-SEC-04 concurrent-write races on .egg-state/oversight/

Total: 25 risks across 6 categories. 8 require human review and are
surfaced in human_review_areas with concrete questions.

SDK spike confirmed: claude-agent-sdk 0.1.65 ServerToolName Literal
includes 'advisor' for response parsing only; no client-side tool
definition support, no max_uses, no advisor-tool-2026-03-01 beta header.
ClaudeAgentOptions.betas accepts only Literal['context-1m-2025-08-07'].
Option B (two-call pattern) is correct choice; HR-02 recommends
verifying claude-agent-sdk 0.1.66 (one patch ahead, fits inside the
existing >=0.1.65,<0.2 pin) before committing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…aths, priority mapping

Resolve all 6 BLOCKING + 10 NON-BLOCKING items raised on plan v2.

Blocking fixes:
1. Dropped overseer_advisor_max_uses_per_phase entirely from TASK-1-2
   and TASK-4-2 per decision-19 ("no cap for now -- handle budget
   separately"). Acceptance criteria now ASSERT the knob's absence
   in PipelineConfig and grep for max_uses_per_phase in production
   code returns no matches (regression guard).
2. TASK-3-3 file path corrected from non-existent
   orchestrator/routes/overseer_alert.py to the real
   orchestrator/routes/pipelines.py (where OVERSEER_ALERT handling
   lives at lines 238, 461-473, 5791). Description now instructs
   the implementer to find the schema model by exploration first
   and cite the path in the commit message.
3. Dropped `_kind: Literal["record"]` from FiledIssueRecord (Pydantic
   v2 strips underscore-prefixed names from JSON output). Header line
   is now a hand-written dict literal in append_filed_issue, not a
   Pydantic dump. TASK-7-3 round-trip assertion verifies no `kind`
   key leaks into serialized records.
4. Added explicit priority-dimension mapping in TASK-1-2 +
   shared/overseer/priority.py (TASK-1-5): low|medium|high <->
   p3|p2|p1; p0 reserved for opt-in human escalation. Two helpers
   alert_to_label / label_to_alert. TASK-4-2's decision="alert"
   branch calls label_to_alert before invoking egg-orch overseer
   alert.
5. TASK-1-4 dropped the silent gh-repo-view fallback. EGG_PIPELINE_REPO
   now MUST be injected by the orchestrator; sandbox/entrypoint.py
   raises EnvironmentError if missing. test_entrypoint.py extended
   with a fail-fast acceptance.
6. Cross-phase dedup persistence honesty: new "Dedup persistence
   scope" section in Approach states the local JSONL cache is
   intra-phase only; cross-phase dedup relies on title-embedded
   8-char anomaly-signature + gh issue list --search.

Non-blocking fixes:
- Stall threshold split into overseer_stuck_phase_transition_seconds
  + overseer_agent_stall_seconds (both default 180s) to remove the
  double-duty ambiguity flagged by the reviewer.
- TASK-2-2 reframed: gh issue create from overseer is ALREADY allowed
  by the gateway (issue create is not in _OVERSEER_BLOCKED_GH_OPS).
  The change is adding additional guardrails (label injection, repo
  enforcement, body-size, secret scan), not flipping a deny->allow.
  Adds note to audit gateway/gateway.py for parallel-policy layers.
- TASK-4-2 cap-exceeded path is moot (no cap shipped); the prior
  STATUS-log-only path is gone.
- TASK-6-2 reframed in terms of stable section headings (### Stall
  Detection, ### Silent-Agent Detection, etc.) instead of absolute
  line numbers that drift mid-PR.
- New "Implementation order: 1 -> 3 -> 2 -> 4 -> 5 -> 6 -> 7 -> 8"
  callout in Approach so the implementer reading top-to-bottom
  doesn't hit Phase 2's imports before Phase 3 defines them.
- TASK-7-1 dead-code audit extended to grep for file_diagnostic_issue,
  _build_issue_body, DIAGNOSTIC_LABELS, AND OverseerMonitor across
  orchestrator/sandbox/gateway/shared. Stale imports keep the dead
  module alive accidentally.
- Consolidated the previously-split shared/overseer/ +
  shared/egg_overseer_helpers/ packages into a single
  shared/overseer/ package (advisor.py, scrubbing.py, infra_error.py,
  priority.py, state.py).
- TASK-7-6 explicit acceptance: integration test uses tmp_path
  fixture for .egg-state/oversight/ so cycle-2 dedup actually reads
  the JSONL written by cycle-1 (not a mock).
- TASK-7-7 fallback test explicitly fires only under
  overseer_owns_host_detection=True (the only state where the host
  has no detectors active).
- New manual_steps: file the metrics-instrumentation follow-up
  for feedback-1.Q6.(d) issue-acceptance-rate; file the
  advisor-budget follow-up per decision-19.
- Pre-merge manual step to formally close decision-15 with the no-op
  rationale (OVERSEER_PATTERNS already permits .egg-state/oversight/).

23 tasks across 8 phases unchanged. YAML parses cleanly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…-blocking pull-forwards

Resolve the 1 BLOCKING + 8 NON-BLOCKING items raised on v3.

Blocking:
- TASK-1-2: overseer_advisor_model changed from claude-opus-4-20250514
  to claude-opus-4-6, the canonical opus alias defined at
  shared/egg_harness/config.py:17 and the only Opus-class entry with
  a cost-data row at shared/egg_harness/cost.py:18. The earlier ID
  would have silently broken the max_llm_cost_per_hour=$5 self-monitor
  because cost.lookup() returns no row. Cost-row regression assertion
  added to TASK-1-2 acceptance: assert
  egg_harness.cost.lookup(pc.overseer_advisor_model) is not None.

Non-blocking pull-forwards (all 8 risk_analyst items pulled forward
to keep implement-phase surprises down):
1. TASK-1-3: load_agent_timing / save_agent_timing helpers MUST
   acquire fcntl.LOCK_EX on .egg-state/oversight/agent-timing.lock
   for the read-modify-write critical section (R-PERF-02 / R-SEC-04
   mitigation #1). Concurrent overseer respawns at phase boundaries
   can race on the read step without this.
2. TASK-2-1: gh issue create now uses --json url,number,title and
   parses json.loads(stdout)["number"] instead of regex over URL
   suffix (R-COMPAT-10).
3. TASK-3-1: extracted single-source-of-truth template into
   shared/overseer/issue_template.py with TEMPLATE_LITERAL constant
   and render(**fields) function. Both the dead orchestrator path
   and the sandbox helper import from it; canonical-byte-equality
   test guards drift (R-COMPAT-08).
4. TASK-1-3: compute_anomaly_signature gets a fourth input
   tier1_alert_types: tuple[str, ...] = () (sorted, default empty)
   so two anomalies sharing (anomaly_type, agent_role, repo) but
   triggered by different Tier-1 alerts don't collapse to the same
   signature (HR-06 default).
5. TASK-3-3: OVERSEER_ALERT gets explicit schema_version: int = 2
   field. /sdlc parsing in TASK-6-2 reads the version (defaulting
   to 1 if absent) and falls back gracefully (R-COMPAT-06).
6. TASK-7-8 (NEW): sandbox/tests/test_shared_overseer_imports.py
   smoke-tests every new shared/overseer/ module imports inside
   the sandbox container; runs the imports inside the built image
   when Docker is available (R-COMPAT-09).
7. TASK-1-3: FiledIssueRecord gets hitl_outcome: Literal["filed",
   "skipped", "modified_and_filed"] | None = None field. issue_number
   is now Optional (None when hitl_outcome=="skipped"). Prevents
   re-prompting on the same anomaly after overseer respawn (R-OP-03).
8. TASK-6-2: /sdlc overseer-absent fallback writes a sentinel file
   .egg-state/oversight/sdlc-fallback-fired-{pipeline}-{phase}.flag
   on first emit; pre-emit check skips AskUserQuestion if sentinel
   exists. TASK-7-7 simulates two consecutive ticks and asserts
   AskUserQuestion fires exactly once (R-OP-06).

Total tasks: 24 across 8 phases (was 23). YAML parses cleanly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds documentation for the planned overseer changes in issue #1962:

- pipeline-health-monitoring.md: new "Advisor Gate", "Auto-Issue Filing
  (Shadow vs Live)", "Host Detector Migration", and "MCP Advisor Tool"
  sections; six new PipelineConfig knobs in the configuration table
  (overseer_advisor_model, overseer_auto_file_issues_mode,
  overseer_owns_host_detection, overseer_stuck_phase_transition_seconds,
  overseer_agent_stall_seconds, overseer_silent_agent_threshold_seconds,
  overseer_long_running_phase_seconds, overseer_nack_unresolved_seconds);
  diagnostic body template extended with the Pipeline Links sub-block;
  EGG_PIPELINE_REPO env-var contract documented.
- reference/agent-roles.md: overseer entry updated for the new
  egg-orch overseer file-issue capability, the two new state files
  (.egg-state/oversight/filed-issues.jsonl, agent-timing.json), the
  EGG_PIPELINE_REPO requirement, the gateway constraints on
  gh issue create, and the OVERSEER_ALERT schema_version=2 fields.
- architecture/orchestrator.md: added a host-to-overseer migration
  paragraph covering the calibration-window flag, advisor strategy,
  and the new MCP advisor tool, with cross-links to the guide.

These updates anchor on the planner artifact at
.egg-state/drafts/1962-plan.md (TASK-8-1) and will be tightened against
the coder/tester output as it lands.

Refs #1962
…+ schemas

Lands the foundation work the rest of the #1962 implementation depends
on. No behavior change yet — these are the data models and config
surface that Phases 2/3/4/6 plug into.

TASK-1-1: SDK spike record at .egg-state/agent-outputs/1962-sdk-spike.md.
The vendored claude-agent-sdk==0.1.65 does NOT expose advisor_20260301
or max_uses; ship Option B (two-call advisor pattern) accordingly.

TASK-1-2: PipelineConfig knobs (orchestrator/models.py):
- overseer_advisor_model="claude-opus-4-6" (canonical alias; cost row
  populated in shared/egg_harness/cost.py:18, so the
  max_llm_cost_per_hour self-monitor stays accurate)
- overseer_auto_file_issues_mode: Literal["shadow","live"]="shadow"
- overseer_owns_host_detection: bool=False (calibration-window flag)
- overseer_stuck_phase_transition_seconds=180 (was hard-coded ~60s)
- overseer_agent_stall_seconds=180
- overseer_silent_agent_threshold_seconds=600
- overseer_long_running_phase_seconds=3600
- overseer_nack_unresolved_seconds=180
Per decision-19, no overseer_advisor_max_uses_per_phase knob is added;
the existing max_llm_cost_per_hour=$5 envelope remains the only budget
control until the follow-up advisor-budget issue lands.

TASK-1-3 + TASK-1-5: New shared/egg_overseer/ package:
- priority.py: alert_to_label / label_to_alert (low↔p3, medium↔p2,
  high↔p1; p0 collapses to high on label_to_alert).
- scrubbing.py: scrub_secrets() + find_secret_kinds() — covers
  ghp_/ghs_/gho_/ghu_/ghr_ PATs, AKIA AWS keys, Slack webhooks,
  GITHUB_TOKEN/GH_TOKEN/ANTHROPIC_API_KEY env exports.
- infra_error.py: is_infra_error / classify_infra_error — gh API
  rate-limit, container OOM, network DNS / connection / timeout
  patterns.
- state.py: FiledIssueRecord, AgentTimingEntry, AgentTimingState
  Pydantic models; load/append helpers for filed-issues.jsonl
  (header line on first append, line-position-disambiguated records);
  load/save helpers for agent-timing.json (atomic tempfile+rename
  AND fcntl.LOCK_EX on .lock sentinel per R-PERF-02 / R-SEC-04);
  compute_anomaly_signature(anomaly_type, agent_role, repo,
  tier1_alert_types) → 16-hex SHA-1 prefix.
- advisor.py: AdvisorVerdict Pydantic model (with model_validator that
  enforces issue_title/issue_body/priority required when
  decision==file_issue; alert_summary required when decision==alert)
  and the consult_advisor coroutine (Option B two-call pattern via
  egg_agent.client.run_agent_async; defense-in-depth scrub_secrets on
  issue_body before return).
- shared/pyproject.toml include list extended with "egg_overseer*".

NOTE on package naming: planner referred to this as
shared/overseer/ in the plan document. The actual Python package is
named egg_overseer to match the existing egg_* convention used by
egg_orchestrator, egg_health, etc., because /opt/egg-runtime/shared/
(not its parent) is on PYTHONPATH at runtime — `from shared.overseer`
imports would fail in the sandbox. Imports across the patch use
`from egg_overseer.* import ...`.

TASK-1-4: EGG_PIPELINE_REPO env var:
- orchestrator/kubernetes_spawner.py: derive owner/repo from repos[0]
  (the same source EGG_REPO_PATH uses) and inject into the spawned
  container's environment dict next to EGG_REPO_PATH.
- sandbox/entrypoint.py: fail-fast in setup_environment when the
  overseer role's container starts without EGG_PIPELINE_REPO; writes
  a structured stderr line and raises OSError. No `gh repo view`
  fallback per the plan — silently mis-targeting an issue is worse
  than aborting.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ert schema

TASK-3-1: Single-source-of-truth issue template.
- shared/egg_overseer/issue_template.py: TEMPLATE_LITERAL + render(**fields).
  The literal is the canonical issue-body source extended with a
  "Pipeline Links" sub-block per decision-8 opt-2.
- sandbox/egg_lib/overseer_issue_body.py (NEW):
  - compose_issue_title(anomaly_type, agent_role, anomaly_signature) →
    "[Pipeline Diagnostic] <anomaly> - <role> [<sig8>]" (title embeds
    the 8-char signature so cross-phase gh search succeeds).
  - compose_issue_body(...): renders the canonical template with the
    Pipeline Links sub-block, then runs the result through
    scrub_secrets() as defense-in-depth (advisor is the primary
    scrubber).
  - find_existing_issue(repo, anomaly_signature, ...): reads the local
    .egg-state/oversight/filed-issues.jsonl cache first; falls back to
    `gh issue list --label agent:overseer --state open --search <sig8>
    --json number,title --limit 100` and returns the first issue whose
    title carries the signature prefix.
- orchestrator/overseer/issue_filer.py marked DEAD CODE; literal
  delegated to egg_overseer.issue_template.TEMPLATE_LITERAL via .format()
  so the historical orchestrator path stays operational for the
  byte-equality regression test without a parallel copy. Production
  filing happens sandbox-side via the new CLI verb (decision-9 opt-1).

TASK-3-2: scrub_secrets() body landed in Phase 1 commit 83f282a9d
(shared/egg_overseer/scrubbing.py); this commit only exercises it
through compose_issue_body's defense-in-depth scrub.

TASK-3-3: OVERSEER_ALERT schema extension (no new fields on Message —
extension is carried in the existing free-form metadata dict so legacy
callers see no schema change at the BaseModel level):
- sandbox/egg_agent_tools/handlers/progress.py: progress_overseer_alert
  now accepts optional 'recommendation' (validated against
  {'file_issue'}) and 'recommendation_payload' (dict, ≤50 KB).
  Stores them under metadata.recommendation /
  metadata.recommendation_payload alongside an explicit
  metadata.schema_version=2 marker. Pre-#1962 callers omit both fields
  and the message round-trips with schema_version=2 but no
  recommendation; legacy parsers that ignore unknown metadata keys see
  identical behavior.
- sandbox/egg_lib/orch_cli.py: ov_alert subparser exposes
  --recommendation and --recommendation-payload-file; the latter is
  required when --recommendation is set; the file is JSON-parsed and
  forwarded to the handler.

The advisor-side composer (TASK-4-1) emits the OVERSEER_ALERT with
recommendation=file_issue + the fully-composed payload; /sdlc renders
the alert and (in shadow mode, the default) raises a HITL decision
the human resolves. The CLI verb in TASK-2-1 reads that approval and
calls gh.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…-issue CLI

TASK-2-1: New `egg-orch overseer file-issue` subcommand in
sandbox/egg_lib/orch_cli.py:
- Required flags: --anomaly-type, --priority {p0,p1,p2,p3},
  --agent-role, --anomaly-signature (16-hex), --issue-title-file,
  --issue-body-file. Optional: --parent-alert-message-id, --dry-run.
- Reads title + body from local files (no shell-escape headaches);
  enforces title ≤120 chars and body ≤50 KB locally so the gateway
  doesn't have to fail at the network boundary.
- Pre-call dedup: find_existing_issue(repo, anomaly_signature) reads
  the local .egg-state/oversight/filed-issues.jsonl cache first;
  falls back to `gh issue list --label agent:overseer --state open
  --search <sig8> --json number,title --limit 100` and short-circuits
  on a hit (returns {filed: false, dedup_match: <number>}).
- Live path: subprocess.run(["gh", "issue", "create", "--repo",
  $EGG_PIPELINE_REPO, "--title-file", ..., "--body-file", ...,
  "--label", "agent:overseer", "--label", priority, "--json",
  "url,number,title"]). Parses json.loads(stdout)["number"] (NOT a
  regex over the URL suffix per risk_analyst R-COMPAT-10).
- On success appends a FiledIssueRecord to filed-issues.jsonl and
  emits a structured `overseer_event` log line with
  outcome=filed|dedup, issue_number, anomaly_signature for the
  metrics instrumentation in TASK-7-5.

TASK-2-2: Gateway guardrails in gateway/agent_restrictions.py.
Importantly preserves the verified baseline ("issue create *" is NOT
on _OVERSEER_BLOCKED_GH_OPS, so gh issue create from the overseer is
already permitted by the role-level rule). The new check adds
*additional* guardrails on top:
- check_overseer_gh_issue_create(role, repo, pipeline_repo, labels,
  title, body) → OverseerGhCheckResult.
- (a) repo enforcement: --repo MUST equal EGG_PIPELINE_REPO when set;
  cross-repo filing rejected.
- (b) label injection: agent:overseer + a p0..p3 priority label
  auto-added when caller forgot. Defense-in-depth against accidental
  bypass.
- (c) size limits: title ≤120 chars, body ≤50 KB.
- (d) defense-in-depth secret scan via egg_overseer.scrubbing.
  find_secret_kinds; rejects with structured error citing the
  matched pattern kinds (gh-pat / aws-key / slack-webhook / env-export).
- No rate limit (per feedback-1.Q4 — dedup + shadow-mode rollout are
  the rate-limiting controls).

The actual wiring of check_overseer_gh_issue_create into the gateway's
gh-passthrough handler (so it intercepts the live request) is left as
a follow-up because the gateway's request-handling shape is unrelated
to the policy module — exposing the check function here lets the
existing handler call it without restructuring. This matches the
`add a gateway allowlist rule now, defer PATH restructuring` decision
in decision-14.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…status

TASK-4-2: Orchestrator-side MCP tool surface for the advisor.
- orchestrator/mcp/tools/overseer_advisor.py (NEW): CONSULT_ADVISOR_TOOL
  schema dict + handle_consult_advisor async handler. Auth-gates to the
  overseer role (other roles get a permission error). Forwards to
  egg_overseer.advisor.consult_advisor with PipelineConfig threading.
- The full FastMCP wiring (adding the tool to PIPELINE_TOOLS in
  orchestrator/mcp_tools.py and the dispatch table in
  PipelineToolHandler.handle_tool_call) is left as a small follow-up
  edit so the schema + handler can land here without restructuring
  the existing tool registration loop. The overseer rule doc (Phase 5,
  documenter task) references the eventual MCP tool name
  `mcp__overseer__consult_advisor` so callers don't have to learn a
  new name when the wiring lands.

TASK-1-2 (status endpoint extension): orchestrator/routes/pipelines.py
get_pipeline_status now embeds the eight new overseer config knobs
under data.config so the sandbox-side overseer_monitor can read
PipelineConfig values via the existing status endpoint (no new
endpoint required).

TASK-6-1: Migrated detectors land in sandbox/overseer_monitor.py.
- run_migrated_detectors(...) reads .egg-state/oversight/agent-timing.json
  via egg_overseer.state.load_agent_timing (flock-protected), runs the
  four detectors, persists alerted_anomalies bookkeeping back, and
  returns alert dicts the agent should consider emitting via
  `egg-orch overseer alert`.
- detect_agent_stall — phase_entered_at older than
  overseer_agent_stall_seconds. Priority=medium. Recommends
  `egg-checkpoint show` for diagnostics.
- detect_agent_silent — first_seen_at older than
  overseer_silent_agent_threshold_seconds AND no progress events.
  Priority=medium.
- detect_nack_unresolved — outstanding NACK older than
  overseer_nack_unresolved_seconds. Priority=high.
- detect_phase_long_running — implement phase elapsed beyond
  overseer_long_running_phase_seconds. Priority=medium.
- All detectors honour overseer_owns_host_detection (calibration-window
  flag, default False). When False, run_migrated_detectors returns []
  immediately — /sdlc keeps owning these detectors during the
  calibration window. When True, the overseer is the sole source.
- Per-anomaly suppression: each detector skips emitting if the same
  (role, anomaly) fired within `2 * threshold` seconds; the
  alerted_anomalies map on AgentTimingEntry is the bookkeeping store.
- run_once now appends `detector_alerts` (list of alert dicts) and
  `advisor_gate` (Tier-1 alert presence + sorted alert types) to the
  cycle JSON line. The overseer agent reads `advisor_gate` to decide
  whether the Haiku-flag × Tier-1-intersection gate has tripped and
  the advisor MCP tool should be invoked.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… quotes

Trivial cleanup on top of the four #1962 phase commits:

- sandbox/egg_lib/orch_cli.py: drop the unused `body` local in
  cmd_overseer_file_issue — the body is passed to gh via --body-file
  so the decoded text was never read; the byte-length cap is what
  matters and is already enforced.
- sandbox/overseer_monitor.py: drop unused AgentTimingState import in
  run_migrated_detectors (never instantiated; AgentTimingEntry is the
  only type referenced).
- shared/egg_overseer/advisor.py: drop forward-reference quotes on
  AdvisorVerdict and PipelineConfig — the module already enables
  `from __future__ import annotations` so all type annotations are
  strings by default.

No behavior change. Ruff now passes clean on all #1962 files.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eway, schema, calibration

Address 4 of 5 blocking issues raised by reviewer_contract on the
initial proposal. Blocker 3 (package naming `shared/egg_overseer/` vs
`shared/overseer/`) surfaced as decision-26 HITL for human resolution.

Blocker 1 (TASK-4-2 — MCP advisor tool not registered): wire the
`consult_advisor` tool into the FastMCP server.
- orchestrator/mcp_tools.py: add the tool entry to PIPELINE_TOOLS
  (auth-gated to overseer role at the handler level) and the
  `consult_advisor → _handle_consult_advisor` dispatch in
  PipelineToolHandler.handle_tool_call. Handler runs the async
  consult_advisor coroutine in a fresh event loop because the
  FastMCP wrapper invokes handle_tool_call from a thread-pool worker.
- sandbox/overseer_monitor.py: add `maybe_consult_advisor(classification,
  cycle_report)` that encodes the Tier-1 intersection gate
  (classification.confidence ≥ 0.8 AND tier1_alerts_present) and
  forwards to the MCP tool when both conditions trip. The advisor_gate
  field in the cycle report now also carries `gate_open` so the
  overseer agent reads it directly.

Blocker 2 (TASK-2-2 — gateway guardrail not wired into live request
path): invoke check_overseer_gh_issue_create from the existing
gh-passthrough handler.
- gateway/gateway.py: after the role-level check_agent_gh_operation
  pass, when session_role=="overseer" AND args[0:2]==["issue","create"],
  parse --repo / --label / --title{,-file} / --body{,-file} from the
  argv and call check_overseer_gh_issue_create. Failure → 403 with
  structured error. Auto-injects required labels (agent:overseer +
  p2 default) when caller forgot, with audit log entry.

Blocker 4 (TASK-6-1 — detector logic inverted vs side-by-side
calibration): the original implementation returned [] when the flag
was False, leaving no overlap. Fix:
- run_migrated_detectors now runs UNCONDITIONALLY (both calibration
  and live modes). Each emitted alert dict carries
  `calibration_only: True` when overseer_owns_host_detection=False
  (the calibration default — observational, /sdlc keeps the
  authoritative detectors firing) and `False` when True
  (authoritative, host detectors expected silent). This satisfies
  the plan's "side-by-side calibration with the overseer's new
  detectors" intent and the feedback-1.Q6 success-criterion that
  needs comparable signal from both sides during the calibration
  release.

Blocker 5 (TASK-3-3 — schema-version field on metadata, not OVERSEER_ALERT
model): add first-class fields on the message envelope.
- orchestrator/message_store.py: Message gains optional
  `recommendation: str | None = None`,
  `recommendation_payload: dict | None = None`,
  `schema_version: int = 1` fields. to_dict() emits them only when
  populated / non-default so legacy callers serialize byte-identically
  to today (the regression guarantee TASK-7-1 requires).
- sandbox/egg_agent_tools/handlers/progress.py: progress_overseer_alert
  now writes the new fields to the top-level message data dict (not
  to metadata). schema_version=2 is set whenever recommendation or
  recommendation_payload is populated; otherwise the message
  defaults to schema_version=1 (pre-#1962 implicit).

PR commit message identifies the schema-file path
(orchestrator/message_store.py) as TASK-3-3 acceptance requires.

Tests: 232 orchestrator tests pass (test_models, test_message_store,
test_overseer_monitor, test_overseer_alert_isolation,
test_overseer_issue_filer); ruff clean on all touched files.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… schema)

Refreshes the TASK-8-1 docs against coder commit 9bdaa64 to reflect
two real-implementation deltas from the plan:

1. Package name. The planner referred to the new package as
   shared/overseer/. The actual Python package is egg_overseer
   (located at shared/egg_overseer/) because the runtime PYTHONPATH
   is /opt/egg-runtime/shared, not its parent — `from shared.overseer.*`
   would fail in the sandbox. All import paths in the docs now use
   `egg_overseer.*` (e.g. `egg_overseer.advisor.consult_advisor`,
   `egg_overseer.state.compute_anomaly_signature`,
   `egg_overseer.priority.label_to_alert`).

2. OVERSEER_ALERT schema versioning is metadata-carried, not
   message-envelope. `recommendation` and `recommendation_payload`
   ride on the existing `metadata: dict` field alongside an explicit
   `metadata.schema_version=2` discriminator. Pre-#1962 alerts
   implicitly carry schema_version=1 (consumers default to 1 when
   the key is absent). The CLI uses
   `--recommendation file_issue --recommendation-payload-file ...`;
   sandbox handler enforces a 50 KB payload cap. Backwards-compat
   note clarifies the message envelope itself is unchanged.

Also adds the MCP tool registration path (`orchestrator/mcp/tools/
overseer_advisor.py` — `CONSULT_ADVISOR_TOOL` schema +
`handle_consult_advisor` handler) to the Advisor Gate and
agent-roles overseer entry, plus the helper function names
(`load_agent_timing` / `save_agent_timing` / `load_filed_issues` /
`append_filed_issue`) on the agent-roles state-file note.

Refs #1962
…ad-code anchor, packaging fail-loud, gateway flag-parsing, JSONL flock, label hygiene

Address all addressable blockers in reviewer_code's v2 NACK on commit
b8a11d2. Documenter-scope items (TASK-5-1, TASK-6-2) remain out of
scope for the coder role.

Blocker 1 (consult_advisor SDK kwargs): renamed `system=` to
`system_prompt=` (the actual SDK signature at
shared/egg_agent/client.py:65) and switched `str(result)` to
`result.stdout` so the assistant's text body lands in the JSON
parser instead of the AgentResult repr. Code-fence-stripping
(```json … ```) hoisted out of the default runner so it covers
caller-supplied test runners too.

Blocker 2 (issue_filer.py canonical literal anchor): re-added
LEGACY_BODY_LITERAL byte-for-byte preserved as the historical
canonical literal that the planned TASK-7-1 byte-equality test
asserts against. Live rendering still uses TEMPLATE_LITERAL.format()
from egg_overseer.issue_template; the constant is the historical
anchor that ensures drift is caught.

Blocker 3 (silent ImportError in run_migrated_detectors):
production now fails loud with a structured
`_overseer_error: egg_overseer_packaging_missing` stderr line and
re-raises so the cycle visibly fails. Only swallowed when
EGG_OVERSEER_TEST_MODE=1 (lightweight unit tests that mock the
cycle).

Blocker 4 (detect_phase_long_running min over all entries):
filter `state.entries.values()` to entries whose `entry.phase ==
phase_name` and skip synthetic `_` keys before taking min, so an
entry left over from a prior phase doesn't make the current phase
appear "long-running" within milliseconds of starting.

Blocker 5 (filed-issues.jsonl writes without flock):
egg_overseer.state.append_filed_issue now acquires `_file_lock`
on the same `agent-timing.lock` sentinel used by
save_agent_timing, so concurrent overseer respawns don't
interleave records (POSIX only guarantees atomic writes ≤
PIPE_BUF; FiledIssueRecord JSON can exceed that).

Blocker 6 (silent UTF-8 corruption on body decode):
gateway/gateway.py reads --body-file and --title-file with
`errors="strict"` and surfaces a 400 with the UnicodeDecodeError
detail when invalid bytes are present, so a leaked-secret byte
sequence cannot be silently swapped past the regex check.

Blocker 9 (spoofable role in MCP tool schema): dropped `role`
from CONSULT_ADVISOR_TOOL inputSchema. The handler now sources
the calling role from the orchestrator-side EGG_AGENT_ROLE env
var (defaulting to "overseer" because the MCP server runs inside
the orchestrator pod). Stricter session-aware auth is deferred
to #1786 per decision-14.

Blocker 10 (maybe_consult_advisor POSTed to non-existent REST
endpoint): replaced with `should_consult_advisor`, a pure
predicate the agent reads to decide whether to invoke the MCP
tool through its own MCP client surface. The orchestrator MCP
server is exposed only over FastMCP streamable-HTTP (`/mcp`); no
REST route exists, so the prior helper would have 404'd on
first use.

Blocker 11 (asyncio.new_event_loop+close leaks SDK background
tasks): `_handle_consult_advisor` now uses `asyncio.run(...)`
which atomically creates+closes a fresh loop AND calls
shutdown_asyncgens so the SDK's background tasks tear down
cleanly between calls.

Blocker 12 (gateway flag-parsing loop is order-dependent and
silently drops unknowns): rewrote the parser to require known
value-flags consume a value that does NOT start with `-`,
returning a structured 400 when the value is missing or is
another flag. Eliminates the `--repo --label foo` bypass class.

Blocker 13 (auto-injection allows agent:fake alongside
agent:overseer): reject any `agent:*` label that isn't
`agent:overseer` before considering injection. Caller cannot
sneak a contradicting label in.

232 orchestrator tests still pass; advisor + agent_restrictions
exercised via in-line smoke tests; ruff clean on all touched files.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…NACK fixes

Addresses reviewer_code NACK on v2 (commit 72ef4f2):

Blocking item 1: TASK-5-1 — sandbox/agent-config/rules/overseer.md
- Lifted the issue-filing prohibition (former line 206).
- Added "Tier-2 advisor gate" section under Two-Tier Architecture
  describing the `Haiku-confidence ≥ 0.8 AND Tier-1 alert present`
  intersection gate, the orchestrator MCP tool
  `mcp__overseer__consult_advisor`, and the three advisor outcomes
  (alert / file_issue / watch).
- Added "Auto-issue filing protocol" section with the three control
  pillars stated verbatim per the plan's grep-based acceptance bar:
  Dedup before recommend (×2), Flag-vs-HITL gating (×2), Secret
  scrubbing (×2), labels & title format, CLI invocation.
- Updated escalation triggers list with the new migrated detectors
  (agent-stall, agent-silent, agent-nack-unresolved,
  phase-long-running) and the bumped stuck-phase-transition default
  (60s → 180s).
- Added "Files the overseer reads/writes" table documenting
  filed-issues.jsonl, agent-timing.json, and agent-timing.lock.
- CLI Commands Reference: added `egg-orch overseer file-issue` row
  and the new `--recommendation` / `--recommendation-payload-file`
  flags on `overseer alert`. Added "Allowed actions (additions)"
  section. Updated the deprecated prohibition note.
- Verified greps: "egg-orch overseer file-issue" (8), "Dedup before
  recommend" (2), "Flag-vs-HITL gating" (2), "Secret scrubbing" (2),
  "180" + "stuck-phase-transition" (1).

Blocking item 2: TASK-6-2 — skills/sdlc/SKILL.md
- Added a "Host detector migration (issue #1962)" section before
  Consensus Monitoring documenting the gating semantics
  (`overseer_owns_host_detection` False = host runs detectors;
  True = overseer is sole source) and the host's "Overseer-Absent
  Fallback" AskUserQuestion under the
  `.egg-state/oversight/sdlc-fallback-fired-{pipeline_id}-{phase}.flag`
  sentinel for at-most-once-per-phase behavior.
- Added "Skip when overseer_owns_host_detection=True" guards on
  five detection blocks (Stall detection, Silent agent detection,
  NACK escalation, Long-Running Phase Detection, Stuck Pipeline
  Rescue) plus the duplicated Stall detection in Phase S5 — Monitor
  short-flow.
- Updated State tracking paragraph to describe the
  `.egg-state/oversight/agent-timing.json` migration and
  flock-guarded read/modify/write under the True path.

Blocking item 3: side-by-side calibration claim was wrong
- Rewrote the paragraph in pipeline-health-monitoring.md to
  describe the actual flag semantics (host XOR overseer, not
  host AND overseer) since `run_migrated_detectors` early-returns
  when the flag is False; noted the parallel-run idea as a
  follow-up enhancement out of scope here.
- Mirrored the same correction in architecture/orchestrator.md
  (same migration paragraph) and split the long single-sentence
  paragraph at the "Concurrently, the overseer's decision tier"
  pivot per the reviewer's non-blocking note.

Non-blocking polish addressed:
- agent-roles.md: tightened the gateway constraints paragraph to
  cite check_overseer_gh_issue_create explicitly and noted the
  wiring is part of the same PR (verify on merged commit).
- agent-roles.md: added FiledIssueRecord / load_filed_issues /
  append_filed_issue cross-link next to the filed-issues.jsonl
  description for symmetry with AgentTimingState.
- guides/sdlc-pipeline.md: corrected stale `overseer-alert` label
  reference to `agent:overseer` + matching priority label, citing
  issue #1962.

Refs #1962

@egg-reviewer egg-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.

Re-review at 9efc2a9 — review-nit fixes since 6c081ab

Delta is a single commit, three small nit fixes (49 changed files in the PR overall, but only 3 files in this delta: sandbox/egg_lib/orch_cli.py, sandbox/overseer_monitor.py, sandbox/tests/test_egg_orch_overseer_consult_advisor.py).

Verification of the three fixes

  1. Stale comment block at sandbox/overseer_monitor.py:517-527. ✓ Updated. The new comment correctly references egg-orch overseer consult-advisor (the actual CLI verb that exists at sandbox/egg_lib/orch_cli.py:1747) and the actual helper should_consult_advisor (defined at sandbox/overseer_monitor.py:562, a pure predicate as the comment claims — confidence >= HAIKU_CONFIDENCE_THRESHOLD AND tier1, no side effects). The phantom maybe_consult_advisor reference is gone, and the EGG200 boundary citation is correct.

  2. Exit-code 3 for advisor SDK runtime failures at sandbox/egg_lib/orch_cli.py:1825-1835. ✓ Implementation is correct:

    • AdvisorParseError (which inherits from RuntimeError, verified at shared/egg_overseer/advisor.py:102) is caught first → exit 1.
    • The new except Exception arm comes second → exit 3, with type(exc).__name__ and the message echoed to stderr so the caller can distinguish e.g. APIConnectionError from RateLimitError from AuthenticationError without parsing the stderr text.
    • BaseException derivatives (KeyboardInterrupt, SystemExit, asyncio.CancelledError) are correctly NOT swallowed.
    • Docstring exit-code table updated and accurate.
    • test_advisor_runtime_error_returns_3 covers the new arm with a synthetic RuntimeError("connection reset by peer"); verifies exit code 3, the "advisor runtime failure" substring, the type name, and the original message all reach stderr.
    • All 17 tests in sandbox/tests/test_egg_orch_overseer_consult_advisor.py pass locally.
  3. --json flag help text at sandbox/egg_lib/orch_cli.py:3103-3110. ✓ The help string now spells out the asymmetry: --json tees JSON to stdout when --output-file is set, and is a no-op without --output-file because stdout is already JSON. Inline comment at 1852-1855 mirrors the same explanation. No behavior change — this is documentation only — which matches the author's explicit decision in their feedback-addressing comment to keep the flag rather than drop it.

No new issues introduced

  • The bare except Exception is intentionally broad (the docstring explicitly enumerates "or any other unhandled exception from the SDK call"). It will also surface programming bugs (e.g. TypeError from a future consult_advisor signature change) as exit 3 with a diagnostic message — that's a benign outcome since the caller treats exit 3 as "transient/runtime" and stderr names the actual exception type.
  • No new pre-existing-issue exposure: the comment update doesn't add new code paths; the exit-code arm only widens what was an unhandled-traceback path; the help-text change doesn't touch behaviour.

Carry-forward non-blocking items (still deferred, none regressed)

The author's feedback-addressing comment explicitly defers these to follow-ups, and none are touched by this delta:

  • overseer_advisor_model knob unused (CLI passes config=None so the knob defaults to "opus").
  • shared/egg_overseer/state.py:122-126 lock docstring mismatch (claims shared lock with agent-timing.lock, actually uses filed-issues.jsonl.lock).
  • shared/egg_overseer/scrubbing.py missing fine-grained GitHub PAT (github_pat_<22>_<59>), sk-ant-api03-…, GCP service-account JSON markers — relevant before flipping overseer_auto_file_issues_mode=live.
  • shared/egg_overseer/advisor.py::_default_runner JSON extraction is fence-only; bare-object-with-prose fall-through to AdvisorParseError.
  • orchestrator/overseer/issue_filer.py DEAD CODE half-deprecation with byte-equality test.
  • sandbox/overseer_monitor.py:334 pipeline_data.get("config", {}) silent default on InvalidPipelineIdError.
  • sandbox/agent-config/rules/overseer.md Forbidden Actions table whiplash.
  • recent_log_lines byte cap on the prompt-builder side.
  • End-to-end test coverage of the subprocess.run / consult_advisor wrapper-vs-CLI handshake without mocks.

These remain appropriate follow-ups, not blockers for this PR.

Verdict

Approve. The three nit fixes are correct, scoped, and well-tested. No new issues introduced; no regressions of previously approved work. The deferred carry-forward items are correctly tracked rather than silently dropped.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor Author
egg is addressing review feedback...

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

No code changes required.

The two most recent reviews on 9efc2a9 are both approvals:

  • egg-reviewer[bot] review 4176390282: APPROVED — verifies the three nit fixes (stale comment block at sandbox/overseer_monitor.py:517-527, new exit code 3 for advisor SDK runtime failures at sandbox/egg_lib/orch_cli.py:1825-1835, and --json flag help-text clarification at sandbox/egg_lib/orch_cli.py:3103-3110). Verdict: "Approve. The three nit fixes are correct, scoped, and well-tested. No new issues introduced; no regressions of previously approved work."
  • james-in-a-box[bot] contract-verification review 4176389763: verdict=approve. "No new tasks introduced, no acceptance criteria broken, regression test added for the new exit-code path."

The nine carry-forward items the reviewer enumerated (overseer_advisor_model knob unused; state.py:122-126 lock docstring mismatch; scrubbing.py missing fine-grained-PAT/sk-ant-api03-…/GCP-SA patterns; _default_runner fence-only JSON extraction; issue_filer.py DEAD-CODE half-deprecation; overseer_monitor.py:334 silent default; overseer.md Forbidden Actions whiplash; recent_log_lines byte cap; end-to-end subprocess.runconsult_advisor test) are explicitly tagged: "These remain appropriate follow-ups, not blockers for this PR."

No line-level comments outstanding. All CI checks SUCCESS. Holding.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

24 previous review(s) hidden.

@jwbron
jwbron merged commit 4d6aca3 into main Apr 26, 2026
39 of 40 checks passed
jwbron added a commit that referenced this pull request Apr 27, 2026
#2159)

* Fix #2120: enforce recent_log_lines byte cap on advisor prompt-builder

Carry-forward from the #2096 re-review. The advisor prompt-builder
joined `recent_log_lines` raw, leaving three failure modes open: a
single pathological log line could push the prompt past the model's
context window (AdvisorParseError or hard SDK error); a burst of merely
long lines could silently spend advisor budget far above per-call
expectations; and a longer input widens the leak surface for secrets
that survive scrubbing.

This adds a configurable byte cap (default 256 KiB, sized for the opus
context window with comfortable headroom). When the joined block
exceeds the cap, oldest lines are dropped first so the most-recent
lines (highest signal) survive, and a marker is prepended so the
advisor knows truncation happened. A structured `advisor_log_truncated`
log event fires with `dropped_lines`, `dropped_bytes`, `cap_bytes`, and
`input_line_count` so pathological producers stay observable.

The cap is plumbed end to end: `PipelineConfig.overseer_advisor_recent_log_bytes_cap`
for ops tuning, surfaced on the status endpoint config payload, and
passed through `egg-orch overseer consult-advisor --recent-log-bytes-cap`
into `consult_advisor`. Resolution order in the advisor is explicit
arg → config field → module default. `0` disables the cap.

* Address PR #2159 review feedback

- Use None fallback in status endpoint for overseer_advisor_recent_log_bytes_cap
  to match adjacent overseer_advisor_model pattern (avoids duplicating the
  256_000 default literal across two sites).
- Reject negative values in --recent-log-bytes-cap CLI flag via
  _non_negative_int validator, matching PipelineConfig ge=0.
- Add status-endpoint test asserting overseer_advisor_recent_log_bytes_cap
  is exposed under data.config.
- Add CLI parser tests covering negative-rejection and zero-accepted.

---------

Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
jwbron added a commit that referenced this pull request Apr 27, 2026
) (#2158)

* overseer: wire overseer_advisor_model through consult-advisor CLI (#2113)

The `egg-orch overseer consult-advisor` verb hardcoded `config=None`
when calling `consult_advisor`, so `PipelineConfig.overseer_advisor_model`
silently defaulted to the `"opus"` alias regardless of how the pipeline
was configured. This carry-forward from #2096 plumbs the configured
value through the CLI:

- Adds an optional positional `pipeline_id` to `consult-advisor`
  (auto-resolved from `EGG_PIPELINE_ID`, matching every other overseer
  verb).
- When provided, reads `data.config.overseer_advisor_model` from the
  orchestrator status endpoint (already exposed by #2096 in
  `orchestrator/routes/pipelines.py`) and passes a duck-typed config
  to `consult_advisor`.
- Status fetch failures fall back to `config=None` with a warning,
  preserving the historic default for unreachable orchestrators.

Adds regression tests covering: positional arg, env-var resolution,
status-fetch failure fallback, and parser shape. Also drops a per-test
`monkeypatch.delenv("EGG_PIPELINE_ID")` autouse fixture so ambient env
state cannot mask the new wiring.

* overseer: harden consult-advisor pipeline-id lookup against bad input

Addresses non-blocking findings from the egg-reviewer pass on PR #2158:

- Pre-check pipeline_id with _SAFE_ID_PATTERN.match before calling the
  orchestrator client so a malformed EGG_PIPELINE_ID (stray space, slash,
  etc.) no longer escapes validate_id's sys.exit(1) — which would have
  collided with AdvisorParseError's exit-code semantics. Malformed ids
  emit a warning and fall back to the historic "opus" default.
- Catch ImportError alongside OrchestratorError for the lazy
  egg_lib.orch_client import so an environment that cannot resolve the
  client module degrades to the default rather than crashing.
- Add a regression test exercising the malformed-id branch and asserting
  OrchClient is never invoked.
- Drop the redundant positional from the overseer.md example (the
  overseer always has EGG_PIPELINE_ID set; the verb auto-resolves) and
  rephrase the prose to match.
- Add a forward-looking comment near the SimpleNamespace shim noting
  that any future config.* reads in consult_advisor must be added here.

* overseer: fix consult-advisor ImportError fallback (NameError on bind)

The combined except (OrchestratorError, ImportError) clause from
026502c crashes with NameError when the from-import itself fails:
Python evaluates the tuple, can't resolve OrchestratorError (which
was never bound because the import raised), and the handler never
runs.

Switch to a nested try so the ImportError is handled before
OrchestratorError is referenced. Adds a regression test that
patches builtins.__import__ to make egg_lib.orch_client unimportable
and asserts rc == 0 + the warning text — without this, future
re-arrangement of the lazy import would silently re-break the
branch (which is the bug that just slipped through).

Review feedback on PR #2158.

* Fix checks: apply automated formatting fixes

---------

Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: egg <egg@localhost>
james-in-a-box Bot pushed a commit that referenced this pull request Apr 28, 2026
#2159)

* Fix #2120: enforce recent_log_lines byte cap on advisor prompt-builder

Carry-forward from the #2096 re-review. The advisor prompt-builder
joined `recent_log_lines` raw, leaving three failure modes open: a
single pathological log line could push the prompt past the model's
context window (AdvisorParseError or hard SDK error); a burst of merely
long lines could silently spend advisor budget far above per-call
expectations; and a longer input widens the leak surface for secrets
that survive scrubbing.

This adds a configurable byte cap (default 256 KiB, sized for the opus
context window with comfortable headroom). When the joined block
exceeds the cap, oldest lines are dropped first so the most-recent
lines (highest signal) survive, and a marker is prepended so the
advisor knows truncation happened. A structured `advisor_log_truncated`
log event fires with `dropped_lines`, `dropped_bytes`, `cap_bytes`, and
`input_line_count` so pathological producers stay observable.

The cap is plumbed end to end: `PipelineConfig.overseer_advisor_recent_log_bytes_cap`
for ops tuning, surfaced on the status endpoint config payload, and
passed through `egg-orch overseer consult-advisor --recent-log-bytes-cap`
into `consult_advisor`. Resolution order in the advisor is explicit
arg → config field → module default. `0` disables the cap.

* Address PR #2159 review feedback

- Use None fallback in status endpoint for overseer_advisor_recent_log_bytes_cap
  to match adjacent overseer_advisor_model pattern (avoids duplicating the
  256_000 default literal across two sites).
- Reject negative values in --recent-log-bytes-cap CLI flag via
  _non_negative_int validator, matching PipelineConfig ge=0.
- Add status-endpoint test asserting overseer_advisor_recent_log_bytes_cap
  is exposed under data.config.
- Add CLI parser tests covering negative-rejection and zero-accepted.

---------

Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
james-in-a-box Bot pushed a commit that referenced this pull request Apr 28, 2026
) (#2158)

* overseer: wire overseer_advisor_model through consult-advisor CLI (#2113)

The `egg-orch overseer consult-advisor` verb hardcoded `config=None`
when calling `consult_advisor`, so `PipelineConfig.overseer_advisor_model`
silently defaulted to the `"opus"` alias regardless of how the pipeline
was configured. This carry-forward from #2096 plumbs the configured
value through the CLI:

- Adds an optional positional `pipeline_id` to `consult-advisor`
  (auto-resolved from `EGG_PIPELINE_ID`, matching every other overseer
  verb).
- When provided, reads `data.config.overseer_advisor_model` from the
  orchestrator status endpoint (already exposed by #2096 in
  `orchestrator/routes/pipelines.py`) and passes a duck-typed config
  to `consult_advisor`.
- Status fetch failures fall back to `config=None` with a warning,
  preserving the historic default for unreachable orchestrators.

Adds regression tests covering: positional arg, env-var resolution,
status-fetch failure fallback, and parser shape. Also drops a per-test
`monkeypatch.delenv("EGG_PIPELINE_ID")` autouse fixture so ambient env
state cannot mask the new wiring.

* overseer: harden consult-advisor pipeline-id lookup against bad input

Addresses non-blocking findings from the egg-reviewer pass on PR #2158:

- Pre-check pipeline_id with _SAFE_ID_PATTERN.match before calling the
  orchestrator client so a malformed EGG_PIPELINE_ID (stray space, slash,
  etc.) no longer escapes validate_id's sys.exit(1) — which would have
  collided with AdvisorParseError's exit-code semantics. Malformed ids
  emit a warning and fall back to the historic "opus" default.
- Catch ImportError alongside OrchestratorError for the lazy
  egg_lib.orch_client import so an environment that cannot resolve the
  client module degrades to the default rather than crashing.
- Add a regression test exercising the malformed-id branch and asserting
  OrchClient is never invoked.
- Drop the redundant positional from the overseer.md example (the
  overseer always has EGG_PIPELINE_ID set; the verb auto-resolves) and
  rephrase the prose to match.
- Add a forward-looking comment near the SimpleNamespace shim noting
  that any future config.* reads in consult_advisor must be added here.

* overseer: fix consult-advisor ImportError fallback (NameError on bind)

The combined except (OrchestratorError, ImportError) clause from
026502c crashes with NameError when the from-import itself fails:
Python evaluates the tuple, can't resolve OrchestratorError (which
was never bound because the import raised), and the handler never
runs.

Switch to a nested try so the ImportError is handled before
OrchestratorError is referenced. Adds a regression test that
patches builtins.__import__ to make egg_lib.orch_client unimportable
and asserts rc == 0 + the warning text — without this, future
re-arrangement of the lazy import would silently re-break the
branch (which is the bug that just slipped through).

Review feedback on PR #2158.

* Fix checks: apply automated formatting fixes

---------

Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: egg <egg@localhost>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve overseer escalation/issue opening behavior

1 participant