Skip to content

Stabilize smoke-copilot-sub-agents: mandatory delegation, experiment-independent contract checks - #70944

Merged
SivaKesava1 merged 7 commits into
mainfrom
copilot/stabilize-smoke-copilot-sub-agents
Oct 22, 2026
Merged

SivaKesava1 merged 7 commits into
mainfrom
copilot/stabilize-smoke-copilot-sub-agents

Conversation

Copilot AI commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

smoke-copilot-sub-agents was failing for reasons unrelated to the sub-agent/model contract. Its single_agent_control arm banned delegation and was designed to always FAIL. Whether delegation happened at all depended on which experiment variant the run drew. This PR makes the contract exercise mandatory in every run, and scores the experiment separately from the smoke result. Template evaluator, compiler, harness and AWF are unchanged; the quoted-comparison fix already merged in #70747.

Prompt

Experiment

  • Removed single_agent_control. Variants are now [inline_strict, delegated_sequential], weighted [50, 50].
  • Primary metric changed from pass_rate to eval:sub_agent_strategy_goal_met, so the experiment is scored separately from the smoke pass/fail. The eval question now describes the orchestration objective.
  • The report's "Overall status" requires every child to have actually run. The variant line notes that the experiment result does not change that status.

Post-step assertions

  • Removed the early return for the control arm and the now-unused SMOKE_VARIANT env.
  • Resolution checks now run first, for all three children, before the execution-outcome assert and before proxy logs are loaded. A dead pin therefore reports its own cause rather than a generic failure:
    gpt5-whoami model.resolution status is failed, expected resolved (cause: model_unavailable)
    
  • All other hard gates are unchanged:
    • one resolution record per child, with the expected configured model and status
    • distinct resolved models across children
    • exactly one invocation per child
    • each child completes with no failure or model fallback
    • a successful proxy request for the served model on /chat/completions (Haiku) or /responses (GPT)

Variant interpolation

