You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
canary-rollout: ingress attribution follow-ups from the #1238 review (pre-adoption runs, decision-class read failures, role scoping) #1244
The pr-review cascade's findings on #1238 (ADR-0007 ingress attribution) that were not fixed in that PR. The liveness blockers were fixed there: the CANARY_INGRESS_JOBS_MAX cap no longer holds a busy repo BLOCKED forever for the baseline read, the jobs-read failure circuit breaker, and the .nf marker ordering. These are lower-severity and need their own tests and review. All fail closed or only affect issue text, except the third item, which should be checked first.
Items
Pre-adoption ingress runs make a new role UNRESOLVED. An ingress run from before the role's job was added to agent-ingress.yml has no role-carrying job, so it reduces to UNATTRIBUTED and the member stays UNRESOLVED until that run ages out of both windows. Treat no-role runs older than the oldest run that carries the role as not-yet-adopted. (_ingress_agent_runs)
startup_failure on a job-less ingress run is attributed to every role, so it counts as a failure for every agent in the repo. It fails closed but can produce false blocks.
_run_decision_class swallows jobs-read failures and returns an empty class, which reads as insufficient decision-mix. A sustained jobs failure is ignored in mix-shift sampling while the gate records UNRESOLVED for the same condition elsewhere. Check this first. It should either fail closed or be recorded as UNRESOLVED.
_run_is_stale role scoping. The reviewer says another role's pinned release in a shared ingress run could mark this agent's failure as an old-release failure. _run_reusable_sha only matches Uses: lines for the agent's own reusable, so this may not reproduce. Verify and add a test either way.
Candidate-window cap. The strict cap still marks a member UNRESOLVED when the candidate window (cut → now) holds more than CANARY_INGRESS_JOBS_MAX ingress runs. That is only reachable on a very busy repo after a long soak, but a per-role attributed-sample bound would remove it.
_unresolved_evidence re-walks the API separately from the gate decision, so the blocker issue text can disagree with the triage if job state changes in between. It is cosmetic.
Cross-repo parity. The role-reduction logic is deliberately duplicated from .github-private's run-attribution.sh. Add a parity test or a shared source so the two cannot drift.
Acceptance criteria
Each item is fixed or explicitly closed as "does not reproduce" with a test.
Issue:#1244 — canary-rollout: ingress attribution follow-ups from the #1238 review
Blocker
All seven items target code that exists only on PR #1238's branch (dev-lead/issue-1224-20261002-2328): _ingress_agent_runs, the role-scoped _run_decision_class, _unresolved_evidence, CANARY_INGRESS_JOBS_MAX, and the run-attribution.sh reduction. #1238 is still open / REVIEW_REQUIRED. main (2716f0e), which this run's branch is cut from, has none of it.
Implementing here would mean merging the whole unreviewed #1238 diff into a second PR against main. That would duplicate #1238 and could conflict with its review, so no source changes were made in this run.
Pre-analysis against the #1238 branch (85ff3d3), for whoever picks this up
Item 3 (check first) — reproduces by reading the code._run_decision_class does _run_jobs_json … || echo '{}', then decision_class returns empty. _sample_decision_counts silently continues on an empty class. A sustained jobs-read failure therefore degrades to INSUFFICIENT and is never recorded as UNRESOLVED (fail-open, as the function's own comment says). A failed read is also cached in _RUN_DECISION_CACHE. Suggested fix: return non-zero on a read error, distinct from "no decision step". _sample_decision_counts would then surface it, and _correctness_verdict would map it to UNRESOLVED or a hold.
Item 4 — likely does not reproduce, with one caveat. _run_reusable_sha keeps only the first Uses: line per job and requires Uses: $host/$reusable@. Another role's pin to a different reusable can't match. The open case is two roles in one ingress run that call the same reusable at different pins: _run_reusable_sha is not role-scoped (no job-name prefix filter), so the other role's SHA would be collected. That is still fail-closed whenever any collected SHA is the candidate's. A test should cover both the different-reusable and the same-reusable cases.
Update: item 1 (pre-adoption runs) and the job-less cancelled-run part of the startup_failure/job-less item were fixed in #1238 (8fa31d4) after pr-review rated pre-adoption MAJOR. A no-role run older than the role's first appearance is now ignored; a newer one, or no run carrying the role at all, stays UNRESOLVED. Job-less cancelled runs are no longer UNATTRIBUTED. Items 2 (startup_failure attributed to every role), 3, 4, 5, 6 and 7 remain open.
Two non-blocking notes from pr-review's approval of #1238 at 8fa31d4, deferred to avoid a re-review for nits:
role_oldest set from a job-less startup_failure. In _ingress_agent_runs, a job-less startup_failure record carries no role job but still sets role_oldest. With a misspelled ingress_job, no-role runs older than such a run would read as pre-adoption instead of blind. It does not fail open (the startup_failure counts as a failure, and newer no-role runs stay UNRESOLVED), but role_oldest should be set only when a role job was actually observed. Overlaps item 2.
Doc drift._ingress_note in standards/canary-rings.json still says a job-less ingress run is UNRESOLVED. That is no longer true for a cancelled run, and the note does not mention the pre-adoption rule.
Follow-ups carried forward from pr-review's approval of #1238 (merged, 36444b1), all non-blocking:
Item 11: The ingress run list (-L ${CANARY_INGRESS_RUN_LIMIT:-5000}) can silently truncate; the jobs cap fails closed but the list cap does not. Detect a list that hit its limit and treat it like the jobs cap (UNRESOLVED or truncation-aware baseline).
Item 12: A job-less cancelled run is classed as benign cancelled before the UNATTRIBUTED branch, so a lagging jobs API drops it rather than flagging it. Distinguish "cancelled before any job started" from "jobs not recorded yet".
Item 14 (policy, from #1243 review): In _frontier_state_resilient's data-gap path, an unresolvable (empty) source commit is classified PRE_EXISTING on main (_reusable_differs returns 0 when the candidate is empty). Reviewers disagreed on the right triage: CodeAnt wanted it not to suppress regression triage, cubic objected to REGRESSION escalating a tag-gap hold to needs-human, and an intermediate - (indeterminate) was tried and then reverted because it is a behaviour change outside a pure refactor. Decide the intended triage for "pair pending, source tag unresolvable, run history unreadable" (likely -/indeterminate, since a tag gap says nothing about the reusable), implement it in its own PR with a test, and confirm how sync-issues ranks it (PRE_EXISTING and - currently rank the same for needs-human). The #1243 regression test pins today's PRE_EXISTING, so changing it will flip that test deliberately.
Item 10 (added late; it was referenced from the #1238 review threads but not recorded here): When the per-role workflow is absent, the ingress exists, and no ingress run completed in the window, _ingress_agent_runs returns [] without checking that the live ingress declares the registered ingress_job. A misspelled or renamed ingress_job then looks like "no caller", and waive_sample_if_no_caller can allow a dwell-only promotion. Impact is limited: the moment any ingress run appears without the role's job it is flagged NOROLE/UNRESOLVED. Both pr-review reviewers and Codex (P1) raised it; it was deferred in #1238 because it needs an authoritative workflow-definition read (the live agent-ingress.yml jobs), which is a design change. The registry already carries an unused ingress.file that could drive the check. Options: validate the role job against the live ingress file, warn on "ingress present, zero runs, role never seen", or accept as is.
Also held here (from the #1243 review, non-blocking): _frontier_state_resilient hand-checks the unknown flag instead of using pair_verdict, the empty-vs-- normalization is repeated at each call site, and the orchestrator case arms have no direct tests.
Sequencing: one PR for #1246 items 3, 11, 12 together with #1248 (item 13), then item 10 once its approach is decided, then #1249 in its own PR (it flips a pinned test on purpose). The dev-lead label is removed from this umbrella so automation does not try to implement all items in one PR.
Item 15 (found in the #1250 review, pre-existing):_repo_wf_runs_cached passes successful gh run list output through without validating it is a JSON array. On the legacy per-role path a non-array body is then reduced by jq … || echo '[]' to [], which reads as "no runs / no caller" (fail-open on malformed data). #1250 fixes this for the ingress path only (non-array → UNRESOLVED). Fix the shared reader instead: have _repo_wf_runs_cached treat a non-array success body like a failed fetch (retry, then fail closed with the existing fetch-failure flag), with a test. Low severity (needs a malformed 2xx from gh), small change.
Item 16 (policy question, raised by cubic on #1250, inherited from #1238): When a capped (truncated) baseline read observes nothing countable for this agent, _baseline_daily emits a non-zero floor (1), which sizes the sample target at its clamp minimum (so it can never read as "no caller" and waive sampling, the #1238 fix). cubic argues the fail-closed choice is the clamp maximum instead: an unknown baseline should demand the largest sample, not the smallest. Both fail closed against the waive-sampling hole; they differ in how much evidence a promotion needs when the baseline is unknown.
Today (clamp minimum): a busy collapsed repo whose capped window shows no runs for this agent can promote on the minimum sample (default 3 executed runs).
Alternative (clamp maximum): safer, but a busy repo whose cap is always hit would then always need the maximum sample, which may be slow to accumulate.
This applies equally to the jobs cap (#1238) and the run-list cap (#1250), so it should be decided once. Needs a maintainer decision; implementation would signal "unknown baseline" from _baseline_daily rather than a fake count, and force sample_clamp_max in the target computation. Not changed in #1250.
Item 17 (found by pr-review on #1250, pre-existing; corrects an overstatement of mine): the gate's timestamp window filters compare ISO strings lexicographically (createdAt >= since, and #1250 added the same convention to the run-list completeness check), which is only sound when every since is Zulu. _iso_now_minus_days and the tag-date path through _to_z are Zulu, but candidate_cut_date's last-resort local-host fallback returns git log -1 --format=%cI "$commit" (scripts/canary-rollout.sh, ~line 537) without_to_z, so it can carry a non-Z offset. I said on #1250 that every since is Zulu; that is true except for this fallback.
Fix (one line + a test): _to_z "$(git log -1 --format=%cI "$commit" 2>/dev/null)", falling back to "", so the Zulu assumption holds everywhere. A test should feed a %cI with a non-Z offset and assert the window filters see the Zulu equivalent. Low severity: needs a cut date resolved from the local checkout with no release tag, and an offset-bearing committer date.
Open, small: validate the run-list shape in the shared reader (legacy path)
16
Open, needs a maintainer decision: empty truncated baseline sized at clamp minimum (today) vs maximum
17
Open, one-line fix + test: _to_z the local-host candidate_cut_date fallback
6, 7
On hold
Decisions waiting on the maintainer: #1246 item 10 (approach), item 16 (clamp min vs max), #1249 (item 14 triage), and the #1247 policy questions (job cap value, collapsed-member-lacks-a-role rule). Items 15 and 17 are small and unblocked; they can be bundled into one PR whenever convenient.
@petry-projects/solution-architect — structural review requested by the owner on the decisions still open in this tracker. Context: the canary-rollout gate (scripts/canary-rollout.sh) attributes ADR-0007 agent-ingress.yml runs to a role by job key; recent merges are #1250, #1271, #1276 (static registry lint for ingress_job, canonical standards/workflows/agent-ingress.yml) and #1282 (per-role run list date-bounded to 15 days). The standing principle is "fail closed, but an expected outcome must not read as a failure". For each question please give a recommendation, the governing ADR if one applies (ADR-0007, ADR-0010, others), and say plainly when no ADR governs.
1. Item 16 — unknown baseline sizing. When a capped (truncated) baseline read observes nothing countable for an agent, _baseline_daily emits a floor of 1, which sizes the required promotion sample at its clamp minimum (default 3 executed runs). Alternative: signal "unknown baseline" explicitly and force the clamp maximum. Minimum is cheap but lets a busy collapsed repo with a capped, empty-looking window promote on the least evidence; maximum is safer but a repo whose cap is always hit may never accumulate the sample. Which is right, and should "unknown" be an explicit signal rather than a fake count? (Applies to both the jobs cap and the run-list cap.)
2. #1249 (item 14) — triage for "pair pending, source tag unresolvable, run history unreadable". Today PRE_EXISTING; options are - (indeterminate, my recommendation: a tag gap says nothing about whether the reusable changed), PRE_EXISTING, or REGRESSION (fails closed hardest, but escalates an expected data gap to needs-human). sync-issues and promote --allow-pre-existing currently rank - and PRE_EXISTING the same. Is - structurally sound, and does adding a fourth triage value need an ADR?
3. #1247 decision 1 — CANARY_INGRESS_JOBS_MAX (default 300). Each ingress run costs one gh run view --json jobs, so this is rate-limit headroom against fleet-wide freezes (all roles share the ingress). Keep 300, raise it, or replace the raw run count with a per-role attributed-sample bound?
4. #1247 decision 2 — collapsed member that does not carry a registered role. Today UNRESOLVED, which holds the gate until the member carries the job or leaves the ring. Should a legitimate non-consumer instead read as "no caller"? Note the #1276 lint only checks the registry against the canonical template, not each member's live ingress, so it does not separate a non-consumer from template drift. What is the right structural way to tell them apart (explicit opt-out/exemption in the registry, or something derived)?
5. Truncated legacy run lists (new). The ingress path already refuses to trust a capped list unless it reaches back before the window start. The legacy per-role path (gh run list -L 1000, now date-bounded by #1282) silently accepts a list that hit 1000 even if its oldest run is newer than the window start, so the front of a 14-day baseline can be cut on a busy repo. Should the legacy reader fail closed (UNRESOLVED) the same way, and is it acceptable for that to hold promotions on busy repos until the cap is addressed?
Items 6 and 7 stay on hold. This is advisory input only; no code changes are being made from this comment.
Run in-session by Claude Code adopting the Solution Architect persona (prompts/solution-architect/advisory.md, bmad-agent-architect v6.8.0), because the mention above routes nowhere: this repo has no workflow calling persona-mention-reusable.yml. Advisory only. No code, registry or label was changed. A second, independent read (a fresh agent given the same persona files, ADRs, code and questions, but not these conclusions) agreed on 2, 4 and 5 and differed on 1 and 3. Both positions are shown there.
ADR scope. The ADR log (0000 to 0012) is recorded in .github-private; this repo has none of its own. Where I cite ADR-0004 below it is by adoption (the gate already follows its pure-core plus bats split), not because the record names this repo. ADR-0007 and ADR-0010 govern the ingress contract the gate reads, so they apply directly.
Measured first (2026-10-11, about 14:30Z)
Four facts change the questions as posed.
#
Fact
Evidence
F1
The legacy per-role run list covers less than a day on the busiest workflows, not most of a 14-day window.
Newest 1,000 runs reach back to 01:09Z today for .github-private Dev-Lead Agent, 23:35Z yesterday for PR Review Agent, 20:34Z yesterday for .github Dev-Lead Agent, 20:04Z yesterday for .github-private CI Failure Analyst.
F2
Because unseen days are zero-filled, the busiest agents are already sized at the clamp minimum.
robust_sample_target 250 3 15 3 437 0 0 0 0 0 0 0 0 0 0 0 0 0 returns 3. The same day alone returns 15.
F3
Four open blockers are question 4 in production.
#1042 apply-repo-settings, #828 initiative-planner, #827 idea-triage, #826 idea-enhancer, all next->ring0 UNRESOLVED with "registers no ingress_job".
F4
The ingress attribution path (_ingress_agent_runs, the 300 cap, the truncated-baseline floor) is not exercised in production today.
markets is the only collapsed repo. It is a named ring member for one agent only (apply-repo-settings, ring1), which is not an ingress role. Everywhere else it is under *, which the gate does not read.
Behind F3: .github-private has an "Agent Ingress" workflow record (created 2026-09-13 from a PR branch) with no agent-ingress.yml on the default branch and no runs in 15 days, so the gate treats it as collapsed. On markets, "Apply repo settings" is disabled_manually, and gh run list --workflow does not resolve a disabled workflow without --all, so it reads as absent.
Suggested order: 5, 4, 2, then 3 and 1 together before any named ring member is collapsed.
5. Truncated legacy run lists
Recommendation. Yes, hold the legacy reader to the same completeness rule, but not as a bare flip. For gating windows, read failure-class runs with a server-side status filter and treat the executed count from the general list as a lower bound. Mark a member UNRESOLVED only if a failure-class list itself reaches the cap before the window start. For the baseline, stop zero-filling days the list did not reach.
Governing ADR. None on record for run-list completeness. Nearest is ADR-0004 ("Fail Loud, Never Fake": emit an indeterminate result, never a fabricated one), by adoption.
Trade-off. A bare fail-closed is about ten lines, and per F1 it would mark UNRESOLVED nearly every pair whose window exceeds half a day on .github-private and .github. That is a busy repo reading as a failure, which is not acceptable as the steady state. The status-filtered read costs up to three extra list calls per member and workflow per sweep. Failure lists are small and complete: 139, 397, 21 and 0 failures in 14 days on the four lists in F1. Leaving it as is means a failure since the cut drops out of view once 1,000 newer runs exist, so by code reading (not observed) a REGRESSION hold can clear without a fix.
To implement. A status-filtered read in _cumulative_health; truncation tracking in the legacy branch of _agent_run_json; one pure completeness predicate in scripts/lib/canary-rollout.sh shared with the ingress path, where the same test appears twice (the unbounded fallback in _ingress_runs_fetch and the refusal in _ingress_agent_runs), so this would be the third copy; bats for each.
4. Collapsed member that does not carry a registered role
Recommendation. Decide by role, not by member, and add no opt-out field.
Agent registers an ingress_job: keep UNRESOLVED when a collapsed member's ingress lacks the job. That is template drift.
Governing ADR. ADR-0007. It gives up per-repo, per-role narrowing on purpose ("Per-repo, per-role staging is lost") and keeps Class 2, Class 3 and pull_request_target roles on their own stubs. A registry exemption for an ingress role would re-create the lever ADR-0007 declined, and that needs a new ADR.
2. #1249 triage for a pending pair with an unresolvable source commit
Recommendation.-. It is structurally sound and it is not a fourth value: the triage field already carries - for the tag-lookup hold and the unresolved-cut-date hold, and _blocker_body already renders it as INDETERMINATE.
Governing ADR. No ADR governs the triage vocabulary, and none is needed because no value is added. ADR-0004's principle applies by adoption: PRE_EXISTING on an empty candidate asserts "reusable byte-identical" when nothing was compared.
Trade-off. The claim that both consumers rank the two the same needs a correction. sync-issues does (neither adds needs-human). promote --allow-pre-existing does not in general: PRE_EXISTING is the one triage it can advance. It is harmless here only because _frontier_state_resilient is called by sync-issues alone and promote refuses a - candidate before it reads the triage. - removes the dependence on that ordering. REGRESSION would page a human for a tag gap that clears itself.
Recommendation. Keep 300 and do not raise it. Change what it bounds before a named ring member is collapsed. Two candidate shapes:
Persist job reads across sweeps (keyed by run id and attempt) and let the cap bound uncached reads per sweep. This is the second read's position and the one I now lean to.
Evidence-shaped bound: read every failure-class run, and successful runs only until the role's sample target is met. This was my first position.
Governing ADR. No ADR governs the value. ADR-0007's Consequences name the attribution cost and set no read budget. Either shape is a structural choice (cross-sweep state, or a definition of complete evidence), so record it as a new ADR extending ADR-0007.
Trade-off. Per F4 nothing depends on the cap today, so raising it buys nothing, and it does not scale: one jobs read takes about 0.8 s, and a union ingress on .github-private would exceed any raw cap within hours (F1). A candidate's window only grows, so a cap hit cannot clear until the next cut. markets produced 265 ingress runs in 7.3 days (peak 127 in a day): 163 cancelled, 93 success, 8 skipped, 1 failure. Health must read every non-success run, so the evidence-shaped bound saves only the successful third on that shape. It is also sound only if a successful run cannot hide a failed role job (ADR-0010 keeps continue-on-error out of the ingress schema; the reusable tier is unverified). Persistence adds state to a gate that keeps none today. The state is evidence, not a decision, so a cache miss costs a read and nothing else.
To implement. For persistence: attempt in the run-list fields, the _run_jobs_json cache key, actions/cache for the jobs files in canary-rollout.yml, and _ingress_agent_runs counting cache misses only. The ADR first in either case.
1. Item 16, unknown baseline sizing
Recommendation. Agreed by both reads: make "unknown" an explicit signal and never a count, size from the days actually observed, and never let a truncated read reach the waive_sample_if_no_caller waiver. The two reads split on the case where a truncated read observed nothing:
Clamp maximum (my read). A role that cannot accumulate it gets an explicit sample_min in the registry, which already bypasses the baseline.
Clamp minimum (second read). Keep today's sizing, with the signal made explicit.
Trade-off. Maximum makes an unreachable 15 a permanent SOAKING for a quiet role. SOAKING opens no issue, so the stall is silent until an owner notices and sets sample_min. Minimum leaves the exposure at 3 runs against 15 on a narrow shape (a role busy enough to deserve 15 with none of its runs in the newest reads), while health and dwell still gate. Per F2, sizing from observed days already moves the busy agents to 15 without either constant, which removes most of what the maximum was meant to protect. The choice also matters little while the gating window itself is capped, since a rare role stalls at 3 as well as at 15. That is why 3 and 5 come first. This one is the owner's call; if the maximum is chosen, pair it with something that surfaces a long SOAKING.
To implement._baseline_daily returns a basis (measured, truncated or unknown) with the known-day counts; a small pure function chooses the target; the legacy path gains truncation tracking (shared with 5); bats. Baseline sizing applies only to next->ring0.
Risk tier and escalation. MEDIUM for 3, since it sets a boundary others will build on with no decision on record. LOW for 1, 2, 4 and 5 as changes. Escalate: no. None of these overturns an accepted ADR. F1 and F3 are live behaviour for the owner to weigh, not ADR conflicts.
Advisory only. I comment, I do not change code. Ground: BMAD Method architect (bmad-agent-architect v6.8.0).
Activity
don-petry commentedon Oct 3, 2026
Dev-Lead Implementation Plan — BLOCKED on #1238
Issue: #1244 — canary-rollout: ingress attribution follow-ups from the #1238 review
Blocker
All seven items target code that exists only on PR #1238's branch (
dev-lead/issue-1224-20261002-2328):_ingress_agent_runs, the role-scoped_run_decision_class,_unresolved_evidence,CANARY_INGRESS_JOBS_MAX, and therun-attribution.shreduction. #1238 is still open / REVIEW_REQUIRED.main(2716f0e), which this run's branch is cut from, has none of it.Implementing here would mean merging the whole unreviewed #1238 diff into a second PR against
main. That would duplicate #1238 and could conflict with its review, so no source changes were made in this run.Pre-analysis against the #1238 branch (85ff3d3), for whoever picks this up
_run_decision_classdoes_run_jobs_json … || echo '{}', thendecision_classreturns empty._sample_decision_countssilentlycontinues on an empty class. A sustained jobs-read failure therefore degrades to INSUFFICIENT and is never recorded as UNRESOLVED (fail-open, as the function's own comment says). A failed read is also cached in_RUN_DECISION_CACHE. Suggested fix: return non-zero on a read error, distinct from "no decision step"._sample_decision_countswould then surface it, and_correctness_verdictwould map it to UNRESOLVED or a hold._run_reusable_shakeeps only the firstUses:line per job and requiresUses: $host/$reusable@. Another role's pin to a different reusable can't match. The open case is two roles in one ingress run that call the same reusable at different pins:_run_reusable_shais not role-scoped (no job-name prefix filter), so the other role's SHA would be collected. That is still fail-closed whenever any collected SHA is the candidate's. A test should cover both the different-reusable and the same-reusable cases.Proposed order once #1238 merges (or if this issue is retargeted onto the #1238 branch)
startup_failureattribution._unresolved_evidence..github-private'srun-attribution.sh.Test Command
npx bats tests/canary_rollout.bats+bash .dev-lead/scripts/dev-lead-lint.shNeeds a maintainer decision: merge #1238 first and re-run dev-lead on this issue, or explicitly authorize stacking this work on the #1238 branch.
don-petry commentedon Oct 3, 2026
Update: item 1 (pre-adoption runs) and the job-less cancelled-run part of the
startup_failure/job-less item were fixed in #1238 (8fa31d4) after pr-review rated pre-adoption MAJOR. A no-role run older than the role's first appearance is now ignored; a newer one, or no run carrying the role at all, stays UNRESOLVED. Job-lesscancelledruns are no longer UNATTRIBUTED. Items 2 (startup_failureattributed to every role), 3, 4, 5, 6 and 7 remain open.Generated by Claude Code
don-petry commentedon Oct 3, 2026
Two non-blocking notes from pr-review's approval of #1238 at
8fa31d4, deferred to avoid a re-review for nits:role_oldestset from a job-lessstartup_failure. In_ingress_agent_runs, a job-lessstartup_failurerecord carries no role job but still setsrole_oldest. With a misspelledingress_job, no-role runs older than such a run would read as pre-adoption instead of blind. It does not fail open (thestartup_failurecounts as a failure, and newer no-role runs stay UNRESOLVED), butrole_oldestshould be set only when a role job was actually observed. Overlaps item 2._ingress_noteinstandards/canary-rings.jsonstill says a job-less ingress run is UNRESOLVED. That is no longer true for acancelledrun, and the note does not mention the pre-adoption rule.Generated by Claude Code
don-petry commentedon Oct 3, 2026
Follow-ups carried forward from pr-review's approval of #1238 (merged, 36444b1), all non-blocking:
-L ${CANARY_INGRESS_RUN_LIMIT:-5000}) can silently truncate; the jobs cap fails closed but the list cap does not. Detect a list that hit its limit and treat it like the jobs cap (UNRESOLVED or truncation-aware baseline).cancelledbefore the UNATTRIBUTED branch, so a lagging jobs API drops it rather than flagging it. Distinguish "cancelled before any job started" from "jobs not recorded yet"._pair_state(BLOCKED 0 0 0 0 0 0 0 - -) prints 13 of 18 state-line fields and is on main since before feat: implement issue #1224 — canary-rollout health gate goes blind on an ADR-0007 collapsed repo: it resolves runs by per-role workflow NAME, which the ingress replaces with jobs #1238;cmd_sync_issuesappendsdatagap, so it shifts intodowngrade. Pad to the full 18 fields (same fix feat: implement issue #1224 — canary-rollout health gate goes blind on an ADR-0007 collapsed repo: it resolves runs by per-role workflow NAME, which the ingress replaces with jobs #1238 applied to theFLAG_ERRORrow) and cover it with a test.Status: #1224 closed by #1238. #1243 (pure-core pair verdict, closes #1242) has main merged in and is under review.
Generated by Claude Code
don-petry commentedon Oct 3, 2026
Item 14 (policy, from #1243 review): In
_frontier_state_resilient's data-gap path, an unresolvable (empty) source commit is classifiedPRE_EXISTINGon main (_reusable_differsreturns 0 when the candidate is empty). Reviewers disagreed on the right triage: CodeAnt wanted it not to suppress regression triage, cubic objected toREGRESSIONescalating a tag-gap hold toneeds-human, and an intermediate-(indeterminate) was tried and then reverted because it is a behaviour change outside a pure refactor. Decide the intended triage for "pair pending, source tag unresolvable, run history unreadable" (likely-/indeterminate, since a tag gap says nothing about the reusable), implement it in its own PR with a test, and confirm how sync-issues ranks it (PRE_EXISTINGand-currently rank the same for needs-human). The #1243 regression test pins today'sPRE_EXISTING, so changing it will flip that test deliberately.Generated by Claude Code
don-petry commentedon Oct 3, 2026
Item 10 (added late; it was referenced from the #1238 review threads but not recorded here): When the per-role workflow is absent, the ingress exists, and no ingress run completed in the window,
_ingress_agent_runsreturns[]without checking that the live ingress declares the registeredingress_job. A misspelled or renamedingress_jobthen looks like "no caller", andwaive_sample_if_no_callercan allow a dwell-only promotion. Impact is limited: the moment any ingress run appears without the role's job it is flagged NOROLE/UNRESOLVED. Both pr-review reviewers and Codex (P1) raised it; it was deferred in #1238 because it needs an authoritative workflow-definition read (the liveagent-ingress.ymljobs), which is a design change. The registry already carries an unusedingress.filethat could drive the check. Options: validate the role job against the live ingress file, warn on "ingress present, zero runs, role never seen", or accept as is.Generated by Claude Code
don-petry commentedon Oct 4, 2026
#1244 is now the tracker. Status of every item after the #1238 / #1243 merges, per maintainer direction:
role_oldestfrom a job-lessstartup_failure_ingress_notedoc driftstartup_failureattributed to every role_ingress_note_unresolved_evidencere-walk, 7 cross-repo parityAlso held here (from the #1243 review, non-blocking):
_frontier_state_resilienthand-checks the unknown flag instead of usingpair_verdict, the empty-vs--normalization is repeated at each call site, and the orchestrator case arms have no direct tests.Sequencing: one PR for #1246 items 3, 11, 12 together with #1248 (item 13), then item 10 once its approach is decided, then #1249 in its own PR (it flips a pinned test on purpose). The
dev-leadlabel is removed from this umbrella so automation does not try to implement all items in one PR.Generated by Claude Code
don-petry commentedon Oct 4, 2026
Item 15 (found in the #1250 review, pre-existing):
_repo_wf_runs_cachedpasses successfulgh run listoutput through without validating it is a JSON array. On the legacy per-role path a non-array body is then reduced byjq … || echo '[]'to[], which reads as "no runs / no caller" (fail-open on malformed data). #1250 fixes this for the ingress path only (non-array → UNRESOLVED). Fix the shared reader instead: have_repo_wf_runs_cachedtreat a non-array success body like a failed fetch (retry, then fail closed with the existing fetch-failure flag), with a test. Low severity (needs a malformed 2xx fromgh), small change.Generated by Claude Code
don-petry commentedon Oct 4, 2026
Item 16 (policy question, raised by cubic on #1250, inherited from #1238): When a capped (truncated) baseline read observes nothing countable for this agent,
_baseline_dailyemits a non-zero floor (1), which sizes the sample target at its clamp minimum (so it can never read as "no caller" and waive sampling, the #1238 fix). cubic argues the fail-closed choice is the clamp maximum instead: an unknown baseline should demand the largest sample, not the smallest. Both fail closed against the waive-sampling hole; they differ in how much evidence a promotion needs when the baseline is unknown.This applies equally to the jobs cap (#1238) and the run-list cap (#1250), so it should be decided once. Needs a maintainer decision; implementation would signal "unknown baseline" from
_baseline_dailyrather than a fake count, and forcesample_clamp_maxin the target computation. Not changed in #1250.Generated by Claude Code
fix(canary-rollout): close ingress fail-open gaps — decision-class re…
don-petry commentedon Oct 4, 2026
Item 17 (found by pr-review on #1250, pre-existing; corrects an overstatement of mine): the gate's timestamp window filters compare ISO strings lexicographically (
createdAt >= since, and #1250 added the same convention to the run-list completeness check), which is only sound when everysinceis Zulu._iso_now_minus_daysand the tag-date path through_to_zare Zulu, butcandidate_cut_date's last-resort local-host fallback returnsgit log -1 --format=%cI "$commit"(scripts/canary-rollout.sh, ~line 537) without_to_z, so it can carry a non-Z offset. I said on #1250 that everysinceis Zulu; that is true except for this fallback.Fix (one line + a test):
_to_z "$(git log -1 --format=%cI "$commit" 2>/dev/null)", falling back to"", so the Zulu assumption holds everywhere. A test should feed a%cIwith a non-Z offset and assert the window filters see the Zulu equivalent. Low severity: needs a cut date resolved from the local checkout with no release tag, and an offset-bearing committer date.Generated by Claude Code
don-petry commentedon Oct 4, 2026
Tracker update after #1250 (PR A) merged:
_to_zthe local-hostcandidate_cut_datefallbackDecisions waiting on the maintainer: #1246 item 10 (approach), item 16 (clamp min vs max), #1249 (item 14 triage), and the #1247 policy questions (job cap value, collapsed-member-lacks-a-role rule). Items 15 and 17 are small and unblocked; they can be bundled into one PR whenever convenient.
Generated by Claude Code
fix(canary): fail closed on a non-array run list; Zulu cut date; cano…
don-petry commentedon Oct 10, 2026
@petry-projects/solution-architect — structural review requested by the owner on the decisions still open in this tracker. Context: the canary-rollout gate (
scripts/canary-rollout.sh) attributes ADR-0007agent-ingress.ymlruns to a role by job key; recent merges are #1250, #1271, #1276 (static registry lint foringress_job, canonicalstandards/workflows/agent-ingress.yml) and #1282 (per-role run list date-bounded to 15 days). The standing principle is "fail closed, but an expected outcome must not read as a failure". For each question please give a recommendation, the governing ADR if one applies (ADR-0007, ADR-0010, others), and say plainly when no ADR governs.1. Item 16 — unknown baseline sizing. When a capped (truncated) baseline read observes nothing countable for an agent,
_baseline_dailyemits a floor of 1, which sizes the required promotion sample at its clamp minimum (default 3 executed runs). Alternative: signal "unknown baseline" explicitly and force the clamp maximum. Minimum is cheap but lets a busy collapsed repo with a capped, empty-looking window promote on the least evidence; maximum is safer but a repo whose cap is always hit may never accumulate the sample. Which is right, and should "unknown" be an explicit signal rather than a fake count? (Applies to both the jobs cap and the run-list cap.)2. #1249 (item 14) — triage for "pair pending, source tag unresolvable, run history unreadable". Today
PRE_EXISTING; options are-(indeterminate, my recommendation: a tag gap says nothing about whether the reusable changed),PRE_EXISTING, orREGRESSION(fails closed hardest, but escalates an expected data gap toneeds-human).sync-issuesandpromote --allow-pre-existingcurrently rank-andPRE_EXISTINGthe same. Is-structurally sound, and does adding a fourth triage value need an ADR?3. #1247 decision 1 —
CANARY_INGRESS_JOBS_MAX(default 300). Each ingress run costs onegh run view --json jobs, so this is rate-limit headroom against fleet-wide freezes (all roles share the ingress). Keep 300, raise it, or replace the raw run count with a per-role attributed-sample bound?4. #1247 decision 2 — collapsed member that does not carry a registered role. Today UNRESOLVED, which holds the gate until the member carries the job or leaves the ring. Should a legitimate non-consumer instead read as "no caller"? Note the #1276 lint only checks the registry against the canonical template, not each member's live ingress, so it does not separate a non-consumer from template drift. What is the right structural way to tell them apart (explicit opt-out/exemption in the registry, or something derived)?
5. Truncated legacy run lists (new). The ingress path already refuses to trust a capped list unless it reaches back before the window start. The legacy per-role path (
gh run list -L 1000, now date-bounded by #1282) silently accepts a list that hit 1000 even if its oldest run is newer than the window start, so the front of a 14-day baseline can be cut on a busy repo. Should the legacy reader fail closed (UNRESOLVED) the same way, and is it acceptable for that to hold promotions on busy repos until the cap is addressed?Items 6 and 7 stay on hold. This is advisory input only; no code changes are being made from this comment.
Generated by Claude Code
don-petry commentedon Oct 11, 2026
Solution-architect advisory (run in-session)
Run in-session by Claude Code adopting the Solution Architect persona (
prompts/solution-architect/advisory.md,bmad-agent-architectv6.8.0), because the mention above routes nowhere: this repo has no workflow callingpersona-mention-reusable.yml. Advisory only. No code, registry or label was changed. A second, independent read (a fresh agent given the same persona files, ADRs, code and questions, but not these conclusions) agreed on 2, 4 and 5 and differed on 1 and 3. Both positions are shown there.ADR scope. The ADR log (0000 to 0012) is recorded in
.github-private; this repo has none of its own. Where I cite ADR-0004 below it is by adoption (the gate already follows its pure-core plus bats split), not because the record names this repo. ADR-0007 and ADR-0010 govern the ingress contract the gate reads, so they apply directly.Measured first (2026-10-11, about 14:30Z)
Four facts change the questions as posed.
.github-privateDev-Lead Agent, 23:35Z yesterday for PR Review Agent, 20:34Z yesterday for.githubDev-Lead Agent, 20:04Z yesterday for.github-privateCI Failure Analyst.robust_sample_target 250 3 15 3 437 0 0 0 0 0 0 0 0 0 0 0 0 0returns 3. The same day alone returns 15.next->ring0UNRESOLVED with "registers no ingress_job"._ingress_agent_runs, the 300 cap, the truncated-baseline floor) is not exercised in production today.marketsis the only collapsed repo. It is a named ring member for one agent only (apply-repo-settings, ring1), which is not an ingress role. Everywhere else it is under*, which the gate does not read.Behind F3:
.github-privatehas an "Agent Ingress" workflow record (created 2026-09-13 from a PR branch) with noagent-ingress.ymlon the default branch and no runs in 15 days, so the gate treats it as collapsed. Onmarkets, "Apply repo settings" isdisabled_manually, andgh run list --workflowdoes not resolve a disabled workflow without--all, so it reads as absent.Suggested order: 5, 4, 2, then 3 and 1 together before any named ring member is collapsed.
5. Truncated legacy run lists
.github-privateand.github. That is a busy repo reading as a failure, which is not acceptable as the steady state. The status-filtered read costs up to three extra list calls per member and workflow per sweep. Failure lists are small and complete: 139, 397, 21 and 0 failures in 14 days on the four lists in F1. Leaving it as is means a failure since the cut drops out of view once 1,000 newer runs exist, so by code reading (not observed) a REGRESSION hold can clear without a fix._cumulative_health; truncation tracking in the legacy branch of_agent_run_json; one pure completeness predicate inscripts/lib/canary-rollout.shshared with the ingress path, where the same test appears twice (the unbounded fallback in_ingress_runs_fetchand the refusal in_ingress_agent_runs), so this would be the third copy; bats for each.4. Collapsed member that does not carry a registered role
ingress_job: keep UNRESOLVED when a collapsed member's ingress lacks the job. That is template drift.ingress_job: it is not an ingress role, so the member's ingress is irrelevant. "No per-role workflow" should read as not a consumer ([], the [Fleet Monitor] petry-projects/.github — .github/workflows/canary-rollout.yml #747 rule), as it does on an uncollapsed repo.pull_request_targetroles on their own stubs. A registry exemption for an ingress role would re-create the lever ADR-0007 declined, and that needs a new ADR.ingress_jobis forgotten in the registry, since it would then read as no caller. Guard against that statically by making the fix(canary): fail closed on a non-array run list; Zulu cut date; canonical agent-ingress.yml + registry lint #1276 lint bidirectional: every job key in the canonical template must be claimed by exactly one agent. Today it only checks registry to template.rc=3branch of_agent_run_json; a reverse-direction lint test; decide "collapsed" from the ingress file on the default branch, not from a workflow record that a PR branch can create; correct the blocker text, which advises registering aningress_jobthat cannot exist; flip the pinned canary-rollout health gate goes blind on an ADR-0007 collapsed repo: it resolves runs by per-role workflow NAME, which the ingress replaces with jobs #1224 test deliberately.2. #1249 triage for a pending pair with an unresolvable source commit
-. It is structurally sound and it is not a fourth value: the triage field already carries-for the tag-lookup hold and the unresolved-cut-date hold, and_blocker_bodyalready renders it as INDETERMINATE.PRE_EXISTINGon an empty candidate asserts "reusable byte-identical" when nothing was compared.sync-issuesdoes (neither addsneeds-human).promote --allow-pre-existingdoes not in general:PRE_EXISTINGis the one triage it can advance. It is harmless here only because_frontier_state_resilientis called bysync-issuesalone andpromoterefuses a-candidate before it reads the triage.-removes the dependence on that ordering.REGRESSIONwould page a human for a tag gap that clears itself._reusable_differsfor an empty candidate in the data-gap branch; print INDETERMINATE in the issue title in place of a bare-; make the banner line "derived from the reusable diff alone" conditional; flip the feat: implement issue #1242 — canary-rollout: move the pair/ring verdict (per-pair state, tag-gap hold) into the pure gate core #1243 test deliberately.3.
CANARY_INGRESS_JOBS_MAX.github-privatewould exceed any raw cap within hours (F1). A candidate's window only grows, so a cap hit cannot clear until the next cut.marketsproduced 265 ingress runs in 7.3 days (peak 127 in a day): 163 cancelled, 93 success, 8 skipped, 1 failure. Health must read every non-success run, so the evidence-shaped bound saves only the successful third on that shape. It is also sound only if a successful run cannot hide a failed role job (ADR-0010 keepscontinue-on-errorout of the ingress schema; the reusable tier is unverified). Persistence adds state to a gate that keeps none today. The state is evidence, not a decision, so a cache miss costs a read and nothing else.attemptin the run-list fields, the_run_jobs_jsoncache key,actions/cachefor the jobs files incanary-rollout.yml, and_ingress_agent_runscounting cache misses only. The ADR first in either case.1. Item 16, unknown baseline sizing
waive_sample_if_no_callerwaiver. The two reads split on the case where a truncated read observed nothing:sample_minin the registry, which already bypasses the baseline.1floor drifts from it.sample_min. Minimum leaves the exposure at 3 runs against 15 on a narrow shape (a role busy enough to deserve 15 with none of its runs in the newest reads), while health and dwell still gate. Per F2, sizing from observed days already moves the busy agents to 15 without either constant, which removes most of what the maximum was meant to protect. The choice also matters little while the gating window itself is capped, since a rare role stalls at 3 as well as at 15. That is why 3 and 5 come first. This one is the owner's call; if the maximum is chosen, pair it with something that surfaces a long SOAKING._baseline_dailyreturns a basis (measured, truncated or unknown) with the known-day counts; a small pure function chooses the target; the legacy path gains truncation tracking (shared with 5); bats. Baseline sizing applies only tonext->ring0.Risk tier and escalation. MEDIUM for 3, since it sets a boundary others will build on with no decision on record. LOW for 1, 2, 4 and 5 as changes. Escalate: no. None of these overturns an accepted ADR. F1 and F3 are live behaviour for the owner to weigh, not ADR conflicts.
Advisory only. I comment, I do not change code. Ground: BMAD Method architect (
bmad-agent-architectv6.8.0).Generated by Claude Code
docs(adr): apply review wording suggestions to ADR-0014
docs(adr): propose ADR-0014 — the canary gate signals incomplete evid…
fix(canary-rollout): a capped per-role run list no longer loses the f…
fix(canary-rollout): an unresolvable source commit in a data gap is I…
fix(canary-rollout): an agent with no ingress_job is not an ingress r…
fix(canary-rollout): an unknown baseline is an explicit signal, not a…