Repository navigation
fix(engine): harden integration, recorded diffs, and step comments - #43
Merged
Merged
Conversation
- run status --json carries an unexpanded block: each bound issue that holds no step, with its unmet depends_on chains and the status of each blocker. - The text view renders the same block, so a close-out question can state roster coverage instead of diffing the issue list against the step list. - The key is absent when every bound issue holds a step.
- A claim refused for class headroom or scope overlap now names the holding steps and their status in the message and in the Error. - The claim-refused event carries the same holders, so a conductor releases a held lane when its holder reaches a state that frees it instead of relaunching against a holder that is still gated.
- dispatch open lists a gap-only park as retry_eligible once every depends_on prerequisite of its issue is done, and records one park-retry-eligible event per park (keyed on step and row version). - run status lists the same parks without writing. - A conductor that lost its parks to a context reset no longer holds them until an operator asks.
- run hold-issue parks all pending steps of one issue, ready and staged, with the holder's reason; step hold now accepts a staged step too. - dispatch open lists held steps with their reasons, so a withheld row is visibly held and the conductor keeps no exclusion filter of its own. - Release is step resolve --as retry; in-flight steps refuse the hold.
- step annotate --integrated-sha now refuses, recording nothing, a sha whose line holds no patch-equivalent of some commit of the writer's chain, as the pre-pick HEAD did when a conflicted cherry-pick was skipped. - The issue.diff re-pin covers the whole integrated range instead of the last commit, so a multi-commit writer's reviewers see every file. - The record carries the writer chain, so a later annotation judges the same chain.
- comment add with a blank -m is a validation error instead of falling through to the editor. - The editor opens only when stdin and stdout are terminals; a script that produced an empty body no longer hangs on a headless editor. - Issue and doc comments share one helper.
- The packet header gains a round line holding the step ordinal, so a contract that asks a worker to name the round no longer leaves it to parse the instance suffix.
- The packet replaces a generated file's diff hunks with a stat line and the blob hash; a file is generated by its "Code generated ... DO NOT EDIT" header or a linguist-generated attribute in the checkout. - The recorded diff artifact and the step context stay whole, so only the render shrinks. A change that regenerated protobuf output no longer pushes its review packet past the Workflow script cap.
- A vote sealed during the dispatch with an approve-with-concerns cast and no linked issue labelled tribunal now refuses the close, naming the vote and its seats. - close --no-conditions REASON waives the check. The votes and the reason ride on the close event and the reason is recorded as a run note; a bound run needs the conductor token, as with the integration override.
…en dispatch - run refresh-scope, refresh-files and refresh-body no longer refuse run-wide while a dispatch is open. The per-issue rule stays: a claimed, running or gated step of the issue still refuses. - The refresh event names the dispatch and the issue's manifest rows that now render the amended packet; the open dispatch still verifies.
…an AC - A gap header line "AC-Blocking: true" on a step that also recorded a non-empty emit routes the completion waiting-human with the new partial-with-gap park class, before any review row is offered. - A gap without the header, or marked false, still completes done.
- the generated marker must be Go's whole-line form, in the first hunk of a hunk that starts at line 1, on the file as it stood before the change; a file the change creates may carry it on an added line. A hand-written file can no longer be hidden from a reviewer by adding or quoting the phrase - the out-of-scope and round-delta heading lines that ride at the end of the last in-scope section survive the elision, so the hunks after them are not read as in-scope
… closes - the waiver's run note is written after the close commits, so a reason the note would refuse left the dispatch closed and the caller with an error - the note text is now built once and its size checked before the close
- a regenerated file's diff starts its first hunk deep in the file, so the header never appears in it and the hunks stayed inline - the header is now also read from the first line of the old blob the diff's index line names, in the run's checkout; a writer cannot add a line to a blob that already exists, and an unreadable blob marks nothing
…is open - the offer already withholds a re-review row behind an open loop body, but a claim that names the row directly skipped that check, so a review, verify or vote could judge the tree before the fix and park a false loop-stalled - the claim now asks the same predicate the offer does and refuses with the after-predecessor condition, naming the open body
…abel - run activate --dry-run lists bound issues without the security label whose scope overlaps a labelled issue's, or whose title, scope or body matches the configured lexicon, as routing_warnings - routing.security_label (default security) names the label; empty turns the lint off. routing.security_lexicon ships empty - the lint only warns: it changes no routing and a real activation writes nothing for it
…t's limits - a dry run without --json printed nothing for the security-routing lint - the label's config doc no longer claims the engine routes on it - the lexicon doc states which scopes the overlap check skips, so an empty list is not read as a clearance
- step complete --comment-file records an issue-comment artifact and the engine posts it on the step's own issue, authored by the step, in the same transaction as the recording - the comment is keyed by its artifact, so it posts once; a second recording of the step is refused by the retired lease - no target override: at most 4 comments of 16 KiB per step, none empty - db.CreateCommentIdempotentTx lets a caller insert inside its transaction
- the step-comment idempotency key shares its scope with `issue comment add --idempotency-key`, so a pre-seeded key made the recording succeed with the artifact stored and the comment skipped - a key already recorded now refuses the whole recording - the help text states the cap is per recording, that a retried step records and posts again, and that the comment stands even if a gate later fails
- the step-comment key lived in the issue-comment scope, where `issue comment add --idempotency-key` could squat on a predictable artifact id and make every recording that carries a comment refuse - the key now lives in its own scope that no verb takes a key for - the issue-comment artifact kind is reserved to the engine
- a worktree-recorded step captures its writer's head and fork point at record time as a recorded-head artifact, while the worktree still exists - the completion gates measure a detached checkout of that sha and remove it when the stage settles, so a workflow that removes the writer's worktree after the record no longer leaves the gates a missing tree - a writer that committed nothing, or a sha that cannot be checked out, keeps the recorded worktree as the tree - the gate runner takes the run's recorded exec root for containment and the default cwd instead of the invoking checkout's, so a detached run launched from a removed worktree no longer refuses with an unresolvable repo path
- exec.Resolve tags the failure to resolve the repo root as ErrEnvironment beside ErrRefused; a binary inside the repo, a name resolved against the cwd, a missing binary and a pin mismatch stay plain refusals - the gate runner records an environment refusal as a skipped row with an environment reason, so a vanished checkout parks as an unmeasured gate instead of a gate-unmatched park reserved to the operator - unmatched still means no trust entry matched
- a head is recorded only for a clean worktree with a commit of its own off the shared checkout; uncommitted or untracked work keeps the live worktree as the tree the gates measure - the gates measure the recorded sha only while the worktree still holds it; a worktree that moved on since the record is measured live, so gates that passed one commit never stand for another - the gate runner also refuses a binary inside its own repository or the tree it measures, not only the run's root - the recorded-head artifact kind is reserved to the engine
…ntainment root - the diff stage reads the commit the step recorded, in a checkout of it against the fork point recorded with it, so the commit the gates measure is the commit the diff names and close integrates; a worktree that moves on or vanishes after the record changes neither - an operator re-pin records the commit its checkout holds, or clears the record when none reproduces it, so a rerun of the gates measures the tree the operator just named - the writer's recorded worktree stays a containment root when the gates measure an engine-owned checkout - the clean check runs git without fsmonitor or an index lock
…rded - a worktree that cannot be replaced by a checkout (untracked state, no commit of its own, a different repository) records a live head that pins its commit: the gates and the diff read the worktree only while it still stands at that head, and record a skipped gate or refuse the diff otherwise - a recorded commit the object database no longer holds records a skipped gate and refuses the diff stage instead of measuring whatever the worktree holds now - every worktree recording writes its record, cleared when no head resolves, so a re-claimed step does not keep the first attempt's head - the fork point is taken from the recorded head, and an operator re-pin keeps its captured head only when the diff named the same commit
- a recorded head for a worktree that cannot be replaced by a checkout now also records a tree object of the worktree's whole content; the gates (before and after running) and the diff compare the live content against it, so uncommitted edits made at the same HEAD after the record are refused like a moved HEAD - a worktree whose content cannot be snapshotted records no head and keeps the live-tree behavior
…HEAD - a diff stage that cannot read the recorded commit now parks the step for the operator; before, the error left the step at the routing stage, which `next` re-ran on every call and `step resolve` refused, wedging the run - the stranded-saga sweep keeps resuming the other steps when one fails - a writer that committed nothing and left a clean worktree is bound to a checkout of its base instead of being measured live - drop the tree snapshot: a live record pins HEAD only, and the residual (uncommitted content edited at the same HEAD) is pinned by a test
- Indent every body line under the comment header in the styled view and collapse wide space runs that wrap a line onto a blank-looking row. - The held cluster reader skips engine-recorded artifact kinds, like the held payload resolver. - Read --comment-file and --gap-file paths as given, without CSV splitting.
- A recorded head shaped like a commit id but naming no object is refused and nothing lands or is annotated.
- The reviewer step is forced done so the refusal comes from the open fix body, for a pending and for a gated fix.
- A recorded head marked live names work its commit does not carry, so step integrate leaves it to be done by hand. - The engine git environments drop GIT_CONFIG like the other config injection variables.
- A park is signalled when the edge or the prerequisite moved after it. - A prerequisite written as blocks counts; the reverse direction does not. - Doc comments now state the moved-after-park condition and blocks.
…ories - A widened scope makes verify report the issue's rows missing while any other issue's claimed or running step overlaps it, in the manifest or not. - The scope and body advisories no longer say a refresh refuses while a dispatch is open.
- The recorded issue.diff producers pass --text so a committed attribute cannot turn a changed file into "Binary files differ".
- Normalize each body once per issue, not once per term. - A term of only separators matches nothing instead of every issue. - The lexicon doc names what is and is not normalized.
- Plain and colorless comment rows are wrapped under the header indent so a terminal wrap never puts body text at the header column. - The space collapse that rewrote tables and indented code is gone.
- An emoji with a presentation selector or a keycap sequence counts as wide as a terminal draws it, so it cannot push text to a wrapped row.
- A re-pin whose tree moved records no head, so nothing says the worktree held no uncommitted work; step integrate now refuses it like a live one. - The annotation re-pin patch is tested to show attribute-marked files.
- Reading every file as text recorded a genuine binary byte for byte. - A section over 64 KiB keeps its header, whose index line names the blobs, and drops the hunks; smaller diffs are unchanged.
- The cap marker carries the file's added and removed line counts, and the diff facts read them, so a padded file cannot measure as small. - Recorded diffs use full-length blob ids, which repository config cannot shorten. - Two comments no longer claim the recorded artifact is the whole diff.
- A text section stays whole however long, so a writer cannot hide an edit by padding a file; an oversize packet is refused, not blinded. - A section over 64 KiB that holds a NUL byte keeps its header and blob ids and drops the hunks.
- The binary probe reads the same 8000-byte window git does, so a text file whose only NUL lies later is still recorded whole.
- A NUL is read as binary only in the first hunk, and only when it starts at line 1 on both sides, so it lies inside the window git reads. - A change below the top, or a NUL in a later hunk, stays recorded whole.
- Generated-file elision no longer trusts anything a writer controls: a header counts only when the file carries it both before and after the change (a created file is elided by attribute alone, and removing the header keeps the file in the packet), and .gitattributes is read from the run's pinned commit instead of the live checkout. - .gitattributes patterns support `**` and anchored `/` patterns. - The elided stat line counts content lines only, so `--- x` / `+++i` content is no longer dropped from the counts. - Blob headers are read through one `git cat-file --batch` per render, and untrackedDiff lists config hooks once for all its per-file diffs. - `step integrate` refuses chains that share commits before the first pick, and refuses a shallow shared checkout as it does a grafted one; the duplicate checkout check per pick is gone. - A probe that cannot answer in a partial clone now says so and how to fetch the missing objects (engine git still never fetches). - A headroom refusal names unacknowledged reaps as holders. - Comments with CRLF line endings are accepted. - Retry-eligible parks use when a prerequisite became done (from the activity log) rather than its last edit, and treat the park's own second as after it. - Drop the CLI env scrub that integrateGitEnv already covers.
erikreinert
pushed a commit
that referenced
this pull request
Oct 22, 2026
- Generated-file elision no longer trusts anything a writer controls: a header counts only when the file carries it both before and after the change (a created file is elided by attribute alone, and removing the header keeps the file in the packet), and .gitattributes is read from the run's pinned commit instead of the live checkout. - .gitattributes patterns support `**` and anchored `/` patterns. - The elided stat line counts content lines only, so `--- x` / `+++i` content is no longer dropped from the counts. - Blob headers are read through one `git cat-file --batch` per render, and untrackedDiff lists config hooks once for all its per-file diffs. - `step integrate` refuses chains that share commits before the first pick, and refuses a shallow shared checkout as it does a grafted one; the duplicate checkout check per pick is gone. - A probe that cannot answer in a partial clone now says so and how to fetch the missing objects (engine git still never fetches). - A headroom refusal names unacknowledged reaps as holders. - Comments with CRLF line endings are accepted. - Retry-eligible parks use when a prerequisite became done (from the activity log) rather than its last edit, and treat the park's own second as after it. - Drop the CLI env scrub that integrateGitEnv already covers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016JiztiErrsSb4v8FyuEFbj
- Escape the zero-width, joiner and bidi-override characters in test string literals (ST1018). - Drop the never-read reassignment of otherSHA in the GIT_DIR test (SA4006); the extra commit alone keeps the two heads distinct.
Since 5960cea `step complete` records the artifact, starts a hidden `step resume` child for the gates, routing and run lifecycles, and returns at once. The QA suite still read the step right after the verb, so ZG13, ZG16, ZG17 and ZG20 saw `gated|recorded` before any gate ran, on main as well as on this branch. run and run_env now wait, after a `step complete`, until no `step resume` child is left (bounded by QA_DETACHED_WAIT_S, default 30s). The child is started before the verb returns and exits once its work is done, so its exit is the signal.
erikreinert
pushed a commit
that referenced
this pull request
Oct 22, 2026
- Generated-file elision no longer trusts anything a writer controls: a header counts only when the file carries it both before and after the change (a created file is elided by attribute alone, and removing the header keeps the file in the packet), and .gitattributes is read from the run's pinned commit instead of the live checkout. - .gitattributes patterns support `**` and anchored `/` patterns. - The elided stat line counts content lines only, so `--- x` / `+++i` content is no longer dropped from the counts. - Blob headers are read through one `git cat-file --batch` per render, and untrackedDiff lists config hooks once for all its per-file diffs. - `step integrate` refuses chains that share commits before the first pick, and refuses a shallow shared checkout as it does a grafted one; the duplicate checkout check per pick is gone. - A probe that cannot answer in a partial clone now says so and how to fetch the missing objects (engine git still never fetches). - A headroom refusal names unacknowledged reaps as holders. - Comments with CRLF line endings are accepted. - Retry-eligible parks use when a prerequisite became done (from the activity log) rather than its last edit, and treat the park's own second as after it. - Drop the CLI env scrub that integrateGitEnv already covers.
erikreinert
force-pushed
the
fix/run-postmordem
branch
2 times, most recently
from
October 22, 2026 19:58
c502fca to
82e5959
Compare
erikreinert
marked this pull request as ready for review
October 22, 2026 23:02
erikreinert
added this pull request to stack #45
October 23, 2026 00:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the gaps found auditing a finished run: step integration, recorded diffs, completion gates, and step comments now bind to the exact commit measured, refuse inputs they cannot trust, and keep operator-facing output bounded.
Changes
step integrate, which cherry-picks and annotates finished writers; it refuses a live record, a non-full commit id, a grafted or replace-ref checkout, a base-less chain, and a hand-resolved pick that touches files beyond the writer's.GIT_CONFIG, fsmonitor, and protocol overrides from engine git.rivo/unisegas a direct dependency).holdfor every pending step of an issue, list unexpanded roster issues in run status, let an issue with nothing in flight refresh under an open dispatch, and state the loop round in the work packet.Testing
go build ./...passes.go test ./internal/...passes in every package at the branch head.Notes