Let a run say what it did, and why it failed - #151
Merged
Conversation
Two things a run could not tell anybody. RUN-TOPOLOGY describes two layers making commits happen: the prompt instructs, the autosave backstops. The first did not exist -- no prompt told an agent to commit -- so the autosave was the only writer and every branch the factory had pushed carried its constant message. The prompt now asks, and the entrypoint exports SPIRE_SUMMARY so the final checkpoint can use the agent's own line instead of a constant. The init container has always written one JSON line naming why it failed, already scrubbed of the credential, and nothing read it: an init failure reached the operator as exit 1 with the cause left in a container log. The runtime reads it now. Writing the test found that JGitInternalException escaped CloneMain's catch list entirely, so that class of failure wrote no line at all; it is caught, and an unstructured crash falls back to its first log line. Rebasing onto the clone fix also closed an UNVERIFIED entry. The publisher's trunk floor could not be reached end to end while the clone refused first; it can now, so the trunk test asserts PUBLISHER_MISCONFIGURED by name instead of an outer guard. That test had also been passing on a leftover workspace volume from its own previous run, which TestImages.clearUnit now removes before the run.
artyomsv
force-pushed
the
feat/agent-commit-summary
branch
from
September 11, 2026 13:26
eaf28d6 to
37b0f1f
Compare
artyomsv
commented
Sep 11, 2026
Owner
Author
There was a problem hiding this comment.
Reviewed the full diff at 37b0f1f. The two inline findings below were reproduced against the actual entrypoint in a Docker container with a real Git repository.
Validation: RunUnitBuilderTest (24), M0WalkingSkeletonTest (9), and Adr040ExistingBranchTest (5) all passed: 38 tests, zero failures/errors/skips. Ran with --rerun-tasks to rebuild the image from the current entrypoint. The two separate reproduction probes exercise cases those tests do not cover.
The autosave loop tested its stop flag only before sleeping, so a checkpoint falling due while the harness exited committed the final dirty files under the generic message and left the summary checkpoint a clean tree. Harmless while both wrote the same constant; a defect the moment one carries the agent's own words. The flag is re-checked after the sleep. cut -c counts bytes, so shortening a non-English summary could split a UTF-8 character and leave a lone continuation byte in the commit message. The cut now walks left until the following byte is not a continuation byte, and the cap is renamed for what it measures.
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.
Two things a run could not tell anybody: what it did, and why it failed.
1. Every commit the factory has ever made says
autosave: work in progressdocs/factory/RUN-TOPOLOGY.mddescribes two layers:Layer 1 did not exist. No prompt anywhere told an agent to commit — not
FixPrompt, not a build dispatch, nowhere. So the autosave was the only writer, and its message is a constant indeploy/agent/spire-agent-entrypoint.sh. Every branch the factory has pushed carries it:On work a human is asked to review.
Both halves, because one is the instruction and one is the mechanism
The instruction.
RunUnitBuilderappends it to every dispatched prompt: commit as you go with a real message, and at the end write one line describing what changed to the file named bySPIRE_SUMMARY.Appended there rather than in the prompt builders for three reasons. A fix run's prompt comes from
FixPromptbut a build run's is the caller's text verbatim, so there is no other single place that covers both. It is delivery-agnostic — it ridescommand.prompt()whether the arm takes stdin or argv. And it lands after the Kafka hop, so it cannot push a dispatch pastMAX_PROMPT_CHARSand make a record unroutable.The mechanism. The entrypoint exports
SPIRE_SUMMARY— it owns the path, so no literal is duplicated across repositories — pointing at a file beside the prompt, outside the working tree, for the same reason the prompt is kept there: an autosave must never commit it. The final checkpoint uses that line as its commit subject. A mid-run autosave does not: nothing has been summarised yet, and a stale line would describe work that is not in the commit.The summary is model output on its way into a message a person and the next review's model both read, so it is bounded rather than trusted — first line only, control characters removed, trimmed, cut to 72. Empty falls back, because git refuses an empty message and a run that ends by failing to commit is the loss that file exists to prevent.
An image that never exports
SPIRE_SUMMARYgets the fallback. No run fails over it.2. An init failure said
exit 1and nothing elseCloneMainhas always written one JSON line naming the cause, already scrubbed of the git credential byOutcomeWriter:{"event":"failed","cause":"CLONE_FAILED","detail":"CheckoutConflictException: ..."}Nothing read it.
DockerRunRuntimethrewinit container failed with exit 1and the reason stayed in a container log on whichever host ran it. Diagnosing #150 tookdocker logs— on a product whose run screen exists to answer exactly that question.DockerRunRuntimenow reads the exited container's log, and the cause reachesfailure_detailand the run page. The exit code always survives; reading the explanation is best-effort and can never replace a real failure with a reading error.And one failure class was never structured at all
Writing the test found it:
JGitInternalExceptionextendsRuntimeException, notGitAPIException, so it escapedCloneMain's catch list entirely — the report was a stack trace on stderr and no JSON line was written. It is now caught, which is that catch list's own comment working as intended ("a new failure mode surfaces as a crash to be classified").For anything still uncaught, the runtime falls back to the first log line — which for a crash is the exception and its message, every line after it being a stack frame.
Verification
M0WalkingSkeletonTest(5 → 9 tests): the harness's summary becomes the commit subject; no summary still commits with the fallback; a long, multi-line, CR-bearing summary is cut to one 72-char line; an unreachable base commit surfacesCLONE_FAILEDwith its detail and no credential.RunUnitBuilderTest24 tests: the commit instruction reachesSPIRE_PROMPT, and the dispatched prompt still arrives first and unaltered../gradlew testFast testServices— full results in the comments.checkpoint "$(final_message)"fails exactly the two summary tests and leaves the fallback test green.Two traps this change walked into, both now closed
A shell-only edit does not invalidate
:spire-run-worker:test— no Gradle input tracks the entrypoint file, so the first mutation run here "passed" in 3 seconds against a stale image. A change to that file needs--rerun. CI is safe on clean runners; locally this will bite.A test that is meant to fail is not re-runnable by default. A failed unit stays behind on purpose — the orphan watchdog needs it — so its workspace volume survives into the next execution, where the clone meets a directory that is not empty and fails for a different reason.
anInitFailureCarriesTheContainersOwnCausepassed on its first run and failed on the next withJGitInternalException: Destination path "workspace" already exists. It now clears its own unit before launching, so a failing assertion cannot skip the cleanup. Proven by running it twice back to back.That second failure was useful: it is what surfaced the
JGitInternalExceptionescape above, which was a real defect and not a test artefact.3. A closed entry in
docs/UNVERIFIED.md, found on the wayRebasing onto #150 changed what
aRunNamingTheTrunkNeverMovesItproves, and the register entry that went with it is now false.That entry read: "The publisher's trunk floor is not exercised end to end, and a container test cannot reach it." It was right at the time — the run died at the init container before the publisher was consulted, because the old clone could not create a second local
main. DeletingPublisherConfig.looksLikeATrunkleft the test green.#150 replaced that checkout with a branch create and a reset for an unrelated reason. The side effect is that the clone now succeeds and the run reaches the publisher. Measured, not assumed:
The test now asserts that cause by name, so a future change that makes the run die earlier fails it rather than quietly going back to proving nothing. Mutation-verified: disabling
looksLikeATrunk— the mutation that used to survive — now fails the test. The register entry is marked closed with what made it visible.It had also been passing on a leftover workspace volume from its own previous run, which is the same trap as below and is why it kept looking fine after #150.
Not in this change
TestImages.clearUnitis a test-hygiene fix, not a product one. The underlying behaviour — a failed unit stays behind so the orphan watchdog can reach it — is correct and unchanged.