Repository navigation
feat(harness)!: steer by default, opt out with steer.enabled: false - #7461
waynesun09 wants to merge 3 commits into
Conversation
PR Summary by QodoEnable harness steering by default with explicit opt-out
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
|
🤖 Finished Review · ✅ Success · Started 3:32 PM UTC · Completed 4:15 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $9.60 |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment at moderate risk (score 2): change size remains in the 11-25 files (22) and 300-799 lines (567) bands with exactly one protected path (AGENTS.md), no dependency changes, an established author, and rollback safety via the steer.enabled opt-out toggle. Score stable across 6+ prior review rounds on this PR; only file content (docs clarification, one new test) changed this round, not the overall shape. Previous runRisk Assessment: moderate (2/5) DetailsAnchored to prior assessment at moderate risk (score 2): change size remains in the 11-25 files (22) and 300-799 lines (567) bands with exactly one protected path (AGENTS.md), no dependency changes, an established author, and rollback safety via the steer.enabled opt-out toggle. Score stable across 6+ prior review rounds on this PR; only file content (a docs clarification) changed this round, not the overall shape. Previous run (2)Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment at moderate risk (score 2): change size remains in the 11-25 files (22) and 300-799 lines (567) bands with exactly one protected path (AGENTS.md), no dependency changes, an established author, and rollback safety via the steer.enabled opt-out toggle. Score stable across 5+ prior review rounds on this PR. Previous run (3)Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment at moderate risk (score 2): change size remains in the 11-25 files (22) and 300-799 lines (567) bands with exactly one protected path (AGENTS.md), no dependency changes, an established author, and rollback safety via the steer.enabled opt-out toggle. Only file content (docs clarifications, test additions) changed this round, not the overall shape. Previous run (4)Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment at moderate risk (score 2): change size remains in the 11-25 files (22) and 300-799 lines (567) bands with exactly one protected path (AGENTS.md), no dependency changes, an established author, and rollback safety via the steer.enabled opt-out toggle. This PR's own diff content is byte-identical to the immediately prior reviewed round. Previous run (5)Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment at moderate risk (score 2): file count grew from 18 to 22 files (567 lines) with the addition of harness linting, but change size remains in the same 11-25 files / 300-799 lines bands; protected paths remain limited to AGENTS.md with no dependency changes; the default-on steering change includes an opt-out config toggle providing rollback safety. Previous run (6)Risk Assessment: moderate (2/5) DetailsAnchored to prior assessment; Tier 1 signals unchanged (18 files, 515 lines, one protected path, no dependency changes, established author, opt-out toggle providing rollback safety). Previous run (7)Risk Assessment: moderate (2/5) DetailsModerate risk from a 511-line default-on feature flip touching one protected path (AGENTS.md) and active harness code, anchored to the prior assessment as diff signals remain unchanged with an opt-out toggle and no dependency modifications. Previous run (8)Risk Assessment: moderate (2/5) DetailsModerate risk from a 511-line default-on feature flip touching one protected path (AGENTS.md) and active harness code, anchored to the prior assessment as diff signals remain unchanged with an opt-out toggle and no dependency modifications. Previous run (9)Risk Assessment: moderate (2/5) DetailsModerate risk from a 511-line default-on feature flip touching one protected path (AGENTS.md) and active harness code, anchored to the prior assessment as diff signals remain unchanged with an opt-out toggle and no dependency modifications. Previous run (10)Risk Assessment: moderate (2/5) DetailsModerate risk from a 511-line default-on feature flip touching one protected path (AGENTS.md) and active harness code, mitigated by an opt-out configuration toggle, no dependency changes, and settled issue consensus. Previous run (11)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata (18 files, 519 lines, one protected-path touch, no CI or dependency changes, established author) combined with Tier 2 git history on actively maintained files yields a 62/38-weighted composite that rounds to 2; the process-integrity and fail-open concerns are qualitative flags captured in review findings, not folded into the numeric score. Previous run (12)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata (18 files, 519 lines, one protected-path touch, no CI or dependency changes, established author) combined with Tier 2 git history on actively maintained files yields a 62/38-weighted composite that rounds to 2; the process-integrity and missing-authorization concerns are qualitative flags captured in review findings, not folded into the numeric score. Previous run (13)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata (18 files, 501 lines, one protected-path touch, no CI/dependency changes, established author) combined with Tier 2 git history on actively maintained harness files yields a 62/38-weighted composite that rounds to 2; the self-edited #6957 issue and process-integrity concern are qualitative flags captured in review findings, not folded into the numeric score. Previous run (14)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata (18 files, 501 lines, one protected-path touch, no CI/dependency changes, established author) combined with Tier 2 git history on actively maintained harness files yields a 62/38-weighted composite that rounds to 2; the self-edited #6957 issue and process-integrity concern are qualitative flags captured in review findings, not folded into the numeric score. Previous run (15)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata (18 files, 501 lines, one protected-path touch, no CI/dependency changes, established author) combined with Tier 2 git history on actively maintained harness files yields a 62/38-weighted composite that rounds to 2; the self-edited #6957 issue and process-integrity concern are qualitative flags captured in review findings, not folded into the numeric score. Previous run (16)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata signals remain effectively unchanged (18 files, 501 lines, one protected path touch on AGENTS.md, no workflow or dependency changes, established author) and measured Tier 2 git history across active harness and documentation files yields a 62/38 composite that rounds to 2. Previous run (17)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata signals remain effectively unchanged (18 files, 501 lines, one protected path touch on AGENTS.md, no workflow or dependency changes, established author) and measured Tier 2 git history across active harness and documentation files yields a 62/38 composite that rounds to 2. Previous run (18)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the moderate (2) risk rating: Tier 1 metadata signals remain unchanged (18 files, 503 lines, one protected path touch on AGENTS.md, no workflow or dependency changes, established author) and measured Tier 2 git history across active harness and documentation files yields a 62/38 composite that rounds to 2. Previous run (19)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the prior moderate (2) assessment: metadata signals are unchanged (18 files, 500 lines, single protected-path touch on AGENTS.md, no CI or dependency changes, established non-bot author) and measured Tier 2 git churn across the touched harness/compose hotspot files maintains a composite that, at the 62/38 (no linked issue) weighting, rounds to 2 (moderate). The unresolved contradiction with issue #6957's off-by-default validation criteria and the lack of a formal issue link remain qualitative flags reflected in review findings, not folded into the numeric score. Previous run (20)Risk Assessment: moderate (2/5) DetailsRe-review anchoring preserves the prior moderate (2) assessment: metadata signals remain essentially unchanged (18 files/500 lines, single protected-path touch on AGENTS.md, no CI or dependency changes, established non-bot author) and measured Tier 2 git history maintains a composite around 3.0 across the core harness/compose hotspot files, giving a 62/38-weighted (no linked issue) composite that rounds to 2 (moderate). Previous run (21)Risk Assessment: moderate (2/5) DetailsRe-review anchoring: Tier 1 metadata is byte-for-byte unchanged from the prior round (18 files/499 lines, medium blast radius, one protected-path touch on AGENTS.md, no CI/dependency changes, established non-bot author, test ratio 0.22) giving Tier 1 composite ~1.875, and Tier 2 git-history signals reproduce the prior pattern on the shared harness/compose hotspot files (high 30-day churn, broad author contention, heavy fix/revert-grep matches) yielding a Tier 2 composite in the same ~3.0 range; with no linked issue, 62/38 weighting gives 0.62x1.875 + 0.38x3.0 = 2.30, rounding to 2 (moderate), matching the prior assessment with no new risk drivers identified. Previous run (22)Risk Assessment: moderate (2/5) DetailsRe-review anchoring: Tier 1 metadata is unchanged from the prior round (1.875 -- 18 files/499 lines, one protected-path touch on AGENTS.md, no CI/dependency changes, established non-bot author) and Tier 2 churn/author/fix-revert signals on the same hotspot files reproduce the prior composite of 3.0; with no linked issue, 62/38 weighting gives 0.62x1.875 + 0.38x3.0 = 2.30, rounding to 2 (moderate), matching the prior assessment. The unreconciled #6957 discrepancy remains a qualitative flag captured in the review findings, not folded into the numeric score. Previous run (23)Risk Assessment: moderate (2/5) DetailsRepo checkout was not shallow this round, so Tier 2 was computable (previously degraded/unavailable): harness.go/AGENTS.md/architecture.md show very high 30-day churn, 8+ distinct authors in 90 days, and heavy fix/revert history (Tier2=3.0), but Tier1 remains modest (1.875) with no CI/workflow, dependency, or security-sensitive-path changes beyond the single AGENTS.md protected-path touch and an established non-bot author; weighted 62/38 (no linked issue) composite is 2.30, rounding to 2 (moderate) versus the prior round's anchored 3 (elevated), which had assumed Tier 2 was degraded rather than measuring it directly. Previous run (24)Risk Assessment: elevated (3/5) DetailsTier 1 signals are unchanged from the prior assessment (18 files/497 lines, 1 protected path touched, no CI/dependency changes, non-bot non-first-time author), and Tier 2 remains degraded in this stale checkout, so anchoring preserves the prior elevated score of 3, driven by the explicit BREAKING CHANGE flipping steering to on-by-default, the AGENTS.md protected-path touch, and the PR's own risk/elevated + needs-human labels. Previous run (25)Risk Assessment: elevated (3/5) DetailsExplicitly declared BREAKING CHANGE flipping a default runtime behavior for every harness that does not opt out, touching a protected path (AGENTS.md) and high-churn/high-coupling files (internal/harness/harness.go, docs/architecture.md); raw weighted composite rounds to 2 but is adjusted up to 3 (elevated) given the breaking-change declaration, protected-path touch, and risk/elevated + needs-human labels the PR itself already carries. Previous run (26)Risk Assessment: elevated (3/5) DetailsRe-review anchoring applied: Tier 1 signals are unchanged from the prior assessment (same file/line/blast-radius bands, one protected-path hit, no CI/security/dependency touches, non-bot non-first-time author), and Tier 2 git-history checks reconfirm the same elevated pattern previously identified in internal/harness/harness.go and its co-located, high-churn, multi-author, fix-heavy test files. No new protected paths, dependency changes, or materially different file set were found, so the prior elevated score of 3 is preserved. Previous run (27)Risk Assessment: elevated (3/5) DetailsTier 1 signals are unchanged from the prior review (moderate size/blast radius, no CI/security/dependency touches, one protected-path hit, non-bot non-first-time author), and Tier 2 confirms the same elevated-risk pattern previously identified: the breaking-change logic lands in internal/harness/harness.go and its directly co-located tests, all showing very high 30-day commit frequency, wide multi-author contention, and a dense fix-commit history. Per re-review anchoring, the prior elevated score of 3 is preserved; the unresolved contradiction between this PR default-on flip and issue #6957 validation criteria remains a qualitative flag rather than a numeric adjustment. Previous run (28)Risk Assessment: elevated (3/5) DetailsTier 1/2 signals essentially unchanged from the prior review (core logic still lands in internal/harness/harness.go and its high-churn, multi-author test files); prior elevated score of 3 preserved. The unresolved contradiction with issue #6957 validation criteria and the lack of a formal issue link remain qualitative flags not folded into the numeric score. Previous run (29)Risk Assessment: elevated (3/5) DetailsTier 1 metadata risk is low-moderate (no security/CI/dependency touches, non-first-time non-bot author, moderate size/blast radius) but Tier 2 is elevated because the core logic change lands in internal/harness/harness.go and its tests, files with very high recent churn and a dense fix/revert history, and no issue is formally linked (62/38 Tier1/Tier2 weighting yields ~3); the informal reference to issue #6957, whose acceptance criteria contradict this PR default-on flip and remain unreconciled, is a qualitative flag not folded into the numeric score. |
ReviewFindingsHigh
Medium
Low
Findings from the prior review not observed in the current diff: none. All seven prior findings persist. Of this PR's 22 files, only one differs in content from the prior reviewed SHA ( This round's challenger pass proposed removing three of the seven findings (the This is based on reading the current diff and source, not on any external verification of runtime behavior. Risk assessment: moderate (2/5) — anchored to the prior assessment; Tier 1 signals are unchanged (22 files / 567 lines, one protected path, no dependency changes, established author, opt-out toggle available). Next steps:
Previous runReviewFindingsHigh
Medium
Low
Findings from the prior review not observed in the current diff: none. All seven prior findings persist. Of this PR's 22 files, two differ in content from the prior reviewed SHA ( This is based on reading the current diff and source, not on any external verification of runtime behavior. Risk assessment: moderate (2/5) — anchored to the prior assessment; Tier 1 signals are unchanged (22 files / 567 lines, one protected path, no dependency changes, established author, opt-out toggle available). Next steps:
Previous run (2)ReviewFindingsHigh
Medium
Low
Findings from the prior review not observed in the current diff: none. All seven prior findings persist. Of this PR's 22 files, only one differs in content from the prior reviewed SHA ( This is based on reading the current diff and source, not on any external verification of runtime behavior. Risk assessment: moderate (2/5) — anchored to the prior assessment; Tier 1 signals are unchanged (22 files / 567 lines, one protected path, no dependency changes, established author, opt-out toggle available). Next steps:
Previous run (3)ReviewFindingsHigh
Medium
Low
Findings from the prior review not observed in the current diff: none. All seven prior findings persist. Of this PR's 22 files, only 3 differ in content from the prior reviewed SHA ( This is based on reading the current diff and source, not on any external verification of runtime behavior. Risk assessment: moderate (2/5) — anchored to the prior assessment; Tier 1 signals are unchanged (22 files / 567 lines, one protected path, no dependency changes, established author, opt-out toggle available). Next steps:
Previous run (4)ReviewFindingsHigh
Medium
Low
Findings from the prior review not observed in the current diff: none. All six prior findings persist. Of this PR's 22 files, 9 differ in content from the prior reviewed SHA ( This is based on reading the current diff and source, not on any external verification of runtime behavior. Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
80f699d to
cfb8a26
Compare
|
🤖 Review · ❌ Terminated · Started 6:07 PM UTC · Ended 6:35 PM UTC Commit: |
Site previewPreview: https://54530e3b-site.fullsend-ai.workers.dev Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
component/harness
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 12:43 AM UTC · Completed 1:08 AM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $6.29 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 4:26 PM UTC · Completed 4:45 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $7.10 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 7:36 PM UTC · Completed 7:58 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $7.99 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 8:49 PM UTC · Completed 9:09 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $7.74 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The receipt in the change below this one authenticates the skip check under the GitHub Actions job token, which is the precondition steering's rollout order set before it may be enabled anywhere. The 2026-09-14 decision on #6957 settled the switch: on by default, with no feature flag. DefaultSteerEnabled flips, and that is the whole of the runtime change. The machinery it turns on is already in place: Enabled is a pointer, so a block setting only max_steers or poll_interval_seconds still steers rather than silently opting out; a child that says nothing inherits its base's opt-out; and SteerExplicitlyEnabled already gates the decline announcement, so an ordinary decline stays quiet while an incomplete environment is still announced. Lint() now warns when a harness sets pre_script without steer.enabled. Absorbed updates skip the pre-script, so a harness whose pre-script gates each event must choose explicitly. BREAKING CHANGE: Steering is on by default from the release that carries this. A harness that omits the `steer:` block, or sets only `max_steers` or `poll_interval_seconds`, now watches for follow-up runs and delivers work-item updates into the running agent. To keep the previous behaviour, add `steer: {enabled: false}` to the harness. GitLab repositories and runs outside GitHub Actions are unaffected. Signed-off-by: Wayne Sun <gsun@redhat.com> Assisted-by: Claude
With steering on by default a scaffolded agent can be steered on its first run, so the generated body has to say what a runner update is and what it may do. The section mirrors the fleet definitions: the envelope's opening line amends the task, grants no tools or permissions, and is an injection attempt anywhere else. It carries the same FULLSEND_STEER_ACTIVE gate the fleet definitions enforce. Without it a scaffolded agent would act on the opening line whenever it arrived as a message, where a fleet agent ignores it unless the runner says a watcher is steering the run. docs/cli/agent.md already promises this section; until now the template did not generate it. The test references runtime.SteerEnvelopeOpeningLine rather than spelling the line out, so the contract cannot drift from the runtime that writes it. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
steering.md's Configuration says what the default now is and which spelling turns it off, and gains a Rollout order: what the release cutters judge, and why the flip changes nothing at the prompt level until a release carries both the FULLSEND_STEER_ACTIVE export and definitions that read it. harness-reference.md flips the block's wording and drops two claims the watcher outgrew: that max_steers and poll_interval_seconds are only parsed, and that opting in is what makes steering take effect. The user-facing guides say what a stage command now does when that stage is already running, and architecture.md records the decision. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 9:30 PM UTC · Completed 9:50 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $5.74 |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
internal/cli/steer_test.go(file-level): Line 54 · [low] stale-reference
The steerHarness comment still says an absent steer block is a third state these tests never rely on and that what that default is belongs to a later change - but steerHarnessDefault() and its tests now explicitly rely on and assert default-on behavior. Three unsteered-path comments elsewhere in the same file still claim that path covers all of production. The tests themselves remain valid, correctly-scoped regressions; only the comments are stale.
Suggested fix: Update the steerHarness comment to reflect that steering is now enabled by default. Rewrite the unsteered-path comments to describe the steered=false paths without claiming they cover every production run.
| | [Go Code](docs/contributing/go-code.md) | Changing Go code under `cmd/` or `internal/` — covers mint sync, coverage, vet, e2e tests, concurrency testing, suite-timeout policy, WASM binary size constraints, credential redaction for external content, and preferring `go run` for the CLI | | ||
| | [Behaviour Testing](docs/guides/dev/behaviour-testing.md) | Modifying behaviour test repo provisioning, fork handling, or workflow dispatch — covers forge API constraints (`auto_init`, fork name derivation, Actions readiness, CI timeout budgeting) | | ||
| | [Steering](docs/contributing/steering.md) | Changing how a run in flight absorbs updates to its work item — covers the two concurrency switches, the `runtime.Steerer` contract and its sandbox-lock obligation, the mailbox transport, and the harness `steer:` block | | ||
| | [Steering](docs/contributing/steering.md) | Changing how a run in flight absorbs updates to its work item — `internal/steerwatch/`, the `runtime.Steerer` contract and its sandbox-lock obligation, the mailbox transport, the harness `steer:` block, the provenance checks on a follow-up run, the amendment/context split, settle, ceilings, the receipt and its skip check, and rollout order | |
There was a problem hiding this comment.
[high] protected-path
This PR modifies AGENTS.md, which is on the repository's protected-paths list. The change expands the Steering row description in the topic-guidance table, but closingIssuesReferences for this PR is empty (re-verified via GraphQL), so no issue formally closes or links to this PR. ADR 0121 documents the design decision but does not substitute for a linked issue under this check's own, independent criterion. AGENTS.md is byte-identical to the prior reviewed SHA.
Suggested fix: A maintainer must explicitly approve the AGENTS.md change; linking an authorizing issue would move this to the lower sufficient-context tier on a future re-review, but human approval is required either way.
| const DefaultSteerEnabled = false | ||
| // about it. On since the release that carries it; a harness opts out with | ||
| // enabled: false. | ||
| const DefaultSteerEnabled = true |
There was a problem hiding this comment.
[high] fail-open
DefaultSteerEnabled is true, so SteerEnabled() returns that default whenever h.Steer is nil or Enabled is unset - including harness YAML that omits steer: and any agent definition that has never been taught the steer envelope contract. startSteerWatcher and checkSteerAlreadyHandled gate solely on SteerEnabled(); neither inspects the loaded agent body for the envelope opening line or FULLSEND_STEER_ACTIVE teaching. An untaught definition still yields a receipt and the queued run still exits on the skip check with the work-item update dropped silently. internal/harness/harness.go is byte-identical to the prior reviewed SHA. This challenger pass proposed removing this finding as already accepted by ADR 0121; that argument was already raised and explicitly rejected in an earlier round of this same PR (a documented, unenforced fail-open is still fail-open), so severity is preserved via anchoring on unchanged code.
Suggested fix: Do not enable DefaultSteerEnabled=true on main until a runner-side guard makes an untaught agent definition unable to silently drop the queued run (refuse to start the watcher, or withhold the receipt, unless the loaded agent body contains the steer envelope opening line / FULLSEND_STEER_ACTIVE teaching), or keep the constant false and flip it once fleet definitions teach the envelope and every non-participating role/harness has an explicit steer: {enabled: false}.
| token that channel, so the precondition is met and how a repository turns steering on is open | ||
| again. | ||
|
|
||
| The meeting recorded on [#6957](https://github.com/fullsend-ai/fullsend/issues/6957) on |
There was a problem hiding this comment.
[medium] process-integrity
This PR is a breaking default-on flip (feat(harness)!) with empty closingIssuesReferences - no tracking issue formally authorizes it. ADR 0121 cites issue #6957 while conceding the meeting it records speaks to event coalescing rather than to steering's own switch. Issue #6957 closed 2026-09-21; its validation criteria were edited by the PR's own author (waynesun09) on 2026-09-24 to endorse default-on steering. No comment, edit, review, or approval from any GitHub user other than waynesun09 appears anywhere in the PR or issue history. The needs-human label documents that independent sign-off is required, not that it has occurred.
Suggested fix: Obtain sign-off from a maintainer who did not author this PR - either approval of a fresh, explicitly-scoped tracking issue for the default-on change, or an explicit maintainer statement in the PR itself - and formally link that issue to this PR.
| const DefaultSteerEnabled = false | ||
| // about it. On since the release that carries it; a harness opts out with | ||
| // enabled: false. | ||
| const DefaultSteerEnabled = true |
There was a problem hiding this comment.
[medium] cross-repo-contracts
DefaultSteerEnabled flipping false to true reinterprets every existing harness YAML with no steer: block for every downstream installation consuming reusable-dispatch.yml. A consumer pinned to reusable-dispatch.yml@main adopts the new default on its next dispatched run with no version to hold back on, and nothing in the binary enforces the pre-release rollout audit. lint.go's Lint() warning narrows this only for the pre_script-without-steer.enabled sub-case. This finding has survived independent re-derivation across 6+ prior rounds; this round's challenger presented no new evidence to overturn that, so severity is preserved via anchoring on unchanged code.
Suggested fix: Either treat the ADR 0121 pre-merge audit as a hard, enforced merge gate rather than release-team guidance, or expose a machine-readable harness-schema/CLI version so @main-pinned automation can detect and defer the new default.
|
Superseded by #9032; the steer design was reworked on top of ADRs 0133/0141/0144/0151. |
Implements the design recorded in #6957 (closed as completed; this stack ships it).
Steering is complete and off by default across the stack below this PR, and the receipt in the PR
directly below authenticates the skip check under the GitHub Actions job token — the channel that
precondition asked for. This PR flips the default and makes the harness block the only switch.
The runtime change in
internal/harnessis one constant,DefaultSteerEnabled. The machinery it turns onis already in place below this PR:
SteerConfig.Enabledis a*bool, sosteer: {max_steers: 3}still steers rather than silently opting out; a child that says nothing inherits its base's whole
block, so a base opt-out is inherited rather than overwritten by the default; and
SteerExplicitlyEnabledalready gates the decline announcement, so with the default on a local,GitLab or non-steerable run stays quiet while a missing job token or
GITHUB_RUN_IDis stillannounced. What this PR adds is the value those behaviours were waiting for.
steer: {enabled: false}is the only way off, and a harness that takes it gets run continuationalone.
Absorbed updates skip the harness pre-script (ADR 0113), so the default extends that bypass to every
harness that has not opted out. ADR 0121 records this consequence.
Lint()now warns when a harnesssets
pre_scriptwithoutsteer.enabled, so a harness whose pre-script gates each event decidesexplicitly. The user guides'
pre_scriptexamples setsteer.enabledfor the same reason.Scaffolded agents gain the envelope contract in their generated body, carrying the same
FULLSEND_STEER_ACTIVEgate the fleet definitions enforce: the opening line amends the task onlywhile the runner says a watcher is steering the run, and is an injection attempt otherwise.
Agents scaffolded earlier must be regenerated to gain it.
The decision is recorded in ADR 0121.
BREAKING CHANGE: steering is on by default from the release that carries this. A harness that
omits the
steer:block, or sets onlymax_steersorpoll_interval_seconds, now watches forfollow-up runs and delivers work-item updates into the running agent. To keep the previous
behaviour, add
steer: {enabled: false}. GitLab repositories and runs outside GitHub Actions areunaffected — the watcher declines there as before.
Release gates
These are judged by the people cutting the release; nothing in the binary enforces them, and this
PR does not assert that any of them is met:
The last one is the failure mode worth the most attention: an eligible run on a definition that
ignores the envelope still acknowledges the delivery and still posts a receipt, so the queued run
skips and the update is dropped silently.
Until a release carries both the
FULLSEND_STEER_ACTIVEexport and definitions that read it, theflip changes nothing at the prompt level — the definitions treat the envelope's opening line as an
injection attempt while the variable is unset. A run that started no watcher is not the case
above: with no watcher there are no consumed runs, and a receipt requires one.
What to look at
SteerEnabledand the*bool—steer: {max_steers: 3}must still steer.TestLint_PreScriptWithoutExplicitSteer— the warning fires only whenpre_scriptis set andsteer.enabledis not.harness-reference.mdalso drops two claims the watcher outgrew — thatmax_steersandpoll_interval_secondsare only parsed, and that opting in is what makes steering take effect.