Skip to content

FLEET LINE-STOP: four deploy/host-effect witness identities sit in both floor_route_gap_roster and frozen_path_deferrals; the floor refuses RouteGapFreezeIntersection on main - #9156

Merged
briansrls merged 2 commits into
mainfrom
session/lively-eagle-533
Aug 25, 2026

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session lively-eagle-533.
Pushing to session/lively-eagle-533 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

…verable

The merge-freshness verdict cannot be reconstructed from outside a run:
refs/pull/N/merge is mutable, and recomputation re-points it at current main,
so a stale run compares EQUAL to the target and reads as fresh. This lands the
strictly weaker measurement that CAN be taken, with the unrecoverable case as a
typed, counted refusal rather than a green.

Deliberately NOT an admission verdict, and named for the observation instead:
gunbc.merge_admission still owns that decision over a receipt the run itself
mints. This module mints none and reads none.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review August 25, 2026 01:22
Review 55631 (REQUEST_CHANGES) on #9156, all four verified before acting:

- NoRunForHead was declared and never constructed. Fixed by construction
  rather than deletion: the observer now ACQUIRES the run start itself via
  github.WorkflowRuns.ListForRef, which is exactly where "this head carries no
  run" arises. Taking run_started_at as a parameter had pushed the hardest
  acquisition onto every caller and left the arm unreachable.
- tested_base_provenance (EpochMs) was a second authority over the same fact
  as the Timestamp version the observer actually calls. Deleted.
- The host half had no consumer. Added `main`, the explicit actuator entry:
  exit 0 iff the tested base is the merge target, else exit 1 carrying the
  reason and the target it was computed against. BaseUnknown FAILS here.
  PrBaseFreshnessReport/Row were speculative with no fold; deleted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

All four findings in review 55631 verified against the code before acting. Three are fixed in cfdaa2bb0e; the fourth I am not fixing here, with reasons.

1. NoRunForHead declared and never constructed — confirmed, and fixed by construction rather than deletion.

You were right that no code path produced it, and right about the cause: the observer took run_started_at as a parameter, so the arm described a case the module could never reach. Deleting it would have removed the symptom and kept the real defect — the hardest fact to acquire was being pushed onto every caller, which is also why there was no caller.

The observer now acquires the run start itself via github.WorkflowRuns.ListForRef, and Absent there is exactly "this head carries no run" — the workflow never triggered, the run was collected, the head was force-pushed past. The arm is constructed and reachable.

2. Two parallel provenance predicates — confirmed, deleted.

tested_base_provenance (EpochMs) had zero callers; tested_base_provenance_from_timestamps is what the observer uses. It was residue from before I discovered the timestamps compare correctly as fixed-width ISO 8601 UTC strings, and you are right that it is the §3 duplication this module argues against elsewhere. Gone.

3. The host half was a floating island — confirmed, and now wired.

Added main, the explicit actuator entry DESIGN names, invoked as:

gunbc run --source-root dag --source-root src/v2 \
  --entry dag/gunbc/pr_base_freshness.dag --function main \
  --arg owner=gunb-ai --arg repo=gunbc --arg pull_number=9122 \
  --arg merge_target=$(git rev-parse origin/main)

Exit 0 iff the tested base is the merge target; otherwise exit 1 carrying the reason and the target it was computed against. BaseUnknown fails here — the entry's contract is "this evidence is current", and an unvouchable run does not establish it. That consumes base_freshness_label. PrBaseFreshnessReport / PrBaseFreshnessRow were speculative with no fold behind them, so they are deleted rather than given a trigger.

Verified live, discriminating both ways:

PR 9122 exit=1 :: cannot determine what this run tested — its merge ref was
                  recomputed after the run started ... (merge target 852a7287…)
PR 9156 exit=0 ::

Four REST calls per invocation, acquiring everything from the arguments.

4. auth_token: "" — the finding is real, and it is not in this diff.

Precisely: the empty string is ignored. The service config declares auth_source: EnvVar { name: "GITHUB_TOKEN" }, and the interpreter resolves the credential from there; the input is the corpus idiom, shared with gunbc/tools/review.dag.

The genuine §5 defect you are pointing at is one layer down and I confirmed it: when the env var is unset the interpreter sends no Authorization header and the request proceeds unauthenticated rather than refusing. For this repo ("private": false) an unauthenticated call returns 200, so the degradation is invisible — a private repo would surface as a misleading 404. That is the absorbing pattern, and its home is the REST auth realization in the v1 interpreter plus the shared service config, not these call sites.

I am not fixing it here because it changes behaviour for every REST consumer in the corpus and touches the frozen seed — a corpus-wide auth change smuggled into a reporting-command PR is exactly the scope confusion that makes a defect like this hard to review. Worth its own change, and I would rather it be filed than bundled. Happy to raise it if you want it tracked.

I also removed two now-unused imports, and left a note on the workflow_runs import explaining why it must stay: it binds names this module never spells, but importing it is what registers the service, and dropping it fails at run time with unknown service operation rather than at resolve time.

— sent from lively-eagle-533

@briansrls
briansrls merged commit 5966669 into main Aug 25, 2026
1 check passed
@briansrls
briansrls deleted the session/lively-eagle-533 branch August 25, 2026 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant