Skip to content

v0.29.0

Choose a tag to compare

@github-actions github-actions released this 30 Jul 22:39
9ea987c

Added

  • /board expert auto -EndToEnd — an autonomous run can now finish what it started, under four
    conditions
    (#530, part of #526). The brake stops being all-or-nothing: a run the owner ordered
    to finish may close code work that carries a real review and recorded tests for the current
    commit. Everything else still stops for him.

    The four conditions, all established at merge time — the first moment the facts exist, since
    the brake marker is written before a line of code is:

    Condition Why it cannot be dropped
    The owner ordered it The permission travels with the instruction, not with a setting written weeks ago
    The change is code-class (#529) What he judges by looking at it stays his, even when he ordered the finish
    A real review exists for this commit (#510) A green check is not a review
    Automated tests ran and left evidence "Testable and untested" is not finished work

    Why four rather than a priority order. Each covers a failure the others cannot see: a
    reviewed, tested change that is a dashboard is still his to approve; an ordered, reviewed,
    code-class change nobody ran is still unverified. Collapsing them into one flag is exactly how a
    control ends up meaning less than it says — the defect already found in the brake (#440), the
    review gate (#510) and the evidence blocks (#479). A refusal names every unmet condition, not
    the first: one reason at a time forces a round trip per condition and misrepresents how far the
    work actually is.

    The tests requirement reads from the contract's dod.tests, never from the run's own opinion
    that its change was untestable — a per-run "no se podía probar" is precisely the self-issued
    excuse this removes. A project with genuinely no automated tests says so once, in writing.

    Failing to establish the facts is not a yes: if the class, the review or the evidence cannot be
    read, the merge is refused with that as the stated reason.

    Found by its own tests while building: the decision function dot-sourced the classifier without
    its guard, so every call also ran a git diff and printed a banner — a decision function with
    a side effect per call, running git while deciding whether a merge is allowed.

    19 decision tests + 5 marker tests, mutation-verified: removing the order condition turns 3 red,
    the work-class condition 6, the review condition 4, and the tests condition 5.

  • The autonomy boundary can finally express the owner's actual rule (#529, part of #526).
    Until now autonomy.irreversible was a flat list of actions — the same for every kind of work —
    so "code the agent closes by itself; anything I can judge by looking at it waits for me" had
    nowhere to live. The boundary is now about what the change produces, not which action performs
    it: a new workClass policy in the contract plus a pure classifier that reads the touched paths.

    Why this framing rather than a stricter action list: asking someone who does not read code to
    approve a diff is not caution, it hands them a decision they have no way to make. Asking them to
    approve a dashboard is the opposite — they are the only one who can look at it and say whether
    it reads right. So reports, pages, themes and images route to the human; scripts, tests, workflows
    and prose stay with the agent.

    Three decisions worth stating, because each is load-bearing:

    • Timing. The brake marker is written at launch, before a line exists, so the class cannot
      be decided there. The contract carries the policy; the classification runs later against the
      paths the run actually touched. Facts first, decision second.
    • One visual file makes the whole change visual. The failures are wildly asymmetric — the
      owner glancing at something routine costs a minute; an agent shipping a report he wanted to see
      costs trust.
    • "I could not tell what changed" is unknown, never code, and unknown goes to the human.
      Conflating "I looked and it was all code" with "I could not look" is precisely the defect this
      repo keeps finding in its own surfaces.

    Globs are compiled to regex rather than handed to -like, which has no ** and whose * crosses
    / — under -like, src/*.css would swallow src/deep/nested/main.css and the classification
    would widen with every subdirectory. A project can declare what is visual for it (a website
    where the posts are the product), replacing the defaults rather than silently merging with them.

    This alone changes no behaviour yet — it adds the vocabulary and the classifier; nothing
    consults them. Said plainly because the alternative is a release note that implies a capability
    the code does not have. merge remains in every default contract's irreversible list, so no run
    can merge regardless of class. Wiring it into the merge decision — against the actual changed
    paths, at merge time, where the facts exist — is #530, and its review requirement is recorded on
    that issue rather than left as a hope.

    36 tests, mutation-verified: classifying nothing as visual turns 8 red, letting unknown pass as
    approved 1, letting a single * cross a directory boundary 1, and restoring the
    TrimStart('./') bug 1 — that last one found by external review: TrimStart takes a character
    set, so it ate the leading dot of .reports/x.md, and a project pattern like .reports/**
    would silently stop matching. A visual change reclassified as code is the one direction that
    costs trust.

Fixed

  • A reviewer that never reviewed no longer reads as approval (#510). On PR #508 the
    claude-review check reported success while the PR ended with zero reviews and zero
    comments — twice, on two runs. The gate then printed the same GATE PASSED it prints for a
    genuinely clean review, with the self-review reminder as a footnote nobody had to act on. This
    landed in the worst possible window: Copilot has been quota-blocked for weeks, so that workflow
    was the only automated reviewer on the repo, and it was passing without reviewing.

    Fixed on both sides:

    • The workflow now fails when it produced no review. Publishing the review is part of the
      task, not a side effect — the job was running, exiting clean and leaving nothing because it had
      neither the instruction nor the tool to comment. It now gets both, and a verification step
      turns the check red when no review and no [abios-review] comment landed.
    • The gate distinguishes "reviewed, found nothing" from "nobody looked." A new exit code
      2 — GATE SIN REVISAR — reports the second case. It is deliberately not 0: any caller
      testing -eq 0 now fails closed, while still telling an unreviewed PR apart from a genuinely
      blocked one (1). Nothing in the repo branched on the gate's exit code programmatically, so this
      changes no existing behaviour silently.
    • A reviewer with no GitHub identity can now be counted. second-opinion (Codex) is the
      reviewer that actually shows up here, and it submits no review object — so to the gate it was
      indistinguishable from nobody. -RecordReview -Reviewer <who> -Summary <what> writes the
      evidence onto the PR itself, where it survives the session.
    • -AllowUnreviewed is the deliberate exception for changes where a review buys nothing (a typo,
      a regenerated file). It prints that nobody read the code rather than implying someone did.

    All evidence is bound to the PR's head commit, and external review found that this was the
    whole ballgame. The first cut counted any review ever left on the PR, which reproduced the
    original defect one level up: approve, push three more commits, and the gate would authorise a
    diff nobody had read on the strength of a review of different code. Now a GitHub review counts
    only for the commit it was performed on, -RecordReview stamps the head SHA into the record, and
    the workflow's verification demands its own marker for this SHA — so a run that publishes
    nothing can no longer coast on an earlier run's comment. When evidence exists but belongs to an
    older commit the gate says so explicitly ("empujaste cambios después de que se revisó") instead of
    reporting a bare zero. The cost is deliberate: a new push invalidates the evidence and someone has
    to look again, which is the correct reading — those commits genuinely have not been reviewed.

    -RecordReview now requires -Summary. Without it, it was a one-flag way to stamp "reviewed"
    on a PR nobody read — the same empty assurance as the original bug, with a different author.
    Having to state what the review found is the cheapest available proof that one happened.

    The root cause, confirmed live while this very PR was being gated (run 30578175712):
    claude-code-action skips itself and exits outcome=success on any PR that edits its own
    workflow file — a GitHub security measure, so a PR cannot rewrite the reviewer that reviews it.
    Correct as a measure; the problem is that the signal it emits is indistinguishable from "reviewed,
    found nothing". This repo edits that file whenever it tunes the review engine, so it is not an
    edge case. The verification step now names this cause in its error output, and the gate stops the
    deadlock it would otherwise create: a failing REVIEWER check asks "was this reviewed?", not
    "does the code work?"
    — so once a real review is on record for the commit, the reviewer's own red
    is no longer a blocker. Narrow on purpose: one non-reviewer failure and it blocks as before, and
    the failing-check list is read from structured data (gh pr checks --json) rather than scraped
    from the printed table. That distinction is load-bearing — the first cut parsed the human-readable
    output, where any failure printed in an unexpected shape would drop out of the list, leave a
    reviewer failure as the only one seen, and wave a genuinely broken build through. If the
    structured read fails, the allowance is simply never offered.

    Found while fixing it, by the tests: the marker check used -like, whose wildcard syntax reads
    the marker's own square brackets as a character class — it threw instead of matching, which would
    have made every external review invisible, i.e. exactly the blindness being removed. 21 tests,
    mutation-verified: treating everything as reviewed turns 4 red, ignoring marked comments 4,
    unbinding reviews from the commit 2, and dropping the fail-closed-without-a-SHA branch 1.

  • The irreversible brake is now a control, not a paragraph (#516, part of #440). /board expert auto printed Brake ARMED while enforcing nothing: the brake lived only as prose in the launch
    briefing, and an observed run merged its own PR to main — closing its epic's sub-issues — while
    every self-report said the brake was on. Instruction alone is what drifted, so the fix is a
    backstop that does not depend on the agent's cooperation:

    • Start-WorktreeSession arms the run by writing .agentic-board/brake-armed.json into the
      launched worktree, carrying that run's own autonomy.irreversible list from the contract (so
      the guard and the briefing can never disagree). If arming fails, the session is not launched
      — an unarmed run that believes it is armed is precisely the #440 failure.
    • A new PreToolUse hook (Brake-PreToolUseHook.ps1) walks up from the session's cwd, finds
      the marker, and denies the tool call before it executes — gh pr merge, Board-Merge.ps1,
      the REST merge endpoints, and the publish/deploy/refresh/delete paths the contract brakes on.
    • Board-Merge.ps1 refuses on its own inside an armed worktree, so the refusal survives a
      session with no hooks installed or a direct call.

    Two design points worth stating, because both are load-bearing:

    A git hook could not have done this. The original suggestion was a pre-push/pre-merge
    hook, but gh pr merge is a server-side API call — no local git operation occurs, so there is
    nothing for a git hook to intercept. The refusal has to happen at the tool layer.

    The classifier's fail direction is inverted from Expert-Autonomy, deliberately. That one
    classifies an action name from a closed vocabulary, so an unknown verb fails safe (stop and ask).
    This one classifies an arbitrary shell command, where the space of harmless commands is unbounded
    — failing safe there would deny everything and the run could not work at all. So it recognizes
    specific dangerous invocations and lets the rest through. Stated plainly rather than papered
    over: this is a backstop against the known irreversible paths, not a sandbox. A novel route to
    the same effect is not caught here; that is what the companion controls (#517 supervisor-side
    detection, #518 auto-clean refusing to destroy the evidence) are for.

    Inside an armed run, any error in the guard denies rather than allows. Outside one — an
    ordinary human session, which has no marker — the hook is silent and never interferes.

    Four bypasses the first cut of this control still had, found by external review and each now
    closed with its own regression test:

    • A -DryRun token anywhere on the command line waved through the whole thing, so
      echo --dry-run; gh pr merge 490 merged. The classifier now splits the command on shell
      separators and judges each segment on its own — a harmless prefix cannot vouch for what
      follows it.
    • Newlines were collapsed into spaces before that split, welding a multi-line script into one
      segment and reopening the same hole. A newline is now an explicit separator, which is what it
      means in a shell.
    • The run could disarm itself: deleting a local file is otherwise allowed, so
      rm .agentic-board/brake-armed.json turned the entire control off in one command. Tampering
      with the marker — deleting it, overwriting it, wiping the state directory, or editing it
      through the file-writing tools — is now refused on its own terms, not gated on the
      contract's list. An emptied irreversible list reads as tampering too, and falls back to the
      full vocabulary rather than to silence.
    • The REST merge endpoint was only recognised behind gh api, leaving the identical request
      open via curl, Invoke-RestMethod, python or node — all holding the same token. It is now
      matched by endpoint, independent of the client.

    Three more the second review round found, after those four were closed:

    • Shell quoting hid the subcommand. gh pr 'merge' 490 did not match, and merged. Quote
      characters are now removed during normalization, the way the shell removes them.
    • A runtime variable is invisible to any string matching$verb='merge'; gh pr $verb 490.
      Rather than pretend otherwise, a high-risk client reaching for an unresolvable subcommand
      (gh pr $x, gh $x) is now refused: an autonomous run has no legitimate need to call gh
      through a variable, and refusing is the safe side of a call the guard genuinely cannot make.
    • The hook failed OPEN if its own guard failed to load. The armed flag was set only after
      dot-sourcing Brake-Guard.ps1, so a broken guard left the error handler believing the run was
      unarmed — silently restoring the exact capability the control exists to remove. A
      dependency-free probe now establishes armed-or-not first, and everything after it can fail
      without changing which way it fails.

    Arming also disarms: a worktree reused from an earlier braked run kept its marker, so the
    hook went on refusing merges for a run whose contract no longer braked on them, while the
    launcher printed Brake OFF. The marker now follows the contract in both directions — which is
    what makes that message true rather than another claim the code does not honour.

    Two more from the third round:

    • A line continuation is one command, not two. gh pr \merge 490 runs as a single
      command in the shell, but the newline-as-separator rule split it into two harmless-looking
      halves. Continuations (backslash for sh, backtick for PowerShell) are now joined before
      newlines become separators.
    • MultiEdit was missing from the hook matcher, leaving one uncovered write path to the
      marker. Listing a tool this harness may not expose costs nothing; omitting one it does expose
      costs the whole control.

    Three more from the fourth round:

    • A preview claim is now only honoured from something that can preview. The dry-run
      exemption skipped a whole segment on the token alone, so
      curl -H "X-Test: --dry-run" -X PUT .../pulls/12/merge was allowed — a header does not make a
      merge stop mutating. The exemption is scoped to commands that actually have a preview mode.
    • gh api deletes were caught in one spelling only. --method=DELETE and -X DELETE issue
      the identical request and went through.
    • git push origin :branch deletes a remote branch through syntax the --delete pattern
      never saw. (HEAD:main, an ordinary push refspec, stays allowed.)

    97 tests, and every protection was verified by reintroducing the exact defect it prevents rather
    than by trusting a green suite: stubbing the classifier turns 16 red, a global dry-run exemption
    3, an unprotected marker 6, a gh api-anchored REST pattern 4, no quote-stripping 3, no
    variable-indirection pattern 2, joining no line continuations 3, an unscoped preview exemption 2,
    one delete spelling 2, no refspec deletion 1, and deciding the armed flag after the dot-source 1.

    The review ran in rounds until one came back empty. Rounds 1–4 found 12 real defects — every
    one of them in code whose tests were already green, and four of them in the fix for the round
    before. That is the honest cost of a control this size, and the reason the companion issues
    (#517 supervisor-side detection, #518 auto-clean refusing to destroy evidence) exist: this is a
    backstop against known irreversible paths, not a sandbox.