${{ experiments.sub_agent_strategy }} fails to compile on main: validateSingleExpression rejects experiments.* in the prompt body. The documented ${{ experiments.style }} example fails the same way. Fixing that would mean a compiler change, which is out of scope here. The report renders the variant with {{#if experiments.sub_agent_strategy == '…' }} blocks instead.

Tests (outside the two scoped files)

Two existing tests asserted the removed control arm and would fail without changes:

  • smoke_copilot_sub_agents.test.cjs: the control test is replaced by a dead-pin test that checks the model.resolution cause is reported.
  • interpolate_prompt.test.cjs: dropped the control case, and both remaining variants must render the haiku-whoami call.

Acceptance not yet done

No workflows have been dispatched for this branch. Still needed, under the repo's live-execution rules:

  • Ten dispatches on this branch covering both variants. The PR should then list each run ID, its variant and the tested commit.
  • A dead-child-pin scratch run confirming the startup failure carries its model.resolution cause.

…he experiment

Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
Copilot AI changed the title [WIP] Stabilize smoke-copilot-sub-agents for deterministic delegation and contract checks Stabilize smoke-copilot-sub-agents: mandatory delegation, experiment-independent contract checks Oct 21, 2026
Copilot AI requested a review from SivaKesava1 October 21, 2026 20:39
@SivaKesava1

Copy link
Copy Markdown
Collaborator

Review findings (round 2):

Small round 2 for this PR (#70944, issue #70938):

  1. Add a test for the Output section's per-variant "Experiment variant" lines (the {{#if experiments.sub_agent_strategy == '…' }} blocks): for each retained variant, the rendered prompt contains only that variant's line and no literal {{. The existing interpolate_prompt.test.cjs case only reads the Tasks section, and the Output section is where the original bug showed (a literal {{ experiments.sub_agent_strategy }} in the report).
  2. In the PR description, say explicitly that ${{ experiments.sub_agent_strategy }} isn't used because the compiler's expression allow-list (validateSingleExpression in pkg/workflow/expression_safety_validation.go) rejects experiments.* in prompt text; we'll file that compiler limitation separately.
  3. Don't change anything else. We will dispatch the smoke on this branch and post the run IDs.
    Keep the PR to this one fix. Before fixing a failing check, compare it with the same check on main; if it also fails on main, leave it and note it in the PR. Run make agent-finish, then restore schemas/logs.schema.json to main's version if it regenerates again (that drift comes from Fix health failure clustering and cache evidence reporting #70885 on main, not from this PR). Reply on the PR with what changed in each item.

…ection

Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>

Copilot AI commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Review findings (round 2):

Small round 2 for this PR (#70944, issue #70938):...

Done in 2883a29:

  1. Added a test in interpolate_prompt.test.cjs that renders the Output section once per retained variant (delegated_sequential, inline_strict). Each rendered prompt has exactly one "Experiment variant" line, for that variant, and no literal {{. The test replaces ${{ … }} with a placeholder first, since those are substituted before interpolate_prompt runs.
  2. The PR description now says ${{ experiments.sub_agent_strategy }} isn't used because the compiler's expression allow-list (validateSingleExpression in pkg/workflow/expression_safety_validation.go) rejects experiments.* in prompt text, and that the limitation will be filed separately.
  3. The commit only touches the test file.

All CI checks on this PR are green. make agent-finish fails locally only in code this PR doesn't touch (pkg/ is identical to main):

  • golint: pkg/agentdrain/spec_test.go, pkg/cli/add_workflow_resolution.go, pkg/cli/spec_test.go
  • Go tests in pkg/cli, pkg/parser and pkg/workflow

I left those failures alone and noted them in the PR description. I reverted everything agent-finish regenerated, which was unrelated workflows and docs plus schemas/logs.schema.json. The schema file now matches the PR base.

@SivaKesava1
SivaKesava1 marked this pull request as ready for review October 21, 2026 21:37
Copilot AI max review requested due to automatic review settings October 21, 2026 21:37
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · haiku45 · 5.2 AIC · ⌖ 7.11 AIC · ◇ 2.47 AIC · ⊞ 9.2K · ◷

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Design Decision Gate 🏗️

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Design Decision Gate 🏗️ · opus55 · 10.3 AIC · ⌖ 31.9 AIC · ◇ 15.2 AIC · ⊞ 1.9K · ◷

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · 5.92 AIC · ⌖ 6.22 AIC · ⊞ 14.8K · ◷

Generated by Ponytail Reviewer for #70944

@github-actions

Copy link
Copy Markdown
Contributor

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)
reviewed_at: 2026-10-21T21:42:00Z
review_event: COMMENT
top_themes: extra sub-agent invocations are not rejected; experiment contract enforcement
afiles_reviewed: .github/workflows/smoke-copilot-sub-agents.md, actions/setup/js/interpolate_prompt.test.cjs, actions/setup/js/smoke_copilot_sub_agents.test.cjs
comment_count: 1

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 2.06 AIC · ⌖ 0.556 AIC · ⊞ 1.6K · ◷
Comment /review to run again

@github-actions github-actions 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.

Verdict

One correctness gap remains in the smoke contract: the post-step validates the three required children but does not reject additional sub-agent invocations. I left an inline comment requesting an exact-name/count assertion so unintended calls cannot silently pass or skew the experiment.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 2.06 AIC · ⌖ 0.556 AIC · ⊞ 1.6K
Comment /review to run again

Comment thread .github/workflows/smoke-copilot-sub-agents.md

@github-actions github-actions 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.

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — this is a well-targeted flakiness fix with solid new regression coverage. Two non-blocking findings posted inline, plus one acceptance-gate note below.

📋 Key Themes & Highlights

Key Themes

  • Reordering resolution checks before the SMOKE_EXECUTION assert is a textbook /diagnosing-bugs root-cause fix: a dead-pinned child now reports its actual model.resolution cause instead of a generic execution-failure message. The new smoke_copilot_sub_agents.test.cjs case (reports a dead child pin's model.resolution cause...) directly pins this regression — good /tdd discipline.
  • Removing single_agent_control eliminates a test design smell (a variant that was designed to always fail was polluting the smoke signal with experiment noise). Scoring via metric: eval:sub_agent_strategy_goal_met instead of pass_rate correctly decouples the orchestration-style experiment from the mandatory contract gate, following the existing copilot-pr-nlp-analysis.md precedent for eval:-prefixed metrics.
  • The "required first step" phrasing mirrors the established pattern in smoke-copilot.md (#70387), keeping prompt conventions consistent across the smoke-test family.
  • Flagged inline: the end_date: 2026-10-23 experiment window looks stale given min_samples: 30 and zero dispatches so far per the PR body — worth a bump before this starts collecting live data.
  • Flagged inline: the new "first tool call must be haiku-whoami" prompt rule has no corresponding ordering assertion in the post-step script or tests, so regressions in call order would pass silently.

Positive Highlights

  • ✅ servedModels becoming a Map keyed by agent name (was a Set of models) is a clean refactor that both preserves the existing "models must be distinct across children" invariant and enables the per-agent lookup needed for the reordered proxy-log checks.
  • ✅ interpolate_prompt.test.cjs adds a dedicated per-variant "Experiment variant:" output-line test, closing a gap where only the Tasks-section instructions were previously verified.
  • ✅ PR description is transparent about outstanding acceptance work (live dispatches, dead-pin scratch run) rather than claiming it's done.

Acceptance note: the PR body's own "Acceptance not yet done" section says no workflows have been dispatched yet. Per the repo's live-execution rules, this should gate merge until the ten dispatches and the dead-child-pin scratch run are completed and linked.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 103.6 AIC · ⌖ 15.1 AIC · ⊞ 10.2K
Comment /matt to run again

Comment thread .github/workflows/smoke-copilot-sub-agents.md
Comment thread .github/workflows/smoke-copilot-sub-agents.md

Copilot AI 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.

🟡 Changes recommended

Experiment scoring and startup diagnostics need corrections, and required hosted acceptance validation remains outstanding.

5 open issues
What changed in this PR

Updates gh-aw’s Copilot SDK sub-agent smoke workflow to require delegation in every run and separate experiment scoring from the smoke contract.

Changes:

  • Remove the negative-control variant and require all three child agents.
  • Prioritize resolution diagnostics while preserving lifecycle, served-model, and endpoint checks.
  • Add dead-pin and variant-rendering regression coverage, and regenerate the workflow lock.
File Description
actions/​setup/​js/​smoke_copilot_sub_agents.test.cjs Replace negative-control coverage with a dead-pin diagnostic case.
actions/​setup/​js/​interpolate_prompt.test.cjs Check mandatory delegation and variant-specific report rendering.
.github/​workflows/​smoke-copilot-sub-agents.md Update delegation requirements, experiment scoring, diagnostics, and reporting.
.github/​workflows/​smoke-copilot-sub-agents.lock.yml Reflect the authored workflow changes in generated Actions configuration.

Review effort: Max

Comment thread .github/workflows/smoke-copilot-sub-agents.md Outdated
Comment thread .github/workflows/smoke-copilot-sub-agents.md
Comment thread .github/workflows/smoke-copilot-sub-agents.md Outdated
Comment thread .github/workflows/smoke-copilot-sub-agents.md
Comment thread .github/workflows/smoke-copilot-sub-agents.md
… pin cause first, and rename sub-agent eval ID

Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>

Copilot AI commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

🟡 Changes recommended...

Round 3 is in 7592c28. Changes per thread:

  • Resolution checks before the execution outcome (fixed): if a child model.resolution record is missing and execution didn't succeed, the post-step now reports the execution failure and any recorded resolution causes. Missing records still fail when execution succeeds. Added regression cases for a failed main-model resolution and for an empty session after skipped execution.
  • Dead child pin (fixed): the status and cause are checked before the configured-pin comparison, and the pin check is kept. Added a dead-child-pin fixture, which reports the cause (acceptance case 2 of Stabilize smoke-copilot-sub-agents: deterministic delegation and experiment-independent contract checks #70938), and a fixture showing that a changed pin that resolves still fails.
  • Reused eval ID (fixed): renamed the eval to sub_agent_strategy_contract_met and updated the primary metric and hypothesis to match.
  • Primary metric excludes runs that fail the smoke (no change): this is about measuring the experiment, not the smoke contract. It's a follow-up if maintainers want the experiment to measure delegation reliability.
  • Evaluator can't see call counts or order (no change): same reason; also a follow-up if wanted.

The lock file is regenerated, and the 17 smoke tests pass.

make agent-finish failed on these, none of them in the files this PR changes:

  • golangci modernize (3 findings): in pkg/cli/audit_analysis_fanout.go, audit_cross_run.go and audit_diff.go, which match main.
  • TestWorkCommandPolicyFromConfigClosedFlags in pkg/cli: test failure.
  • SEC-005 cross-repository conformance check: failed.
  • gosec: failed.
  • govulncheck: no network access in the sandbox.

I didn't run these on main, so I haven't confirmed they also fail there; the latest CI run on main passes. I reverted the regenerated changes the run made to unrelated files to keep this PR to the one fix.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

Potential security threats were detected in the agent output.

Review the workflow run logs for details.

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
  2. Review (.github/workflows/smoke-copilot-sub-agents.md:101): The post-step still accepts runs that invoke extra sub-agents: it asserts exactly one invocation for each expected name, but never asserts that started contains only those three names (or that its length is three). That contradicts the mandatory 'Do not use any other agent' contract and lets unrelated calls contaminate the experiment and increase cost while the smoke gate still passes. Reject unknown agent names and assert started.length === expected.length before validating the per-agent records. - Stabilize smoke-copilot-sub-agents: mandatory delegation, experiment-independent contract checks #70944 (comment)
  3. Review (.github/workflows/smoke-copilot-sub-agents.md:31): [/grill-with-docs] end_date is still 2026-10-23 (2 days from now) with min_samples: 30, but the PR body says no runs have been dispatched yet. At the current every 2 days schedule this experiment window can't collect anywhere near 30 samples before it expires. - Stabilize smoke-copilot-sub-agents: mandatory delegation, experiment-independent contract checks #70944 (comment)
  4. Review (.github/workflows/smoke-copilot-sub-agents.md:146): [/tdd] The prompt now mandates haiku-whoami be the literal first tool call ("before any other tool call, including skills and safe outputs"), but neither the prompt-rendering tests nor smoke_copilot_sub_agents.test.cjs assert any ordering — only that all three ran exactly once. If an agent calls gpt5-whoami first, the post-step assertions still pass. - Stabilize smoke-copilot-sub-agents: mandatory delegation, experiment-independent contract checks #70944 (comment)

Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI.

Sous-chef head: 7592c28
Sous-chef work: 36ed9a8648546600b0d9d259fca19d00d58975a5b66319df1235be0411233822 91218818caa71a102e299c5de374f8a64f2b44aced0a72c23d6b074b493cf839 da9308133e079ee7066124a29b75c1eaf868eaa13bd94392e7c85f56b5c49fc8
Sous-chef state: 71ccee47ee69ac90ba3c731471078f615732d46301c1e14a4975a2ae798d0614

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · 5.85 AIC · ⌖ 6.52 AIC · ⊞ 1.1K · ◷
Comment /souschef to run again

Copilot AI and others added 2 commits October 21, 2026 23:22
…ke-copilot-sub-agents

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

[!CAUTION]
agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation....

Committed and pushed 93c96a4 after merging current main. The smoke now rejects unknown/extra child invocations and enforces Haiku-first ordering, with regression tests; the experiment ends 2027-03-01. Targeted tests and make agent-report-progress pass. make agent-finish stops at golint; the same target also fails on main with existing modernize findings. I replied to each linked review thread, but the available GitHub tools do not expose thread resolution, so they remain open. I did not dispatch CI or smoke runs; those remain for the maintainer.

@SivaKesava1

Copy link
Copy Markdown
Collaborator

Verified at 93c96a4 (fixes gh-aw#70938):

  • End to end on this head: smoke-copilot-sub-agents 39698253184 (delegated_sequential) passed. The children started in the order haiku-whoami, gpt5-whoami, terra-whoami; each ran once and completed, with no subagent.failed. Resolutions: haiku-whoami → claude-haiku-4.5 (exact), gpt5-whoami → gpt-5.6-luna (resolved), terra-whoami → gpt-5.6-terra (exact). Proxy: claude-haiku-4.5 on /chat/completions and gpt-5.6-luna/gpt-5.6-terra on /responses, one 200 each.
  • Earlier branch runs at 575b683: 3 of 3 passed, 2× inline_strict and 1× delegated_sequential (39683800931, 39683813970, 39683825963), all three children each time.
  • Review rounds:
    • A missing child resolution after a failed execution now reports the execution failure.
    • A dead child pin's cause is checked before the pin comparison.
    • The new eval ID sub_agent_strategy_contract_met keeps old answers out.
    • Sous Chef's additions (reject unknown or extra children, Haiku first, experiment end 2027-03-01) only tighten the gate, and both variants share the Haiku-first task.
    • The two experiment-measurement threads are answered as follow-ups outside Stabilize smoke-copilot-sub-agents: deterministic delegation and experiment-independent contract checks #70938's smoke-contract scope.
  • Not used, on purpose: ${{ experiments.* }} in prompt text, because the compiler's expression allow-list rejects it. That limit will be filed separately.
  • Scope: only the workflow, its lock file (a clean recompile) and the two tests that read it.

@SivaKesava1
SivaKesava1 merged commit e3f49c7 into main Oct 22, 2026
25 checks passed
@SivaKesava1
SivaKesava1 deleted the copilot/stabilize-smoke-copilot-sub-agents branch October 22, 2026 00:32
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.

Stabilize smoke-copilot-sub-agents: deterministic delegation and experiment-independent contract checks

4 participants