From d2387634cab2d0444bfd3db4aaa0917c38638d0e Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:15:44 +0100 Subject: [PATCH 01/10] chore: record the established cause of the sub-agent stage loss Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- ...610181606223044-a-sub-agent-stage-retries-a-transient.md | 6 ++++++ ...610051434156079-a-burst-of-subagent-stops-still-loses.md | 6 ++++++ 2 files changed, 12 insertions(+) create mode 100644 .abcd/work/decisions/dcn-2610181606223044-a-sub-agent-stage-retries-a-transient.md diff --git a/.abcd/work/decisions/dcn-2610181606223044-a-sub-agent-stage-retries-a-transient.md b/.abcd/work/decisions/dcn-2610181606223044-a-sub-agent-stage-retries-a-transient.md new file mode 100644 index 000000000..0b002d2c1 --- /dev/null +++ b/.abcd/work/decisions/dcn-2610181606223044-a-sub-agent-stage-retries-a-transient.md @@ -0,0 +1,6 @@ +--- +id: dcn-2610181606223044 +slug: a-sub-agent-stage-retries-a-transient +date: 2026-10-18 +--- +2026-10-18 — A sub-agent stage retries a transient failure and marks a final one, but queues no unresolved transcript (facilitator-on-delegation, iss-2610051434156079): the hook retries resolving the repository when the payload's cwd carries a .git entry but git gave no root commit, re-reading the session note between tries, since burst siblings write it as they resolve; it retries a stage refused by lock contention, waiting longer as the record's remedy asks; both within a bound well inside the host's hook timeout, saying each retry on stderr. A stage that still fails for a known store leaves a counted marker that abcd history staged reports, so the loss is visible to the person and not only in a debug log. No machine-level queue is added for a transcript whose repository cannot be resolved at all, because it would keep unredacted transcripts in a new location outside any store, and the retries remove the transient case the record names (grounds: the cause established on the record on 2026-10-18; gitutil.RepoShaped; the existing sub-agent gap marker's pattern; principles/loud-staging.md) diff --git a/.abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md b/.abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md index 49e0dd01c..110b92a5e 100644 --- a/.abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md +++ b/.abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md @@ -27,3 +27,9 @@ Deferred past v0.13.1: Second deferral, made by the 2026-10-07 autonomous run cu ## Evidence 2026-10-08 Recurred in the merge queue: PR #869 (records only) was dropped at 06:59Z when its merge group's macOS check failed TestHookSubagentStopConcurrentCompletionsAllStage, staging 11 of 12 transcripts from 12 simultaneous completions (run 37736585477). Re-enqueued; it merged. The deferral past v0.13.1 lapsed at v0.13.2, and the v0.13.3 cut's findings gate did not trip on it because it predates that anchor. + +## Cause established 2026-10-18 + +Reproduced deterministically, so the cause no longer rests on the slow macOS runner alone. The staging lock is not it: since iss-2609090828371674 each agent stages behind its own lock, so the twelve completions of the burst never contend for one, and 25 rounds of the burst under -race here staged every transcript. Slowing all seventeen git calls the hook makes (gittest.SlowPipeGit) still stages, only slowly. + +What drops a transcript is a git call that fails once rather than slowly. `gitutil.RootCommit` reads any git failure other than a timeout as "no root commit", so `resolveSubagentStore` takes the payload's cwd for a directory that is not a repository and falls back to the session note. A burst's first completions run before any sibling has written that note, so the hook prints "cannot resolve the repository for this sub-agent … staging nothing" on stderr, exits 0, and the transcript is gone. A git shim that fails the root-commit lookup once reproduces it: the hook exits 0 and nothing is staged. A loaded runner produces the same failure when a fork or a git process fails, and the burst test never printed the hooks' stderr, which is why the cause went unseen. The other path to the same silent loss is a stage that still fails, such as a lock held past its timeout, with the store known. From 46993aee0afebf3a2c6a57cc0b6e38b48f9dc9bf Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:15:47 +0100 Subject: [PATCH 02/10] fix: retry a sub-agent stage that failed once, and mark one that is lost A git call that fails once, as a refused fork or an early-killed git does on a loaded machine, reads as no root commit. A burst's first sub-agent completions run before any sibling has noted the session, so the hook took a repository-shaped cwd for one outside any repository, staged nothing and exited 0, saying so only on a stderr no one reads. When the cwd carries a .git entry but git gave no root commit, the hook now asks again for a few seconds, re-reading the session note between tries. A stage refused by a lock held past its timeout is tried again. A transcript read and still not staged, with the store known, is counted in a marker that history staged reports. The burst test prints each hook's stderr when it fails, so a loss names its cause. Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- .../brief/04-surfaces/11-history.md | 18 ++ commands/history.md | 6 +- internal/core/history/stagemiss.go | 110 +++++++++ internal/core/history/stagemiss_test.go | 62 +++++ internal/surface/cli/history.go | 14 ++ internal/surface/cli/hook_subagent.go | 74 +++++- .../cli/hook_subagent_stage_retry_test.go | 214 ++++++++++++++++++ .../surface/cli/hook_subagent_stop_test.go | 7 +- 8 files changed, 500 insertions(+), 5 deletions(-) create mode 100644 internal/core/history/stagemiss.go create mode 100644 internal/core/history/stagemiss_test.go create mode 100644 internal/surface/cli/hook_subagent_stage_retry_test.go diff --git a/.abcd/development/brief/04-surfaces/11-history.md b/.abcd/development/brief/04-surfaces/11-history.md index 398ab44be..7c9925b69 100644 --- a/.abcd/development/brief/04-surfaces/11-history.md +++ b/.abcd/development/brief/04-surfaces/11-history.md @@ -288,6 +288,24 @@ a burst of simultaneous sub-agent completions stages in parallel instead of queuing behind one lock whose timeout would refuse the tail of the burst. A lock file is removed with the staged file it guards. +The subagent-stop entrypoint does not drop a transcript on a failure that a +second try would clear (iss-2610051434156079). A git call that fails once, as a +fork refused or a git killed early on a loaded machine does, reads as no root +commit, so a burst's first completions, which run before any sibling has noted +the session, once took a repository-shaped working directory for one outside +any repository and staged nothing. When the payload's working directory +carries a `.git` entry but git gave no root commit, the entrypoint asks again +for a few seconds, re-reading the session note between tries. A stage refused +because its lock was held past the lock's timeout is tried again, up to three +more times. Each retry is said on stderr. A transcript read and still not +staged, with the store known, is counted in a marker beside the staging +directory, and the staged listing reports it: the entrypoint must exit 0, and +its stderr reaches only the host's debug log, so the marker is where the person +sees the loss. A working directory with no `.git` entry, whose repository no +store has seen, is not retried and has no store to mark; no queue holds such a +transcript elsewhere, since that would keep unredacted text outside every +store. + Staging is also **the outcome record the store never had.** Before it, an absent record spanned "never ended", "ended before the store existed" and "ended and lost" alike, and nothing could tell them apart, which is why a week of losses diff --git a/commands/history.md b/commands/history.md index e5e119b8e..bcb419b75 100644 --- a/commands/history.md +++ b/commands/history.md @@ -167,7 +167,11 @@ alike. Report `session_id`, `staged_at` and `bytes`, plus `agent_id` and text** until drained, so say so whenever the list is non-empty. The text render also carries a note when the host has fired a sub-agent stop without handing over a transcript path: on such a host no sub-agent transcript can be captured -at all, so an empty sub-agent corpus is the host and not the sessions. An entry +at all, so an empty sub-agent corpus is the host and not the sessions. It +carries a second note when sub-agent transcripts were read but could not be +staged, with how many, when, and the last one's reason: those transcripts are +not in the store, and the hook that lost them exited 0, so this note is the only +place the loss shows. Tell the user how many were lost and why. An entry past the seven-day limit renders as OVERDUE; a quarantined transcript is listed in its own block, and is not awaiting anything — nothing will retry it. diff --git a/internal/core/history/stagemiss.go b/internal/core/history/stagemiss.go new file mode 100644 index 000000000..bd73f1e80 --- /dev/null +++ b/internal/core/history/stagemiss.go @@ -0,0 +1,110 @@ +package history + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/intentdriven/abcd/internal/fsutil" +) + +// stageMissFilename is the marker, beside the staging directory, that counts +// the sub-agent transcripts a SubagentStop hook read but could not stage +// (iss-2610051434156079). The hook exits 0 whatever happens, since a non-zero +// exit is that event's blocking code, and its stderr reaches only a debug log, +// so without the marker a lost transcript would be seen by no one. +const stageMissFilename = "subagent-stage-misses.json" + +// stageMissLockTimeout bounds the wait for the marker's own lock. A burst of +// completions that all miss writes the marker once each, and the critical +// section is one small read and write. +const stageMissLockTimeout = 5 * time.Second + +// stageMissReasonMax bounds the last reason kept, so the marker stays small +// whatever the failure said. +const stageMissReasonMax = 300 + +// StageMissNote is the marker: how many sub-agent transcripts were read but +// not staged, when the first and the last were, and the last one's agent and +// reason. +type StageMissNote struct { + Schema int `json:"schema"` + FirstSeen time.Time `json:"first_seen"` + LastSeen time.Time `json:"last_seen"` + Count int `json:"count"` + LastAgent string `json:"last_agent"` + LastReason string `json:"last_reason"` +} + +// stageMissPath returns the marker's path beside the staging directory. +func stageMissPath(repoRoot, rootSHA string) (string, error) { + lane, err := laneDir(repoRoot, rootSHA) + if err != nil { + return "", err + } + return filepath.Join(lane, stageMissFilename), nil +} + +// NoteStageMiss records one sub-agent transcript that was read and could not +// be staged, under the marker's own lock so concurrent misses each count. +// agentID and reason are stored as given apart from a bound on the reason; +// every reader sanitises them before printing. +func NoteStageMiss(repoRoot, rootSHA, agentID, reason string) error { + if !rootSHARe.MatchString(rootSHA) { + return errors.New(rootSHAErrMsg) + } + path, err := stageMissPath(repoRoot, rootSHA) + if err != nil { + return err + } + if len(reason) > stageMissReasonMax { + reason = reason[:stageMissReasonMax] + } + reason = strings.ReplaceAll(strings.ReplaceAll(reason, "\r", " "), "\n", " ") + return fsutil.WithFileLock(path+".lock", stageMissLockTimeout, func() error { + now := time.Now().UTC() + note := StageMissNote{Schema: 1, FirstSeen: now} + if prior, ok, err := readStageMiss(path); err == nil && ok { + note.FirstSeen, note.Count = prior.FirstSeen, prior.Count + } + note.LastSeen, note.LastAgent, note.LastReason = now, agentID, reason + note.Count++ + data, err := json.Marshal(note) + if err != nil { + return err + } + return fsutil.WriteFileAtomic(path, append(data, '\n'), 0o600) + }) +} + +// StageMiss reads the marker. A missing marker is not an error: no sub-agent +// transcript has been missed. +func StageMiss(repoRoot, rootSHA string) (StageMissNote, bool, error) { + if !rootSHARe.MatchString(rootSHA) { + return StageMissNote{}, false, errors.New(rootSHAErrMsg) + } + path, err := stageMissPath(repoRoot, rootSHA) + if err != nil { + return StageMissNote{}, false, err + } + return readStageMiss(path) +} + +func readStageMiss(path string) (StageMissNote, bool, error) { + data, err := fsutil.ReadGuarded(path, maxStageSidecarBytes) + if errors.Is(err, os.ErrNotExist) { + return StageMissNote{}, false, nil + } + if err != nil { + return StageMissNote{}, false, err + } + var note StageMissNote + if err := json.Unmarshal(data, ¬e); err != nil { + return StageMissNote{}, false, fmt.Errorf("history: the stage-miss marker does not parse: %v", err) + } + return note, true, nil +} diff --git a/internal/core/history/stagemiss_test.go b/internal/core/history/stagemiss_test.go new file mode 100644 index 000000000..927769c2e --- /dev/null +++ b/internal/core/history/stagemiss_test.go @@ -0,0 +1,62 @@ +package history + +import ( + "strings" + "sync" + "testing" +) + +// TestStageMissCountsEveryConcurrentMiss: a burst of completions that all fail +// to stage writes the marker at once, and the marker's lock keeps every one +// counted (iss-2610051434156079). Without it, two read-modify-writes at once +// would each count from the same prior and one miss would vanish from the +// count. +func TestStageMissCountsEveryConcurrentMiss(t *testing.T) { + repoRoot, _ := setupStore(t) + if _, ok, err := StageMiss(repoRoot, testRootSHA); err != nil || ok { + t.Fatalf("a fresh store reports a miss: ok=%v err=%v", ok, err) + } + const n = 12 + var wg sync.WaitGroup + errs := make([]error, n) + for i := range n { + wg.Add(1) + go func() { + defer wg.Done() + errs[i] = NoteStageMiss(repoRoot, testRootSHA, "agent-"+string(rune('a'+i)), "lock held") + }() + } + wg.Wait() + for i, err := range errs { + if err != nil { + t.Fatalf("miss %d: %v", i, err) + } + } + note, ok, err := StageMiss(repoRoot, testRootSHA) + if err != nil || !ok { + t.Fatalf("marker not readable: ok=%v err=%v", ok, err) + } + if note.Count != n { + t.Errorf("count = %d, want %d: a concurrent miss went uncounted", note.Count, n) + } + if note.FirstSeen.IsZero() || note.LastSeen.Before(note.FirstSeen) { + t.Errorf("first/last seen = %v / %v", note.FirstSeen, note.LastSeen) + } +} + +// TestStageMissKeepsTheReasonOneBoundedLine: the reason is kept on one line +// and bounded, whatever the failure said. +func TestStageMissKeepsTheReasonOneBoundedLine(t *testing.T) { + repoRoot, _ := setupStore(t) + long := "first line\nsecond\r" + strings.Repeat("x", 1000) + if err := NoteStageMiss(repoRoot, testRootSHA, "a1", long); err != nil { + t.Fatal(err) + } + note, _, err := StageMiss(repoRoot, testRootSHA) + if err != nil { + t.Fatal(err) + } + if strings.ContainsAny(note.LastReason, "\r\n") || len(note.LastReason) > stageMissReasonMax { + t.Errorf("reason not one bounded line: %q (%d bytes)", note.LastReason, len(note.LastReason)) + } +} diff --git a/internal/surface/cli/history.go b/internal/surface/cli/history.go index 950148791..2ca5cf976 100644 --- a/internal/surface/cli/history.go +++ b/internal/surface/cli/history.go @@ -251,6 +251,15 @@ func newHistoryCommand(asJSON *bool) *cobra.Command { "abcd history: the sub-agent payload marker is unreadable (%s)\n", termsafe.Sanitize(gapErr.Error())) } + // The stage-miss marker counts sub-agent transcripts a hook read and + // could not stage (iss-2610051434156079): the hook exits 0, so this + // is the only place the person can see such a loss. + miss, hasMiss, missErr := history.StageMiss(repoRoot, rootSHA) + if missErr != nil { + fmt.Fprintf(cmd.ErrOrStderr(), + "abcd history: the sub-agent stage-miss marker is unreadable (%s)\n", + termsafe.Sanitize(missErr.Error())) + } if staged == nil { staged = []history.Staged{} } @@ -274,6 +283,11 @@ func newHistoryCommand(asJSON *bool) *cobra.Command { termsafe.Sanitize(orDefault(gap.Event, "SubagentStop")), gap.Count, gap.FirstSeen.Format("2006-01-02T15:04:05Z")) } + if hasMiss { + fmt.Fprintf(w, "\nNOTE: %d sub-agent transcript(s) were read but could not be staged, so they are not in this store (first %s, last %s, agent %s: %s).\n", + miss.Count, miss.FirstSeen.Format("2006-01-02T15:04:05Z"), miss.LastSeen.Format("2006-01-02T15:04:05Z"), + termsafe.Sanitize(miss.LastAgent), termsafe.Sanitize(fsutil.RedactHome(miss.LastReason))) + } }() if len(staged) == 0 && len(quarantined) == 0 { fmt.Fprintln(w, "abcd history — nothing staged; every ended session is stored") diff --git a/internal/surface/cli/hook_subagent.go b/internal/surface/cli/hook_subagent.go index a94f6d9a3..e31747851 100644 --- a/internal/surface/cli/hook_subagent.go +++ b/internal/surface/cli/hook_subagent.go @@ -25,7 +25,9 @@ package cli // Capture. import ( + "errors" "fmt" + "io" "os" "regexp" "strings" @@ -35,6 +37,8 @@ import ( "github.com/intentdriven/abcd/internal/core/ahoy" "github.com/intentdriven/abcd/internal/core/history" + "github.com/intentdriven/abcd/internal/fsutil" + "github.com/intentdriven/abcd/internal/gitutil" "github.com/intentdriven/abcd/internal/termsafe" ) @@ -74,6 +78,20 @@ const ( subagentSettleInterval = 25 * time.Millisecond ) +// subagentResolveWaits are the pauses before each further attempt to resolve +// the repository when the payload's cwd carries a .git entry but git gave no +// root commit: a git that failed once, which on a loaded machine is a fork +// refused or a git killed early, never a directory outside any repository +// (iss-2610051434156079). About three and a half seconds in all. +var subagentResolveWaits = []time.Duration{100 * time.Millisecond, 400 * time.Millisecond, time.Second, 2 * time.Second} + +// subagentStageRetries bounds how many times a stage refused by lock +// contention is tried again. Each try already waits out the staging lock's own +// timeout (history's five seconds), so three more give a held lock twenty +// seconds in all, well inside the host's hook timeout; past that the +// transcript is marked as missed rather than dropped unseen. +var subagentStageRetries = 3 + // newSubagentStopCommand builds `abcd hook subagent-stop`. func newSubagentStopCommand() *cobra.Command { return &cobra.Command{ @@ -114,6 +132,15 @@ func newSubagentStopCommand() *cobra.Command { return warn("this harness fired %s with no agent_transcript_path, so no sub-agent transcript can be captured here; `abcd history staged` reports this", orDefault(in.Event, "SubagentStop")) } + if rootSHA == "" { + // A directory that carries a .git entry but gave no root + // commit is a git that failed this once (a fork refused on a + // loaded machine, a git that exited early), not a directory + // outside any repository. It is asked again, and the session + // note re-read between tries, since a burst's siblings write it + // as they resolve (iss-2610051434156079). + repoRoot, rootSHA, via = retrySubagentStore(cmd.ErrOrStderr(), in) + } if rootSHA == "" { return warn("cannot resolve the repository for this sub-agent — the payload's cwd %q did not resolve and no store has seen session %q; staging nothing", in.Cwd, in.SessionID) @@ -136,9 +163,15 @@ func newSubagentStopCommand() *cobra.Command { } meta := subagentStageMeta(in) - res, err := history.Stage(repoRoot, rootSHA, meta, raw) + res, err := stageWithRetry(cmd.ErrOrStderr(), repoRoot, rootSHA, meta, raw) if err != nil { - return warn("staging sub-agent %s failed (%v); this transcript was not captured", in.AgentID, err) + // The transcript was read and is lost. The marker is what lets + // the person see that: this hook's stderr reaches only a debug + // log, since it must exit 0. + if nerr := history.NoteStageMiss(repoRoot, rootSHA, in.AgentID, err.Error()); nerr != nil { + diagnosticLine(cmd.ErrOrStderr(), "abcd history: the missed stage could not be recorded either (%v)", nerr) + } + return warn("staging sub-agent %s failed (%v); this transcript was not captured, and `abcd history staged` reports it", in.AgentID, err) } staged := hookStageResult{Hook: "subagent-stop", Captured: true, Outcome: hookOutcomeStaged, SessionID: termsafe.Sanitize(in.SessionID), AgentID: in.AgentID, Bytes: res.Staged.Bytes} @@ -209,6 +242,43 @@ func resolveSubagentStore(in hookInput) (repoRoot, rootSHA, via string) { return "", "", "" } +// retrySubagentStore asks again, after resolveSubagentStore found no store, +// when the payload's cwd carries a .git entry: git failing once is a loaded +// machine, not a directory outside any repository, and a burst's siblings +// write the session note as they resolve, so it is re-read between tries. A +// cwd with no .git entry, or one that is gone, is not retried: there is nothing +// a second git call could find. Each retry is said on stderr. +func retrySubagentStore(w io.Writer, in hookInput) (repoRoot, rootSHA, via string) { + if in.Cwd == "" || !gitutil.RepoShaped(in.Cwd) { + return "", "", "" + } + for i, wait := range subagentResolveWaits { + time.Sleep(wait) + if repoRoot, rootSHA, via = resolveSubagentStore(in); rootSHA != "" { + fmt.Fprintf(w, "abcd history: sub-agent %s: the repository resolved on try %d, after git first gave no root commit for a repository-shaped cwd\n", + termsafe.Sanitize(in.AgentID), i+2) + return repoRoot, rootSHA, via + } + } + fmt.Fprintf(w, "abcd history: sub-agent %s: git gave no root commit for the repository-shaped cwd in %d tries\n", + termsafe.Sanitize(in.AgentID), len(subagentResolveWaits)+1) + return "", "", "" +} + +// stageWithRetry stages, trying again when the staging lock was held past its +// timeout, so a stage that cannot take the lock waits longer instead of losing +// the transcript (iss-2610051434156079). Every other failure returns at once: +// it would fail the same way again. +func stageWithRetry(w io.Writer, repoRoot, rootSHA string, meta history.StageMeta, raw []byte) (history.StageResult, error) { + res, err := history.Stage(repoRoot, rootSHA, meta, raw) + for try := 0; err != nil && errors.Is(err, fsutil.ErrLockContention) && try < subagentStageRetries; try++ { + fmt.Fprintf(w, "abcd history: sub-agent %s: the staging lock was held past its timeout; trying again (%d of %d)\n", + meta.Lineage.AgentID, try+1, subagentStageRetries) + res, err = history.Stage(repoRoot, rootSHA, meta, raw) + } + return res, err +} + // subagentStageMeta assembles the lineage for one sub-agent stage, running the // attribution ladder's first rung. // diff --git a/internal/surface/cli/hook_subagent_stage_retry_test.go b/internal/surface/cli/hook_subagent_stage_retry_test.go new file mode 100644 index 000000000..c4aaeb9c7 --- /dev/null +++ b/internal/surface/cli/hook_subagent_stage_retry_test.go @@ -0,0 +1,214 @@ +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/intentdriven/abcd/internal/core/history" + "github.com/intentdriven/abcd/internal/fsutil" +) + +// iss-2610051434156079: a burst of sub-agent completions lost transcripts on a +// loaded runner while every hook exited 0. The cause is a git call that fails +// once: RootCommit reads it as no root commit, the cwd then reads as outside +// any repository, a burst's first hooks run before any sibling has noted the +// session, and the transcript was dropped with only a stderr line no one sees. + +// failRootCommitGit puts a `git` first on PATH that fails the root-commit +// lookup (`rev-list … --max-parents=0`) for its first n calls, as a fork +// refused or a git killed early on a loaded machine does, and runs the real +// git for everything else. The count is shared across processes, so a burst +// of hooks takes the failures between them. +func failRootCommitGit(t *testing.T, n int) { + t.Helper() + real, err := exec.LookPath("git") + if err != nil { + t.Skip("git unavailable") + } + dir := t.TempDir() + count := filepath.Join(t.TempDir(), "calls") + lock := count + ".lock" + body := "#!/bin/sh\n" + + "case \"$*\" in\n" + + "*max-parents*)\n" + + " while ! mkdir '" + lock + "' 2>/dev/null; do :; done\n" + + " k=$(cat '" + count + "' 2>/dev/null || echo 0); k=$((k+1)); echo $k > '" + count + "'\n" + + " rmdir '" + lock + "'\n" + + " if [ $k -le " + strconv.Itoa(n) + " ]; then echo 'fatal: simulated transient failure' >&2; exit 128; fi;;\n" + + "esac\n" + + "exec '" + real + "' \"$@\"\n" + if err := os.WriteFile(filepath.Join(dir, "git"), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +// TestHookSubagentStopRetriesATransientRootFailure: one failed root-commit +// lookup no longer drops the transcript; the hook asks again and stages. +func TestHookSubagentStopRetriesATransientRootFailure(t *testing.T) { + repo, rootSHA := sessionEndRepo(t) + tp := writeAgentTranscript(t, t.TempDir(), "r1", `{"agent":"r1"}`+"\n") + failRootCommitGit(t, 1) + + _, stderr, failed := runHookAllowingFailure(subagentPayload(t, "sess-retry", repo, "r1", tp, "general-purpose"), "hook", "subagent-stop") + if failed { + t.Fatalf("the hook exited non-zero: %s", stderr) + } + staged, err := history.ListStaged(repo, rootSHA) + if err != nil { + t.Fatal(err) + } + if len(staged) != 1 { + t.Fatalf("a transient git failure dropped the transcript: %d staged\nstderr: %s", len(staged), stderr) + } + if !strings.Contains(stderr, "resolved on try") { + t.Errorf("stderr does not say the repository resolved on a retry: %q", stderr) + } +} + +// TestHookSubagentStopBurstSurvivesTransientGitFailures is the record's burst +// on a loaded machine made deterministic: twelve completions at once, with the +// first six root-commit lookups failing, and every transcript staged. +func TestHookSubagentStopBurstSurvivesTransientGitFailures(t *testing.T) { + repo, rootSHA := sessionEndRepo(t) + dir := t.TempDir() + const n = 12 + payloads := make([]string, n) + for i := range n { + id := "t" + string(rune('a'+i)) + tp := writeAgentTranscript(t, dir, id, `{"agent":"`+id+`"}`+"\n") + payloads[i] = subagentPayload(t, "sess-burst-fail", repo, id, tp, "general-purpose") + } + failRootCommitGit(t, n/2) + + var wg sync.WaitGroup + stderrs := make([]string, n) + for i := range n { + wg.Add(1) + go func() { + defer wg.Done() + _, stderrs[i], _ = runHookAllowingFailure(payloads[i], "hook", "subagent-stop") + }() + } + wg.Wait() + staged, err := history.ListStaged(repo, rootSHA) + if err != nil { + t.Fatal(err) + } + if len(staged) != n { + t.Fatalf("want %d staged from %d completions with transient git failures, got %d\n%s", n, n, len(staged), notStaged(stderrs)) + } +} + +// stagingLockFor is the path of the staging lock one agent's stage takes: +// history keys it on the agent id under the staging directory's locks folder. +func stagingLockFor(t *testing.T, repo, rootSHA, agentID string) string { + t.Helper() + store, err := history.Resolve(repo, rootSHA) + if err != nil { + t.Fatal(err) + } + locks := filepath.Join(store.Staging, "locks") + if err := fsutil.EnsureRealDir(store.Staging, 0o700); err != nil { + t.Fatal(err) + } + if err := fsutil.EnsureRealDir(locks, 0o700); err != nil { + t.Fatal(err) + } + return filepath.Join(locks, agentID+".lock") +} + +// holdLock takes path's lock and holds it for d, signalling once it is held. +func holdLock(t *testing.T, path string, d time.Duration) (held <-chan struct{}, done <-chan error) { + t.Helper() + h, out := make(chan struct{}), make(chan error, 1) + go func() { + out <- fsutil.WithFileLock(path, time.Second, func() error { + close(h) + time.Sleep(d) + return nil + }) + }() + return h, out +} + +// TestHookSubagentStopWaitsOutAHeldStagingLock: a staging lock held past its +// own five-second timeout (the record's deliberately slowed lock) is waited +// out by a retry, not reported as a lost transcript. +func TestHookSubagentStopWaitsOutAHeldStagingLock(t *testing.T) { + repo, rootSHA := sessionEndRepo(t) + tp := writeAgentTranscript(t, t.TempDir(), "w1", `{"agent":"w1"}`+"\n") + held, done := holdLock(t, stagingLockFor(t, repo, rootSHA, "w1"), 7*time.Second) + <-held + + _, stderr, failed := runHookAllowingFailure(subagentPayload(t, "sess-held", repo, "w1", tp, "general-purpose"), "hook", "subagent-stop") + if err := <-done; err != nil { + t.Fatal(err) + } + if failed { + t.Fatalf("the hook exited non-zero: %s", stderr) + } + staged, err := history.ListStaged(repo, rootSHA) + if err != nil { + t.Fatal(err) + } + if len(staged) != 1 { + t.Fatalf("a held staging lock dropped the transcript: %d staged\nstderr: %s", len(staged), stderr) + } + if !strings.Contains(stderr, "trying again") { + t.Errorf("stderr does not say the stage was tried again: %q", stderr) + } + if _, ok, _ := history.StageMiss(repo, rootSHA); ok { + t.Error("a stage that succeeded on a retry was recorded as missed") + } +} + +// TestHookSubagentStopMarksAStageThatStillFails: a stage that fails for good, +// with the store known, is counted in a marker that `abcd history staged` +// reports, so the loss is visible to the person and not only in a debug log. +func TestHookSubagentStopMarksAStageThatStillFails(t *testing.T) { + repo, rootSHA := sessionEndRepo(t) + tp := writeAgentTranscript(t, t.TempDir(), "m1", `{"agent":"m1"}`+"\n") + prev := subagentStageRetries + subagentStageRetries = 0 + t.Cleanup(func() { subagentStageRetries = prev }) + held, done := holdLock(t, stagingLockFor(t, repo, rootSHA, "m1"), 7*time.Second) + <-held + + _, stderr, failed := runHookAllowingFailure(subagentPayload(t, "sess-miss", repo, "m1", tp, "general-purpose"), "hook", "subagent-stop") + if err := <-done; err != nil { + t.Fatal(err) + } + if failed { + t.Fatalf("the hook exited non-zero, which blocks the sub-agent: %s", stderr) + } + miss, ok, err := history.StageMiss(repo, rootSHA) + if err != nil || !ok { + t.Fatalf("the missed stage left no marker (%v)\nstderr: %s", err, stderr) + } + if miss.Count != 1 || miss.LastAgent != "m1" || !strings.Contains(miss.LastReason, "lock") { + t.Errorf("marker = %+v; want one miss by m1 naming the lock", miss) + } + t.Chdir(repo) + if out := string(runCLI(t, "history", "staged")); !strings.Contains(out, "1 sub-agent transcript(s) were read but could not be staged") { + t.Errorf("history staged does not report the missed stage:\n%s", out) + } +} + +// notStaged lists the stderr of every hook in a burst that did not report +// staging, so a failing burst names its cause instead of only a count. +func notStaged(stderrs []string) string { + var b strings.Builder + for i, s := range stderrs { + if !strings.Contains(s, "staged sub-agent") { + b.WriteString("hook " + strconv.Itoa(i) + ": " + strings.TrimSpace(s) + "\n") + } + } + return b.String() +} diff --git a/internal/surface/cli/hook_subagent_stop_test.go b/internal/surface/cli/hook_subagent_stop_test.go index a68fab829..01fc87cfc 100644 --- a/internal/surface/cli/hook_subagent_stop_test.go +++ b/internal/surface/cli/hook_subagent_stop_test.go @@ -313,11 +313,14 @@ func TestHookSubagentStopConcurrentCompletionsAllStage(t *testing.T) { var wg sync.WaitGroup fails := make([]string, n) + stderrs := make([]string, n) for i := range n { wg.Add(1) go func() { defer wg.Done() - if _, stderr, failed := runHookAllowingFailure(payloads[i], "hook", "subagent-stop"); failed { + _, stderr, failed := runHookAllowingFailure(payloads[i], "hook", "subagent-stop") + stderrs[i] = stderr + if failed { fails[i] = stderr } }() @@ -333,7 +336,7 @@ func TestHookSubagentStopConcurrentCompletionsAllStage(t *testing.T) { t.Fatal(err) } if len(staged) != n { - t.Fatalf("want %d staged transcripts from %d simultaneous completions, got %d", n, n, len(staged)) + t.Fatalf("want %d staged transcripts from %d simultaneous completions, got %d\n%s", n, n, len(staged), notStaged(stderrs)) } } From 5ba125b8698b1635aa66650aa9b98ae168969aab Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:30:51 +0100 Subject: [PATCH 03/10] fix: never read a failed git call as a repository with no root commit gitutil.RootCommit returned "" for every git failure but a timeout, so a refused fork or an early-killed git on a loaded machine read as "no root commit" at every caller. It now returns (string, error). ("", nil) means only a missing directory, no repository at or above it, or a repository with no commits, which `rev-parse --verify -q HEAD` confirms. Any other failure is ErrNoRootAnswer or ErrGitTimedOut. Every caller now gives the error a deliberate answer. Where it returns errors, it propagates. Where it is total, it keeps its safe fallback and says git did not answer. Nowhere does the error take the empty path with consequences: keying, creating or migrating a store, adopting a transcript, or refusing as "not a repository". ahoy's identity carries the failure apart from a definite "no root". The session-end hook now retries it, as the sub-agent hook does, and both mark a transcript they still cannot stage. The stage-miss marker covers both. Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- commands/history.md | 8 +- internal/core/ahoy/ahoy.go | 12 ++ internal/core/ahoy/apply.go | 8 ++ internal/core/ahoy/managed.go | 12 +- internal/core/ahoy/managed_test.go | 5 +- internal/core/ahoy/remote.go | 5 + internal/core/ahoy/store.go | 9 +- internal/core/ahoy/store_env_test.go | 3 +- internal/core/embargo/everystore_test.go | 5 +- internal/core/embargo/storeroots_test.go | 8 +- internal/core/history/ingest.go | 96 +++++++++----- internal/core/history/ingest_test.go | 67 +++++++++- internal/core/history/stagemiss.go | 35 ++--- internal/core/implement/load.go | 8 +- internal/core/implement/loop/check.go | 8 +- .../core/implement/loop/drain_judge_test.go | 9 +- internal/core/implement/loop/drive_test.go | 9 ++ internal/core/implement/loop/lane.go | 7 +- internal/core/implement/loop/loop.go | 5 +- internal/core/implement/loop/outage.go | 17 ++- internal/core/implement/loop/outage_test.go | 14 +- internal/core/implement/loop/restart.go | 6 +- internal/core/implement/loop/restart_test.go | 5 +- internal/core/lab/lab.go | 7 +- internal/core/lifeboat/probe.go | 17 ++- internal/core/lifeboat/probe_test.go | 4 +- internal/core/memnote/store_test.go | 20 ++- internal/core/memnote/write.go | 10 +- internal/core/peers/peers_test.go | 3 +- internal/core/peers/storeroots_test.go | 5 +- internal/core/recall/items.go | 2 +- internal/core/report/inbox.go | 11 +- internal/core/report/inbox_test.go | 5 +- internal/core/site/build.go | 8 +- internal/core/site/setup.go | 5 + internal/gittest/rootcommit.go | 20 +++ internal/gitutil/context_test.go | 10 +- internal/gitutil/repo.go | 68 +++++++--- internal/gitutil/repo_test.go | 16 +-- internal/gitutil/rootcommit_test.go | 121 ++++++++++++++++++ internal/gitutil/roots_test.go | 4 +- internal/gitutil/waitdelay_test.go | 4 +- internal/surface/cli/cli.go | 35 ++++- internal/surface/cli/dashboard.go | 6 +- .../surface/cli/filing_match_surface_test.go | 4 +- internal/surface/cli/history.go | 8 +- internal/surface/cli/history_recovery.go | 4 + internal/surface/cli/hook_subagent.go | 108 +++++++++++----- .../cli/hook_subagent_stage_retry_test.go | 31 ++++- internal/surface/cli/ideate.go | 6 +- internal/surface/cli/ideate_root_test.go | 4 +- internal/surface/cli/ideate_surface_test.go | 5 +- internal/surface/cli/implement.go | 7 +- internal/surface/cli/lab_surface_test.go | 4 +- internal/surface/cli/memory.go | 25 +++- internal/surface/cli/memory_test.go | 12 +- internal/surface/cli/outage_render_test.go | 4 +- internal/surface/cli/report_surface_test.go | 5 +- internal/surface/cli/source.go | 5 +- internal/surface/cli/statusline.go | 7 +- 60 files changed, 757 insertions(+), 224 deletions(-) create mode 100644 internal/gittest/rootcommit.go create mode 100644 internal/gitutil/rootcommit_test.go diff --git a/commands/history.md b/commands/history.md index bcb419b75..dbf245ba9 100644 --- a/commands/history.md +++ b/commands/history.md @@ -168,10 +168,10 @@ text** until drained, so say so whenever the list is non-empty. The text render also carries a note when the host has fired a sub-agent stop without handing over a transcript path: on such a host no sub-agent transcript can be captured at all, so an empty sub-agent corpus is the host and not the sessions. It -carries a second note when sub-agent transcripts were read but could not be -staged, with how many, when, and the last one's reason: those transcripts are -not in the store, and the hook that lost them exited 0, so this note is the only -place the loss shows. Tell the user how many were lost and why. An entry +carries a second note when transcripts, a session's or a sub-agent's, were +read but could not be staged, with how many, when, and the last one's reason: +those transcripts are not in the store, and the hook that lost them exited 0, +so this note is the only place the loss shows. Tell the user how many were lost and why. An entry past the seven-day limit renders as OVERDUE; a quarantined transcript is listed in its own block, and is not awaiting anything — nothing will retry it. diff --git a/internal/core/ahoy/ahoy.go b/internal/core/ahoy/ahoy.go index e9a83f3d8..f9e6a7e69 100644 --- a/internal/core/ahoy/ahoy.go +++ b/internal/core/ahoy/ahoy.go @@ -83,8 +83,20 @@ type RepoIdentity struct { Name string `json:"name"` Github string `json:"github"` RootSHA string `json:"root_sha"` + // rootErr is why RootSHA is "" when git did not answer for a repository + // (gitutil.ErrNoRootAnswer, gitutil.ErrGitTimedOut): nil when RootSHA is + // the answer, "" included. Unexported, so the JSON envelope is unchanged. + rootErr error } +// RootErr reports why the root commit is unknown when git did not answer for +// the repository, and nil when RootSHA is git's answer — an empty RootSHA with +// a nil RootErr is a directory outside any repository, or one with no commits. +// A caller that would key, create or refuse anything on an empty RootSHA asks +// this first, so a failed git call is never read as "no repository" +// (iss-2610051434156079). +func (id RepoIdentity) RootErr() error { return id.rootErr } + // DetectionResult is the canonical envelope. dry-run marshals exactly this // (with Adopted=nil). type DetectionResult struct { diff --git a/internal/core/ahoy/apply.go b/internal/core/ahoy/apply.go index e7cdd8caf..7822ce7c4 100644 --- a/internal/core/ahoy/apply.go +++ b/internal/core/ahoy/apply.go @@ -1197,6 +1197,14 @@ func (a *applyCtx) stepHistory() { a.note(writeSessionStore, filepath.Join(root, "index.json")) } sha := a.det.RepoIdentity.RootSHA + if rerr := a.det.RepoIdentity.RootErr(); rerr != nil { + // git did not answer for the root commit: the repository is not + // registered or given a store this run, and says so, rather than + // being passed over as one with no commits (iss-2610051434156079). + a.refuse("could not register this repository on this machine: git gave no root commit for it (" + + fsutil.RedactRoot(errText(rerr), a.cwd, ".") + "); run `abcd ahoy install` again") + return + } if sha == "" { return } diff --git a/internal/core/ahoy/managed.go b/internal/core/ahoy/managed.go index e56d122f6..793c42ff0 100644 --- a/internal/core/ahoy/managed.go +++ b/internal/core/ahoy/managed.go @@ -26,6 +26,11 @@ import ( // two file reads and no subprocess; git is asked for the root commit only when // the marker is absent AND there is an index to look it up in. Zero network, // zero writes. +// +// It is total: a git that did not answer for the root commit reads as "not +// managed" here, which is safe only for a caller whose "not managed" is an +// omission (a board block left out, a badge not shown). A caller that refuses +// an act as unmanaged asks ManagedContext, which returns that failure. func Managed(root string) bool { ok, _ := ManagedContext(context.Background(), root) return ok @@ -34,9 +39,10 @@ func Managed(root string) bool { // ManagedContext is Managed bound to ctx, for a caller on a deadline (the // status verb). The one subprocess it may start — git's root commit, asked // only when no marker block answers — runs under ctx and is killed when ctx -// ends (gitutil.RootCommitContext). A context that ended before git answered -// is returned as its own error, never as "not managed", so the caller does not -// mistake a slow git for a checkout abcd does not manage. +// ends (gitutil.RootCommitContext). A context that ended before git answered, +// and a git that gave no answer for the repository (gitutil.ErrNoRootAnswer), +// are returned as errors, never as "not managed", so the caller does not +// mistake a slow or failed git for a checkout abcd does not manage. func ManagedContext(ctx context.Context, root string) (bool, error) { abs, err := filepath.Abs(root) if err != nil { diff --git a/internal/core/ahoy/managed_test.go b/internal/core/ahoy/managed_test.go index b9eca5b29..3c90b2fe8 100644 --- a/internal/core/ahoy/managed_test.go +++ b/internal/core/ahoy/managed_test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // committedRepo stands up a git checkout with one commit, so it has a root @@ -61,7 +60,7 @@ func TestManagedAgreesWithClassify(t *testing.T) { if _, err := bootstrapHistory(); err != nil { t.Fatal(err) } - sha := gitutil.RootCommit(registered) + sha := gittest.RootCommit(t, registered) if sha == "" { t.Fatal("the registered fixture has no root commit") } @@ -154,7 +153,7 @@ func TestManagedContextReportsAnEndedContextAsItself(t *testing.T) { if _, err := bootstrapHistory(); err != nil { t.Fatal(err) } - if err := writeHistoryIndex(&historyIndex{Schema: 1, Repos: []historyRepo{{RootCommit: gitutil.RootCommit(registered), Path: registered, Status: "active"}}}); err != nil { + if err := writeHistoryIndex(&historyIndex{Schema: 1, Repos: []historyRepo{{RootCommit: gittest.RootCommit(t, registered), Path: registered, Status: "active"}}}); err != nil { t.Fatal(err) } if ok, err := ManagedContext(context.Background(), registered); !ok || err != nil { diff --git a/internal/core/ahoy/remote.go b/internal/core/ahoy/remote.go index 1f38f3601..34ff25050 100644 --- a/internal/core/ahoy/remote.go +++ b/internal/core/ahoy/remote.go @@ -315,6 +315,11 @@ func remotePrepare(cwd string) (res RemoteResult, abs string, done bool) { if err != nil { return refuseRemote(res, "could not classify this folder, so abcd will not act on a repository it cannot identify: "+errText(err)), "", true } + if rerr := det.RepoIdentity.RootErr(); det.RootSHA == "" && rerr != nil { + // git not answering is not a folder outside abcd: say which it was, + // and still make no remote write (iss-2610051434156079). + return refuseRemote(res, "git gave no root commit for this repository, so abcd cannot tell whether it manages it: "+errText(rerr)), "", true + } if det.FolderKind != ManagedRepo { return refuseRemote(res, "this is not a repo abcd manages ("+string(det.FolderKind)+ "); a remote write is never made on a folder that did not adopt abcd (run `abcd ahoy install` first)"), "", true diff --git a/internal/core/ahoy/store.go b/internal/core/ahoy/store.go index d90c3c254..3215d3727 100644 --- a/internal/core/ahoy/store.go +++ b/internal/core/ahoy/store.go @@ -60,12 +60,17 @@ func runGit(cwd string, args ...string) (string, error) { return string(out), nil } -// deriveIdentity builds the deterministic RepoIdentity for cwd. +// deriveIdentity builds the deterministic RepoIdentity for cwd. A git that +// did not answer for the root commit leaves RootSHA "" with the failure kept +// in RootErr, so Detect stays total while its callers can still tell "no root +// commit" from "git failed" (iss-2610051434156079). func deriveIdentity(cwd string) RepoIdentity { + sha, err := gitutil.RootCommit(cwd) return RepoIdentity{ Name: filepath.Base(cwd), Github: originURL(cwd), - RootSHA: gitutil.RootCommit(cwd), + RootSHA: sha, + rootErr: err, } } diff --git a/internal/core/ahoy/store_env_test.go b/internal/core/ahoy/store_env_test.go index aa45f8fd6..6e44ec796 100644 --- a/internal/core/ahoy/store_env_test.go +++ b/internal/core/ahoy/store_env_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // TestRunGitIgnoresInheritedGitDir proves an inherited GIT_DIR cannot redirect @@ -36,7 +35,7 @@ func TestRunGitIgnoresInheritedGitDir(t *testing.T) { // pointing at repoB. t.Setenv("GIT_DIR", filepath.Join(repoB, ".git")) - if got := gitutil.RootCommit(repoA); got != shaA { + if got := gittest.RootCommit(t, repoA); got != shaA { t.Errorf("RootCommit(repoA) = %q under inherited GIT_DIR=repoB; want repoA's root %q (GIT_DIR hijacked the query)", got, shaA) } } diff --git a/internal/core/embargo/everystore_test.go b/internal/core/embargo/everystore_test.go index de5ab95c2..863cd96dd 100644 --- a/internal/core/embargo/everystore_test.go +++ b/internal/core/embargo/everystore_test.go @@ -12,7 +12,6 @@ import ( "github.com/intentdriven/abcd/internal/abcdhome" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // ghsaPendingElsewhere is the pending id sampleState holds. @@ -105,7 +104,7 @@ func TestShallowSecondCloneChecksAgainstTheFirstClonesStore(t *testing.T) { cf.f.repo.Write("a.txt", "two\n") cf.f.repo.Commit("c2") clone := shallowClone(t, cf.f) - if key := filepath.Base(filepath.Dir(cf.s.Root)); gitutil.RootCommit(clone) == key { + if key := filepath.Base(filepath.Dir(cf.s.Root)); gittest.RootCommit(t, clone) == key { t.Fatal("the shallow clone's root is the store's key, so it proves nothing") } wantEmbargoedIDRefused(t, cf.f.home, clone) @@ -352,7 +351,7 @@ func TestNoFreePlaceAdviceFitsWhoHoldsThePlace(t *testing.T) { other := gittest.NewRepo(t) other.Write("README.md", "another project\n") datedCommit(t, other, "2011-01-01T00:00:00Z", "its own root") - place := abcdhome.Path(f.home, "worktrees", gitutil.RootCommit(f.repo.Root()), StoreName) + place := abcdhome.Path(f.home, "worktrees", gittest.RootCommit(t, f.repo.Root()), StoreName) if err := os.MkdirAll(filepath.Dir(place), 0o700); err != nil { t.Fatal(err) } diff --git a/internal/core/embargo/storeroots_test.go b/internal/core/embargo/storeroots_test.go index 7813eff8b..706fc5abe 100644 --- a/internal/core/embargo/storeroots_test.go +++ b/internal/core/embargo/storeroots_test.go @@ -36,7 +36,7 @@ func mergeUnrelatedRoot(t *testing.T, r *gittest.Repo) string { t.Helper() other := datedRoot(t, r, "2090-01-01T00:00:00Z", "an unrelated root") r.Git("merge", "-q", "-s", "ours", "--allow-unrelated-histories", "--no-edit", other) - if gitutil.RootCommit(r.Root()) != other { + if gittest.RootCommit(t, r.Root()) != other { t.Fatal("the fixture's rev-list does not meet the merged root first") } return other @@ -341,7 +341,7 @@ func TestStoreFoundAfterAShallowCloneDeepens(t *testing.T) { } holdSample(t, s) cloneGit("-C", clone, "fetch", "-q", "--unshallow") - if gitutil.RootCommit(clone) == boundary { + if gittest.RootCommit(t, clone) == boundary { t.Fatal("the deepened clone still has the boundary for its root") } wantEmbargoedIDRefused(t, src.home, clone) @@ -359,7 +359,7 @@ func TestStoreMadeUnderAReplacedRootIsFound(t *testing.T) { standIn := f.repo.Git("commit-tree", f.repo.Git("rev-parse", "HEAD^{tree}"), "-m", "a stand-in root") f.repo.Git("replace", tip, standIn) stored := f.repo.Git("-c", "core.useReplaceRefs=false", "rev-list", "--max-parents=0", "HEAD") - oldKey := gitutil.RootCommit(f.repo.Root()) + oldKey := gittest.RootCommit(t, f.repo.Root()) if !gitutil.IsFullSHA(oldKey) || oldKey == stored { t.Fatalf("the replace ref does not move the root an earlier abcd keyed on (%q, stored %q)", oldKey, stored) } @@ -469,7 +469,7 @@ func TestOrdinaryCloneKeepsItsStorePath(t *testing.T) { if err != nil || !created { t.Fatalf("openStore: created %v, %v", created, err) } - if want := abcdhome.Path(f.home, "worktrees", gitutil.RootCommit(f.repo.Root()), StoreName); s.Root != want { + if want := abcdhome.Path(f.home, "worktrees", gittest.RootCommit(t, f.repo.Root()), StoreName); s.Root != want { t.Fatalf("store at %s; want %s", s.Root, want) } } diff --git a/internal/core/history/ingest.go b/internal/core/history/ingest.go index a4225b103..5f79d4d0e 100644 --- a/internal/core/history/ingest.go +++ b/internal/core/history/ingest.go @@ -67,6 +67,11 @@ const ( SkipAmbiguousOwner = "ambiguous-owner" SkipNoSession = "no-session-id" SkipAmbiguousAgent = "ambiguous-agent-id" + // SkipOwnerUnanswered is a transcript whose recorded directory is a + // repository git did not answer for: its owner is unknown, not absent, so + // it is neither stored nor offered for adoption as an orphan + // (iss-2610051434156079). + SkipOwnerUnanswered = "owner-unanswered" ) // resolveRootSHA maps a recorded working directory to the root-commit SHA of @@ -81,14 +86,10 @@ const ( // ahoy.Detect: internal/core/ahoy now depends on this package for the store's // location (Resolve), so reaching back into it from here would be an import // cycle. gitutil.RootCommit is the same call ahoy's own identity pass makes, -// and it answers the only question asked here. -var resolveRootSHA = func(cwd string) (string, bool) { - sha := gitutil.RootCommit(cwd) - if sha == "" { - return "", false - } - return sha, true -} +// and it answers the only question asked here: ("", nil) for a directory +// that is gone, outside any repository or in one with no commits, and an +// error when git did not answer for a repository. +var resolveRootSHA = gitutil.RootCommit // Destination is the repository a run of Ingest writes into: its root for the // scanner configuration, and its root-commit SHA for the store key. @@ -116,8 +117,12 @@ type Destination struct { // unresolvable root is not evidence that the pair agrees, and accepting one // would leave the invariant defended only where it happens to be checkable. func (d Destination) verify() error { - sha, ok := resolveRootSHA(d.RepoRoot) - if !ok || sha == "" { + sha, err := resolveRootSHA(d.RepoRoot) + if err != nil { + return fmt.Errorf("history: ingest could not read the root commit of the destination repository at %s, so it cannot prove that root owns the store key %s: %s", + fsutil.DisplayPath(d.RepoRoot), d.RootSHA, fsutil.RedactHome(err.Error())) + } + if sha == "" { return fmt.Errorf("history: ingest cannot resolve the root commit of the destination repository at %s, so it cannot prove that root owns the store key %s; name a git repository with commits as the destination", fsutil.DisplayPath(d.RepoRoot), d.RootSHA) } @@ -274,6 +279,9 @@ type sessionPlacement struct { rootSHA string via string ambiguous bool + // unanswered is a session whose main-thread directory is a repository + // git did not answer for, and that no store owns. + unanswered bool } // placeSessions resolves the owning repository once per SESSION, before any @@ -311,7 +319,7 @@ func placeSessions(probes []transcriptProbe) map[string]sessionPlacement { mainCwds[p.sessionID] = append(mainCwds[p.sessionID], p.cwds...) } out := map[string]sessionPlacement{} - cache := map[string]string{} + cache := map[string]rootAnswer{} for _, p := range probes { if p.sessionID == "" { continue @@ -319,39 +327,55 @@ func placeSessions(probes []transcriptProbe) map[string]sessionPlacement { if _, done := out[p.sessionID]; done { continue } - shas := resolveAll(mainCwds[p.sessionID], cache) - switch len(shas) { - case 1: - out[p.sessionID] = sessionPlacement{rootSHA: shas[0], via: "session-cwd"} - continue - case 0: - default: + shas, unanswered := resolveAll(mainCwds[p.sessionID], cache) + switch { + case len(shas) > 1: out[p.sessionID] = sessionPlacement{ambiguous: true, via: "session-cwd"} continue + case len(shas) == 1 && !unanswered: + out[p.sessionID] = sessionPlacement{rootSHA: shas[0], via: "session-cwd"} + continue } + // No directory answered, or one did and another is a repository git + // did not answer for, which may be a second owner: a store that + // placed the session earlier still settles it. if sha := storeOwner(p.sessionID); sha != "" { out[p.sessionID] = sessionPlacement{rootSHA: sha, via: "store"} continue } + if unanswered { + out[p.sessionID] = sessionPlacement{unanswered: true, via: "session-cwd"} + continue + } out[p.sessionID] = sessionPlacement{} } return out } +// rootAnswer is one directory's root-commit lookup, cached for a run. +type rootAnswer struct { + sha string + unanswered bool +} + // resolveAll maps a set of recorded working directories to the distinct // repositories they belong to, in first-seen order. Directories that no longer // exist simply do not answer; several directories inside one repository (a // worktree, a subdirectory) collapse to one SHA, which is why the count is over -// repositories and not over paths. -func resolveAll(cwds []string, cache map[string]string) []string { - var out []string +// repositories and not over paths. unanswered reports that at least one +// directory is a repository git did not answer for: its owner is unknown, and +// the caller must not read the answer as complete. +func resolveAll(cwds []string, cache map[string]rootAnswer) (out []string, unanswered bool) { seen := map[string]struct{}{} for _, cwd := range cwds { - sha, cached := cache[cwd] + a, cached := cache[cwd] if !cached { - sha, _ = resolveRootSHA(cwd) - cache[cwd] = sha + sha, err := resolveRootSHA(cwd) + a = rootAnswer{sha: sha, unanswered: err != nil} + cache[cwd] = a } + unanswered = unanswered || a.unanswered + sha := a.sha if sha == "" { continue } @@ -361,7 +385,7 @@ func resolveAll(cwds []string, cache map[string]string) []string { seen[sha] = struct{}{} out = append(out, sha) } - return out + return out, unanswered } // ingestOne places and, where the destination owns it, stores one transcript. @@ -371,22 +395,28 @@ func ingestOne(dest Destination, opts IngestOptions, p transcriptProbe, placed s return } sha, via := placed.rootSHA, placed.via - ambiguous := placed.ambiguous - if sha == "" && !ambiguous { + ambiguous, unanswered := placed.ambiguous, placed.unanswered + if sha == "" && !ambiguous && !unanswered { // Fallback: the file's own recorded directory. - shas := resolveAll(p.cwds, map[string]string{}) - switch len(shas) { - case 1: - sha, via = shas[0], "file-cwd" - case 0: - default: + shas, fileUnanswered := resolveAll(p.cwds, map[string]rootAnswer{}) + switch { + case len(shas) > 1: ambiguous, via = true, "file-cwd" + case fileUnanswered: + unanswered, via = true, "file-cwd" + case len(shas) == 1: + sha, via = shas[0], "file-cwd" } } switch { case ambiguous: res.Skipped = append(res.Skipped, IngestSkip{Path: p.path, SessionID: p.sessionID, AgentID: p.agentID, Reason: SkipAmbiguousOwner}) return + case unanswered: + // Never an orphan: an orphan may be adopted into this store, and + // this transcript's owner is a repository git did not answer for. + res.Skipped = append(res.Skipped, IngestSkip{Path: p.path, SessionID: p.sessionID, AgentID: p.agentID, Reason: SkipOwnerUnanswered}) + return case sha == "": if _, claimed := adopt[p.project]; !claimed { res.Orphans = append(res.Orphans, Orphan{ diff --git a/internal/core/history/ingest_test.go b/internal/core/history/ingest_test.go index ce5f09918..fc4b7c2da 100644 --- a/internal/core/history/ingest_test.go +++ b/internal/core/history/ingest_test.go @@ -1,10 +1,14 @@ package history import ( + "fmt" "os" "path/filepath" + "slices" "strings" "testing" + + "github.com/intentdriven/abcd/internal/gitutil" ) // otherRootSHA is a second repository's store key: a transcript that resolves @@ -23,9 +27,22 @@ const otherRootSHA = "cccccccccccccccccccccccccccccccccccccccc" func fakeRepos(t *testing.T, table map[string]string) { t.Helper() prior := resolveRootSHA - resolveRootSHA = func(cwd string) (string, bool) { - sha, ok := table[cwd] - return sha, ok + resolveRootSHA = func(cwd string) (string, error) { + return table[cwd], nil + } + t.Cleanup(func() { resolveRootSHA = prior }) +} + +// fakeReposFailing is fakeRepos where git does not answer for each directory +// in failing (gitutil.ErrNoRootAnswer), as on a loaded machine. +func fakeReposFailing(t *testing.T, table map[string]string, failing ...string) { + t.Helper() + prior := resolveRootSHA + resolveRootSHA = func(cwd string) (string, error) { + if slices.Contains(failing, cwd) { + return "", fmt.Errorf("%w: %s: exit status 128", gitutil.ErrNoRootAnswer, cwd) + } + return table[cwd], nil } t.Cleanup(func() { resolveRootSHA = prior }) } @@ -228,6 +245,50 @@ func TestIngestAdoptsOnlyProjectsNamedByTheDestination(t *testing.T) { } } +// TestIngestNeverAdoptsATranscriptWhoseRepositoryGitDidNotAnswerFor: a +// recorded directory that is a repository git failed for once read as one +// with no repository, so its transcript became an orphan, and naming its +// project adopted it into the destination's store under the destination's +// redaction configuration (iss-2610051434156079). Its owner is unknown, not +// absent: it is skipped and said, never stored, never offered as an orphan. +func TestIngestNeverAdoptsATranscriptWhoseRepositoryGitDidNotAnswerFor(t *testing.T) { + repoRoot, _ := setupStore(t) + src := t.TempDir() + fakeReposFailing(t, map[string]string{repoRoot: testRootSHA}, "/theirs") + transcriptFile(t, filepath.Join(src, "claimed"), "s1.jsonl", "sess-unanswered", "", "/theirs") + + res, err := Ingest(Destination{RepoRoot: repoRoot, RootSHA: testRootSHA}, []string{src}, + IngestOptions{Adopt: []string{"claimed"}}) + if err != nil { + t.Fatalf("Ingest: %v", err) + } + if len(res.Captured) != 0 || len(res.Orphans) != 0 { + t.Fatalf("a transcript whose repository git did not answer for was stored or offered as an orphan: captured %+v, orphans %+v", res.Captured, res.Orphans) + } + if len(res.Skipped) != 1 || res.Skipped[0].Reason != SkipOwnerUnanswered { + t.Fatalf("want one %s skip, got %+v", SkipOwnerUnanswered, res.Skipped) + } + records, err := List(repoRoot, testRootSHA) + if err != nil { + t.Fatal(err) + } + if len(records) != 0 { + t.Errorf("the store must be untouched, got %d records", len(records)) + } +} + +// TestIngestRefusesADestinationRootGitDidNotAnswerFor: the destination's own +// root commit failing is said as git's failure, not as a directory that is no +// repository. +func TestIngestRefusesADestinationRootGitDidNotAnswerFor(t *testing.T) { + repoRoot, _ := setupStore(t) + fakeReposFailing(t, nil, repoRoot) + _, err := Ingest(Destination{RepoRoot: repoRoot, RootSHA: testRootSHA}, []string{t.TempDir()}, IngestOptions{}) + if err == nil || !strings.Contains(err.Error(), "could not read the root commit") || strings.Contains(err.Error(), "name a git repository with commits") { + t.Fatalf("want git's failure named, not a request for a repository with commits: %v", err) + } +} + // TestIngestIsIdempotent: ingesting the same material twice adds nothing. func TestIngestIsIdempotent(t *testing.T) { repoRoot, _ := setupStore(t) diff --git a/internal/core/history/stagemiss.go b/internal/core/history/stagemiss.go index bd73f1e80..7496e9aa2 100644 --- a/internal/core/history/stagemiss.go +++ b/internal/core/history/stagemiss.go @@ -13,11 +13,11 @@ import ( ) // stageMissFilename is the marker, beside the staging directory, that counts -// the sub-agent transcripts a SubagentStop hook read but could not stage -// (iss-2610051434156079). The hook exits 0 whatever happens, since a non-zero -// exit is that event's blocking code, and its stderr reaches only a debug log, -// so without the marker a lost transcript would be seen by no one. -const stageMissFilename = "subagent-stage-misses.json" +// the transcripts a staging hook (sub-agent stop, session end) read but could +// not stage (iss-2610051434156079). Both hooks exit 0 whatever happens, and +// their stderr reaches only a debug log, so without the marker a lost +// transcript would be seen by no one. +const stageMissFilename = "stage-misses.json" // stageMissLockTimeout bounds the wait for the marker's own lock. A burst of // completions that all miss writes the marker once each, and the critical @@ -28,15 +28,15 @@ const stageMissLockTimeout = 5 * time.Second // whatever the failure said. const stageMissReasonMax = 300 -// StageMissNote is the marker: how many sub-agent transcripts were read but -// not staged, when the first and the last were, and the last one's agent and -// reason. +// StageMissNote is the marker: how many transcripts were read but not staged, +// when the first and the last were, and whose the last was (a sub-agent or a +// session) and why. type StageMissNote struct { Schema int `json:"schema"` FirstSeen time.Time `json:"first_seen"` LastSeen time.Time `json:"last_seen"` Count int `json:"count"` - LastAgent string `json:"last_agent"` + LastWho string `json:"last_who"` LastReason string `json:"last_reason"` } @@ -49,11 +49,12 @@ func stageMissPath(repoRoot, rootSHA string) (string, error) { return filepath.Join(lane, stageMissFilename), nil } -// NoteStageMiss records one sub-agent transcript that was read and could not -// be staged, under the marker's own lock so concurrent misses each count. -// agentID and reason are stored as given apart from a bound on the reason; -// every reader sanitises them before printing. -func NoteStageMiss(repoRoot, rootSHA, agentID, reason string) error { +// NoteStageMiss records one transcript that was read and could not be staged, +// under the marker's own lock so concurrent misses each count. who names it +// ("sub-agent " or "session "). who and reason are stored as given, +// apart from a bound on the reason, and every reader sanitises them before +// printing. +func NoteStageMiss(repoRoot, rootSHA, who, reason string) error { if !rootSHARe.MatchString(rootSHA) { return errors.New(rootSHAErrMsg) } @@ -71,7 +72,7 @@ func NoteStageMiss(repoRoot, rootSHA, agentID, reason string) error { if prior, ok, err := readStageMiss(path); err == nil && ok { note.FirstSeen, note.Count = prior.FirstSeen, prior.Count } - note.LastSeen, note.LastAgent, note.LastReason = now, agentID, reason + note.LastSeen, note.LastWho, note.LastReason = now, who, reason note.Count++ data, err := json.Marshal(note) if err != nil { @@ -81,8 +82,8 @@ func NoteStageMiss(repoRoot, rootSHA, agentID, reason string) error { }) } -// StageMiss reads the marker. A missing marker is not an error: no sub-agent -// transcript has been missed. +// StageMiss reads the marker. A missing marker is not an error: no transcript +// has been missed. func StageMiss(repoRoot, rootSHA string) (StageMissNote, bool, error) { if !rootSHARe.MatchString(rootSHA) { return StageMissNote{}, false, errors.New(rootSHAErrMsg) diff --git a/internal/core/implement/load.go b/internal/core/implement/load.go index 3f7e70fc9..2c18c1822 100644 --- a/internal/core/implement/load.go +++ b/internal/core/implement/load.go @@ -354,7 +354,13 @@ func scrubOwnNames(repoRoot string, own []LoadOwnStray) bool { func logLoadWarning(req LoadRequest, res LoadResult) LoadRunLog { sha := req.RootSHA if sha == "" && req.RepoRoot != "" { - sha = gitutil.RootCommit(req.RepoRoot) + var err error + if sha, err = gitutil.RootCommit(req.RepoRoot); err != nil { + // Best effort, like every failure here: the warning stands, and + // the log says it could not find the run rather than that there + // is none. + return LoadRunLog{Error: "reading the repository's root commit to find the run: " + err.Error()} + } } if !gitutil.IsFullSHA(sha) { return LoadRunLog{} diff --git a/internal/core/implement/loop/check.go b/internal/core/implement/loop/check.go index 3f846aff4..040ec2038 100644 --- a/internal/core/implement/loop/check.go +++ b/internal/core/implement/loop/check.go @@ -399,7 +399,13 @@ func readPeers(repoRoot string, now time.Time) (*peerSnapshot, error) { if err := snap.readReservations(repoRoot); err != nil { return nil, err } - if sha := gitutil.RootCommit(repoRoot); gitutil.IsFullSHA(sha) { + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + // Without the root commit the shared run's claims cannot be read, + // and a snapshot without them would show a claimed record as free. + return nil, fmt.Errorf("reading the repository's root commit to find the shared run's claims: %w", err) + } + if gitutil.IsFullSHA(sha) { run, err := implement.Peek(sha) if err != nil { return nil, err diff --git a/internal/core/implement/loop/drain_judge_test.go b/internal/core/implement/loop/drain_judge_test.go index 702ab80db..5f2447933 100644 --- a/internal/core/implement/loop/drain_judge_test.go +++ b/internal/core/implement/loop/drain_judge_test.go @@ -29,15 +29,10 @@ const judgeRoutedConfig = `{"roles":{"drain-judge":{"runner":"claude"}},"runner" // move runs git where the fake's PATH would hide it. func judgeEnv(t *testing.T, mode string) driveEnv { t.Helper() - git, err := exec.LookPath("git") - if err != nil { + if _, err := exec.LookPath("git"); err != nil { t.Skip("no git on PATH") } - e := newDriveEnv(t, mode, "claude") - if err := os.Symlink(git, filepath.Join(e.bin, "git")); err != nil { - t.Fatal(err) - } - return e + return newDriveEnv(t, mode, "claude") } // TestARoutedDrainJudgesAnswerIsTakenWithAbcdComputingTheDigest: the judge diff --git a/internal/core/implement/loop/drive_test.go b/internal/core/implement/loop/drive_test.go index f4bd88d08..8296b8f87 100644 --- a/internal/core/implement/loop/drive_test.go +++ b/internal/core/implement/loop/drive_test.go @@ -197,6 +197,15 @@ func newDriveEnv(t *testing.T, mode string, harnesses ...string) driveEnv { t.Fatal(err) } } + // git stays reachable: it is no harness, and the loop's root-commit + // lookup (the run's outage, the lane's store) refuses on a git that + // cannot start rather than reading the checkout as one with no root + // commit (iss-2610051434156079). + if git, err := exec.LookPath("git"); err == nil { + if err := os.Symlink(git, filepath.Join(e.bin, "git")); err != nil { + t.Fatal(err) + } + } t.Setenv("PATH", e.bin) t.Setenv(loopFakeEnv, mode) t.Setenv(loopFakeLogEnv, e.log) diff --git a/internal/core/implement/loop/lane.go b/internal/core/implement/loop/lane.go index db887e3f7..1effd9b8a 100644 --- a/internal/core/implement/loop/lane.go +++ b/internal/core/implement/loop/lane.go @@ -106,7 +106,12 @@ func laneWorktree(repoRoot, runID, laneID string) (LaneWorktree, error) { if err != nil { return LaneWorktree{}, err } - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + // Not the refusal below: git did not answer, which is no evidence + // the repository has no root commit, and no store path is built. + return LaneWorktree{}, fmt.Errorf("reading the repository's root commit to key the worktree store on: %w", err) + } if !gitutil.IsFullSHA(sha) { return LaneWorktree{}, refuse(string(StageWorktree), "", laneID, "the repository has no root commit to key the worktree store on", "commit to the repository first; the store is keyed on its root commit") diff --git a/internal/core/implement/loop/loop.go b/internal/core/implement/loop/loop.go index 8dfc8fea3..a145cd133 100644 --- a/internal/core/implement/loop/loop.go +++ b/internal/core/implement/loop/loop.go @@ -538,7 +538,10 @@ func pickNote(p RunPick) string { // stage a checkout with no root commit, a session that is not a name, and a run // the session has not joined, before anything is created. func sharedRunFor(repoRoot, session string) (*implement.Run, error) { - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + return nil, fmt.Errorf("reading the repository's root commit to open the shared run: %w", err) + } run, err := implement.OpenJoined(sha, session) if err == nil { _, err = run.Joined(session) diff --git a/internal/core/implement/loop/outage.go b/internal/core/implement/loop/outage.go index f653083d8..4d1d142dd 100644 --- a/internal/core/implement/loop/outage.go +++ b/internal/core/implement/loop/outage.go @@ -120,7 +120,12 @@ type outageGate struct { // the lanes, and the probe and the record are left to `abcd implement outage`. func outageFor(repoRoot, runID string, o Options) (*outageGate, error) { g := &outageGate{now: o.now()} - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + // A gate with no run holds nothing, so git not answering is never + // read as a checkout with no shared run state. + return nil, fmt.Errorf("reading the repository's root commit to find the run's outage: %w", err) + } if !gitutil.IsFullSHA(sha) { return g, nil } @@ -439,7 +444,10 @@ func (g *outageGate) stopRefusal(st State) *Refusal { // CurrentOutage is the shared run's outage in force for the checkout, nil when // there is none or the checkout has no shared run state. It writes nothing. func CurrentOutage(repoRoot string) (*implement.Outage, error) { - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + return nil, fmt.Errorf("reading the repository's root commit to find the run's outage: %w", err) + } if !gitutil.IsFullSHA(sha) { return nil, nil } @@ -455,7 +463,10 @@ func CurrentOutage(repoRoot string) (*implement.Outage, error) { // for a complete run, before its last update. func runOutages(repoRoot string, st State) ([]implement.OutageSpan, error) { out := []implement.OutageSpan{} - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + return nil, fmt.Errorf("reading the repository's root commit to find the run's outages: %w", err) + } if !gitutil.IsFullSHA(sha) { return out, nil } diff --git a/internal/core/implement/loop/outage_test.go b/internal/core/implement/loop/outage_test.go index 9e4eebb6c..da0307ce2 100644 --- a/internal/core/implement/loop/outage_test.go +++ b/internal/core/implement/loop/outage_test.go @@ -274,19 +274,15 @@ func gitOut(t *testing.T, root string, args ...string) string { return string(out) } -// withGit puts git beside the fake harnesses on the drive environment's PATH: -// the loop reads the shared run's outage through the repository's root commit. +// withGit is the drive environment with git beside the fake harnesses on its +// PATH: the loop reads the shared run's outage through the repository's root +// commit. newDriveEnv now links git itself; this insists git is there. func withGit(t *testing.T, harnesses ...string) driveEnv { t.Helper() - git, err := exec.LookPath("git") - if err != nil { - t.Fatal(err) - } - env := newDriveEnv(t, "ok", harnesses...) - if err := os.Symlink(git, filepath.Join(env.bin, "git")); err != nil { + if _, err := exec.LookPath("git"); err != nil { t.Fatal(err) } - return env + return newDriveEnv(t, "ok", harnesses...) } // TestDriveStartsNoRunnerDuringModelOutage: while the model service is down diff --git a/internal/core/implement/loop/restart.go b/internal/core/implement/loop/restart.go index 8fa322c01..4e3b3e375 100644 --- a/internal/core/implement/loop/restart.go +++ b/internal/core/implement/loop/restart.go @@ -479,7 +479,11 @@ func asideDir(root *os.Root, parent string, now time.Time) (string, error) { // one the run gave up on is a stop, since every step refuses after the give-up // and the fresh agent starts only once the connection is back. func outageOpen(repoRoot, laneID string) error { - run, err := implement.Peek(gitutil.RootCommit(repoRoot)) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + return fmt.Errorf("reading the repository's root commit to find the run's outage: %w", err) + } + run, err := implement.Peek(sha) if err != nil { return err } diff --git a/internal/core/implement/loop/restart_test.go b/internal/core/implement/loop/restart_test.go index 4966f4ecb..b77a1d05e 100644 --- a/internal/core/implement/loop/restart_test.go +++ b/internal/core/implement/loop/restart_test.go @@ -12,7 +12,6 @@ import ( "github.com/intentdriven/abcd/internal/core/implement" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // deadLane is a lane whose implementer committed once and then died with work @@ -220,7 +219,7 @@ func TestRestartRecordsYieldReason(t *testing.T) { // is refused as a wait and changes nothing. func TestRestartRefusedWhileOutageOpen(t *testing.T) { repo, runID, l, dir, _ := deadLane(t) - run, err := implement.Open(gitutil.RootCommit(repo.Root())) + run, err := implement.Open(gittest.RootCommit(t, repo.Root())) if err != nil { t.Fatal(err) } @@ -249,7 +248,7 @@ func TestRestartRefusedWhileOutageOpen(t *testing.T) { // way on: clearing the outage once the connection is back. func TestRestartRefusedAfterOutageGaveUp(t *testing.T) { repo, runID, l, dir, _ := deadLane(t) - run, err := implement.Open(gitutil.RootCommit(repo.Root())) + run, err := implement.Open(gittest.RootCommit(t, repo.Root())) if err != nil { t.Fatal(err) } diff --git a/internal/core/lab/lab.go b/internal/core/lab/lab.go index 6ba6a8d03..6abaa0763 100644 --- a/internal/core/lab/lab.go +++ b/internal/core/lab/lab.go @@ -169,7 +169,12 @@ func (s store) display(parts ...string) string { // resolveStore names this repository's lane of the store without touching disk. func resolveStore(repoRoot string) (store, error) { - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + // Not ErrRefused's "no root commit": git did not answer, and the + // store is neither keyed nor read on a guess. + return store{}, fmt.Errorf("reading the repository's root commit to key the lab store on: %w", err) + } if !gitutil.IsFullSHA(sha) { return store{}, fmt.Errorf("%w: the repository has no root commit to key the lab store on (a repository with no commits has no labs)", ErrRefused) } diff --git a/internal/core/lifeboat/probe.go b/internal/core/lifeboat/probe.go index 164a1b1bf..eb0af88fa 100644 --- a/internal/core/lifeboat/probe.go +++ b/internal/core/lifeboat/probe.go @@ -208,7 +208,22 @@ func newSourceContext(repoRoot string) (*SourceContext, error) { } if gitutil.InRepo(abs) { c.isGit = true - c.rootSHA = gitutil.RootCommit(abs) + sha, err := gitutil.RootCommit(abs) + switch { + case errors.Is(err, gitutil.ErrLazyFetchFloor): + // abcd's own refusal to read objects in a partial clone below + // git 2.44, not git failing: every git read of the probe is + // refused alike, the default scan refuses on it (ignoreScopeErr), + // and the asked-for wide scan proceeds with the root unrecorded, + // so the voyage keyed on it is skipped rather than keyed on "". + case err != nil: + // The root commit is recorded in the coverage, the plan and the + // voyage; a git that did not answer stops the probe rather than + // stamping a repository as one with no root commit. + c.Close() + return nil, fmt.Errorf("reading the source repository's root commit: %w", err) + } + c.rootSHA = sha } return c, nil } diff --git a/internal/core/lifeboat/probe_test.go b/internal/core/lifeboat/probe_test.go index 4cafa13be..cdf537a73 100644 --- a/internal/core/lifeboat/probe_test.go +++ b/internal/core/lifeboat/probe_test.go @@ -11,7 +11,7 @@ import ( "strings" "testing" - "github.com/intentdriven/abcd/internal/gitutil" + "github.com/intentdriven/abcd/internal/gittest" ) // gitFixture builds an isolated git repo with the given commits applied in @@ -561,7 +561,7 @@ func TestRootCommitOctopusMerge(t *testing.T) { r.git("reset", "-q", "--hard", "main") want := strings.TrimSpace(r.git("rev-list", "-n", "1", "--max-parents=0", "HEAD")) - if got := gitutil.RootCommit(r.dir); got != want { + if got := gittest.RootCommit(t, r.dir); got != want { t.Errorf("RootCommit = %q, want %q", got, want) } } diff --git a/internal/core/memnote/store_test.go b/internal/core/memnote/store_test.go index f53493b5b..dcbe0d363 100644 --- a/internal/core/memnote/store_test.go +++ b/internal/core/memnote/store_test.go @@ -19,6 +19,7 @@ import ( "github.com/intentdriven/abcd/internal/abcdhome" "github.com/intentdriven/abcd/internal/core/recordid" "github.com/intentdriven/abcd/internal/fsutil" + "github.com/intentdriven/abcd/internal/gitutil" ) const testRoot = "0123456789abcdef0123456789abcdef01234567" @@ -29,7 +30,7 @@ func fixture(t *testing.T) (home, repo string) { t.Helper() home, repo = t.TempDir(), t.TempDir() prevRoot, prevNow := rootCommit, now - rootCommit = func(string) string { return testRoot } + rootCommit = func(string) (string, error) { return testRoot, nil } now = func() time.Time { return time.Date(2026, 10, 11, 12, 0, 0, 0, time.UTC) } t.Cleanup(func() { rootCommit, now = prevRoot, prevNow }) return home, repo @@ -134,9 +135,24 @@ func TestASessionShapedLikeALinkIsRefused(t *testing.T) { } } +// TestTiedNoteRefusesWhenGitGivesNoRoot: a root commit git did not answer +// for is not "no root commit". The write stops with git's failure, is not +// ErrNoRoot's advice to pass --everywhere, and creates nothing. +func TestTiedNoteRefusesWhenGitGivesNoRoot(t *testing.T) { + home, repo := fixture(t) + rootCommit = func(string) (string, error) { return "", gitutil.ErrNoRootAnswer } + _, err := Add(addReq(home, repo)) + if !errors.Is(err, gitutil.ErrNoRootAnswer) || errors.Is(err, ErrNoRoot) { + t.Fatalf("a tied note whose root commit git did not answer for: %v, want gitutil.ErrNoRootAnswer and not ErrNoRoot", err) + } + if _, err := os.Stat(abcdhome.Path(home)); !os.IsNotExist(err) { + t.Errorf("a refused write created the home scope (stat err %v)", err) + } +} + func TestEverywhereNoteCarriesNoRoot(t *testing.T) { home, repo := fixture(t) - rootCommit = func(string) string { return "" } + rootCommit = func(string) (string, error) { return "", nil } req := addReq(home, repo) if _, err := Add(req); !errors.Is(err, ErrNoRoot) { t.Fatalf("a tied note with no root commit: %v, want ErrNoRoot", err) diff --git a/internal/core/memnote/write.go b/internal/core/memnote/write.go index 29fdc02f8..93d34566c 100644 --- a/internal/core/memnote/write.go +++ b/internal/core/memnote/write.go @@ -75,7 +75,8 @@ const machineLeaf = "memory.json" // Add judges and writes one note to the machine layer and returns it. Every // refusal comes before anything is created: a malformed field (*FieldError), -// a tied note with no root commit (ErrNoRoot), the gate (*GateError, or an +// a tied note with no root commit (ErrNoRoot) or one whose root commit git did +// not answer for (gitutil.ErrNoRootAnswer, wrapped), the gate (*GateError, or an // error when the gate cannot judge), or a machine file the guarded read // refuses. func Add(req AddRequest) (Note, error) { @@ -96,7 +97,12 @@ func Add(req AddRequest) (Note, error) { n.Session = &s } if !req.Everywhere { - root := rootCommit(req.RepoRoot) + root, err := rootCommit(req.RepoRoot) + if err != nil { + // git did not answer: the repository may well have a root + // commit, so this is not ErrNoRoot, and nothing is written. + return Note{}, fmt.Errorf("reading this repository's root commit to tie the note to: %w", err) + } if !gitutil.IsFullSHA(root) { return Note{}, ErrNoRoot } diff --git a/internal/core/peers/peers_test.go b/internal/core/peers/peers_test.go index 1ee573ad1..a02ad2cb1 100644 --- a/internal/core/peers/peers_test.go +++ b/internal/core/peers/peers_test.go @@ -11,7 +11,6 @@ import ( "github.com/intentdriven/abcd/internal/core/embargo/storepath" "github.com/intentdriven/abcd/internal/core/peers" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // fixture is one repository ("here" is its main checkout) with worktrees laid @@ -556,7 +555,7 @@ func TestAGoneWorktreeOnAMergedBranchIsCountedOnce(t *testing.T) { // location or the count. func TestPeersSkipsEmbargoStore(t *testing.T) { f := newFixture(t) - store := abcdhome.Path(f.home, "worktrees", gitutil.RootCommit(f.here()), storepath.Name) + store := abcdhome.Path(f.home, "worktrees", gittest.RootCommit(f.t, f.here()), storepath.Name) if err := os.MkdirAll(filepath.Dir(store), 0o700); err != nil { t.Fatal(err) } diff --git a/internal/core/peers/storeroots_test.go b/internal/core/peers/storeroots_test.go index aa7be2f35..38d872908 100644 --- a/internal/core/peers/storeroots_test.go +++ b/internal/core/peers/storeroots_test.go @@ -11,7 +11,6 @@ import ( "github.com/intentdriven/abcd/internal/core/embargo/storepath" "github.com/intentdriven/abcd/internal/core/peers" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // mergeUnrelatedRoot merges, with an empty diff, a root commit of an @@ -29,7 +28,7 @@ func (f *fixture) mergeUnrelatedRoot() string { } other := strings.TrimSpace(string(out)) f.git(f.here(), "merge", "-q", "-s", "ours", "--allow-unrelated-histories", "--no-edit", other) - if gitutil.RootCommit(f.here()) != other { + if gittest.RootCommit(f.t, f.here()) != other { f.t.Fatal("the fixture's rev-list does not meet the merged root first") } return other @@ -72,7 +71,7 @@ func (f *fixture) wantStoresSkipped(n int) { // root commit. func (f *fixture) ownStore() string { f.t.Helper() - return abcdhome.Path(f.home, "worktrees", gitutil.RootCommit(f.here()), storepath.Name) + return abcdhome.Path(f.home, "worktrees", gittest.RootCommit(f.t, f.here()), storepath.Name) } // TestPeersSkipsEmbargoStoreWhicheverRootGitListsFirst: after an empty-diff diff --git a/internal/core/recall/items.go b/internal/core/recall/items.go index e663a9a81..669ce46fa 100644 --- a/internal/core/recall/items.go +++ b/internal/core/recall/items.go @@ -47,7 +47,7 @@ type Item struct { // recall asks it only once matching has found a tied item, and at most once // per call, so a prompt that matches no tied item costs no git call. A caller // that hands the same RootFunc to more than one call memoises it -// (sync.OnceValues). An error is git not answering in time +// (sync.OnceValues). An error is git not answering, in time or at all // (gitutil.RootCommitContext): the tied items are then left out. type RootFunc func() (string, error) diff --git a/internal/core/report/inbox.go b/internal/core/report/inbox.go index 7dab1657e..dbce9f297 100644 --- a/internal/core/report/inbox.go +++ b/internal/core/report/inbox.go @@ -103,7 +103,10 @@ type Sender struct { // reports under its repository's name, not its own directory's). A repository // with no commit has no key and cannot report. func SenderOf(root string) (Sender, error) { - key := gitutil.RootCommit(root) + key, err := gitutil.RootCommit(root) + if err != nil { + return Sender{}, fmt.Errorf("reading the repository's root commit to key the report on: %w", err) + } if !gitutil.IsFullSHA(key) { return Sender{}, fmt.Errorf("%w: the repository has no root commit to key the report on (a repository with no commits cannot report)", ErrRefused) } @@ -572,7 +575,11 @@ func Promote(ledgerRoot, id string, mc *match.Config) (Promoted, error) { if m == nil { return Promoted{}, fmt.Errorf("%w: %q is not a report id (rpt- and sixteen digits)", ErrRefused, id) } - if root := gitutil.RootCommit(ledgerRoot); root != abcdRootCommit { + root, err := gitutil.RootCommit(ledgerRoot) + if err != nil { + return Promoted{}, fmt.Errorf("reading this repository's root commit, which must be abcd's to promote a report: %w", err) + } + if root != abcdRootCommit { here := "has no root commit" if root != "" { here = "has root commit " + root diff --git a/internal/core/report/inbox_test.go b/internal/core/report/inbox_test.go index 61fe63a92..650b70dde 100644 --- a/internal/core/report/inbox_test.go +++ b/internal/core/report/inbox_test.go @@ -16,7 +16,6 @@ import ( "github.com/intentdriven/abcd/internal/core/lint" "github.com/intentdriven/abcd/internal/core/recordid" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // sandbox points HOME at a fresh directory, so the inbox the test sees is its @@ -51,7 +50,7 @@ func committedRepo(t *testing.T) *gittest.Repo { func abcdCheckout(t *testing.T) *gittest.Repo { t.Helper() r := committedRepo(t) - t.Cleanup(SetAbcdRootCommitForTest(gitutil.RootCommit(r.Root()))) + t.Cleanup(SetAbcdRootCommitForTest(gittest.RootCommit(t, r.Root()))) return r } @@ -391,7 +390,7 @@ func TestPromoteRetryAfterAFailedMoveFilesOneCapture(t *testing.T) { // holds this test: the root commit is abcd's identity, and a constant that // drifted from it would refuse every promotion in abcd's own checkout. func TestAbcdRootCommitIsThisCheckouts(t *testing.T) { - root := gitutil.RootCommit(".") + root := gittest.RootCommit(t, ".") if root == "" { t.Skip("not run from a git checkout (an archive copy); nothing to pin against") } diff --git a/internal/core/site/build.go b/internal/core/site/build.go index 6bcc8b9da..6fe952417 100644 --- a/internal/core/site/build.go +++ b/internal/core/site/build.go @@ -380,7 +380,13 @@ func build(req Request, written *labelsAdded) (Result, error) { // for nothing. The PURGE waits, though: it happens at the first write, so a // failure anywhere in between leaves the previous output standing rather than // clearing a good tree in exchange for one that never arrived. - rootCommit := gitutil.RootCommit(repoRoot) + rootCommit, err := gitutil.RootCommit(repoRoot) + if err != nil { + // Never the identity-less build: that one writes an "unknown" marker + // and refuses a non-empty directory as a repository with no root + // commit, neither of which a git that did not answer is evidence of. + return Result{}, fmt.Errorf("site: reading this repository's root commit, which names the build in its %s: %w", siteMarkerName, err) + } outState, err := inspectOutDir(outDir, rootCommit) if err != nil { return Result{}, err diff --git a/internal/core/site/setup.go b/internal/core/site/setup.go index a579d567b..77ed33c45 100644 --- a/internal/core/site/setup.go +++ b/internal/core/site/setup.go @@ -201,6 +201,11 @@ func Setup(req SetupRequest) (SetupResult, error) { if err != nil { return SetupResult{}, fmt.Errorf("site setup: could not classify this folder: %w", err) } + if rerr := det.RepoIdentity.RootErr(); det.RootSHA == "" && rerr != nil { + // git not answering is not a folder outside abcd: say which it was, + // and still set nothing up (iss-2610051434156079). + return SetupResult{}, fmt.Errorf("site setup: git gave no root commit for this repository, so abcd cannot tell whether it manages it: %w", rerr) + } if det.FolderKind != ahoy.ManagedRepo { return SetupResult{}, ErrNotManaged } diff --git a/internal/gittest/rootcommit.go b/internal/gittest/rootcommit.go new file mode 100644 index 000000000..0a914a29f --- /dev/null +++ b/internal/gittest/rootcommit.go @@ -0,0 +1,20 @@ +package gittest + +import ( + "testing" + + "github.com/intentdriven/abcd/internal/gitutil" +) + +// RootCommit is gitutil.RootCommit for a fixture: the root commit of the +// repository at dir, "" for none, and a failed test when git did not answer. +// A fixture's identity is never in doubt, so a lookup that errors is the test +// machine's fault and is said as such rather than read as "no root commit". +func RootCommit(t *testing.T, dir string) string { + t.Helper() + sha, err := gitutil.RootCommit(dir) + if err != nil { + t.Fatalf("gittest: the root commit of %s: %v", dir, err) + } + return sha +} diff --git a/internal/gitutil/context_test.go b/internal/gitutil/context_test.go index 5a58ba57b..4d0df3cab 100644 --- a/internal/gitutil/context_test.go +++ b/internal/gitutil/context_test.go @@ -39,11 +39,17 @@ func hangingGit(t *testing.T) (pidFile string) { // gone — killed, not abandoned to finish on its own. func TestRootCommitContextKillsGitAtTheDeadline(t *testing.T) { pidFile := hangingGit(t) + // Repository-shaped, so the lookup asks git: a directory with no .git + // entry is answered without it. + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, ".git"), 0o755); err != nil { + t.Fatal(err) + } ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() start := time.Now() - sha, err := gitutil.RootCommitContext(ctx, t.TempDir()) + sha, err := gitutil.RootCommitContext(ctx, dir) took := time.Since(start) if sha != "" || !errors.Is(err, context.DeadlineExceeded) { t.Errorf("RootCommitContext = %q, %v; want \"\" and context.DeadlineExceeded", sha, err) @@ -85,7 +91,7 @@ func TestRootCommitContextAnswersAsRootCommitDoes(t *testing.T) { t.Skipf("git %v unavailable: %v (%s)", args, err, out) } } - want := gitutil.RootCommit(repo) + want := gittest.RootCommit(t, repo) if !gitutil.IsFullSHA(want) { t.Fatalf("fixture: RootCommit = %q", want) } diff --git a/internal/gitutil/repo.go b/internal/gitutil/repo.go index c267401ca..0f0438b02 100644 --- a/internal/gitutil/repo.go +++ b/internal/gitutil/repo.go @@ -362,31 +362,46 @@ func IsAncestor(root, ancestor, descendant string) (bool, error) { return false, fmt.Errorf("%w (stderr: %q)", err, strings.TrimSpace(string(e.buf))) } -// RootCommit returns the repository's canonical (first) root-commit SHA, or "" -// when it cannot be derived — git absent, not a repository, no commits. It is -// total: a caller keying a store or a marker on the repository's identity gets -// "" rather than an error, and decides for itself what an unidentified -// repository means. A repository can have several root commits (an octopus of -// unrelated histories); the first `rev-list` reports is the canonical one, the -// same choice the history registry and the lifeboat probe make. +// ErrNoRootAnswer is a root-commit lookup that got no answer from git for a +// directory that is a repository by its shape: git did not start, was killed, +// exited in a way that names neither a root commit nor an empty history, or +// refused the repository. It is never "this repository has no root commit", +// which is ("", nil) (iss-2610051434156079). +var ErrNoRootAnswer = errors.New("git gave no root commit for a repository") + +// RootCommit returns the repository's canonical (first) root-commit SHA. A +// repository can have several root commits (an octopus of unrelated +// histories); the first `rev-list` reports is the canonical one, the same +// choice the history registry and the lifeboat probe make. +// +// It returns ("", nil) only for the two definite answers: root names no +// directory or no repository (nothing at or above it carries a .git entry), +// or the repository has no commits yet. Every other failure is an error a +// caller must handle: ErrNoRootAnswer for a git that did not answer for a +// repository-shaped directory, ErrGitTimedOut for one whose output missed the +// runner's wait twice. A failed git call is never read as an answer, since a +// caller keying a store on the identity would otherwise take a real +// repository for none (iss-2610051434156079). // // The output is bounded (one object name, so the cap is generous) rather than // buffered whole: a hostile repository must not be able to make an identity // probe allocate. It has no deadline, so it waits for git's answer however // loaded the machine is (contextWaitDelay): a slow git is never read as a // repository with no root commit (iss-2610100846469473). -func RootCommit(root string) string { - sha, _ := RootCommitContext(context.Background(), root) - return sha +func RootCommit(root string) (string, error) { + return RootCommitContext(context.Background(), root) } -// RootCommitContext is RootCommit bound to ctx: the same answer, total in the -// same way, except that git not answering in time is returned as its own -// error — a context that ends before git answers (wrapped, with git killed), -// or a git whose output missed the runner's WaitDelay twice (ErrGitTimedOut) — -// so a caller can tell "this repository has no root commit" from "git did not -// answer in time". +// RootCommitContext is RootCommit bound to ctx: the same answers, and a +// context that ends before git answers is returned as that context's error, +// with git killed. func RootCommitContext(ctx context.Context, root string) (string, error) { + if _, err := os.Stat(root); errors.Is(err, os.ErrNotExist) { + return "", nil + } + if !RepoShaped(root) { + return "", nil + } out, err := RunLimitedContext(ctx, root, 4096, "rev-list", "-n", "1", "--max-parents=0", "HEAD") if ctxErr := ctx.Err(); ctxErr != nil { return "", ctxErr @@ -395,7 +410,12 @@ func RootCommitContext(ctx context.Context, root string) (string, error) { if errors.Is(err, ErrGitTimedOut) { return "", err } - return "", nil + if emptyHistory(ctx, root, err) { + return "", nil + } + // The cause stays in the chain (%w), so a caller can still tell + // a deliberate refusal, such as ErrLazyFetchFloor, from git failing. + return "", fmt.Errorf("%w: %s: %w", ErrNoRootAnswer, root, err) } fields := strings.Fields(out) if len(fields) == 0 { @@ -404,6 +424,20 @@ func RootCommitContext(ctx context.Context, root string) (string, error) { return fields[0], nil } +// emptyHistory reports whether a failed root-commit lookup failed because the +// repository has no commits yet. git exits 128 for that and for every other +// fatal error alike, so it is asked directly: `rev-parse --verify -q HEAD` +// exits 1, quietly, for an unborn HEAD in a repository git answers for. Any +// other outcome, including git not running at all, is not an empty history. +func emptyHistory(ctx context.Context, root string, lookupErr error) bool { + var ee *exec.ExitError + if !errors.As(lookupErr, &ee) || ee.ExitCode() != 128 { + return false + } + _, err := RunLimitedContext(ctx, root, 4096, "rev-parse", "--verify", "-q", "HEAD") + return errors.As(err, &ee) && ee.ExitCode() == 1 +} + // fullSHARe is a full object name: forty hex digits under SHA-1, sixty-four // under SHA-256, lower case as git prints them. var fullSHARe = regexp.MustCompile(`^(?:[0-9a-f]{40}|[0-9a-f]{64})$`) diff --git a/internal/gitutil/repo_test.go b/internal/gitutil/repo_test.go index b4d25edbf..50d162757 100644 --- a/internal/gitutil/repo_test.go +++ b/internal/gitutil/repo_test.go @@ -226,12 +226,12 @@ func TestTrackedFilesRepoShapedButUnanswerable(t *testing.T) { } // TestRootCommit pins the identity probe: the first root commit of a -// repository with history, and "" — never an error — outside one or before the -// first commit. +// repository with history, and ("", nil) outside one or before the first +// commit. func TestRootCommit(t *testing.T) { repo := newRepo(t, "") - if got := gitutil.RootCommit(repo); got != "" { - t.Errorf("RootCommit(repo with no commits) = %q, want empty", got) + if got, err := gitutil.RootCommit(repo); got != "" || err != nil { + t.Errorf("RootCommit(repo with no commits) = %q, %v; want \"\", nil", got, err) } if err := os.WriteFile(filepath.Join(repo, "a.txt"), []byte("a\n"), 0o644); err != nil { t.Fatal(err) @@ -245,10 +245,10 @@ func TestRootCommit(t *testing.T) { if err != nil { t.Fatal(err) } - if got := gitutil.RootCommit(repo); got != strings.TrimSpace(want) { - t.Errorf("RootCommit = %q, want %q", got, strings.TrimSpace(want)) + if got, err := gitutil.RootCommit(repo); got != strings.TrimSpace(want) || err != nil { + t.Errorf("RootCommit = %q, %v; want %q, nil", got, err, strings.TrimSpace(want)) } - if got := gitutil.RootCommit(t.TempDir()); got != "" { - t.Errorf("RootCommit(non-repo) = %q, want empty", got) + if got, err := gitutil.RootCommit(t.TempDir()); got != "" || err != nil { + t.Errorf("RootCommit(non-repo) = %q, %v; want \"\", nil", got, err) } } diff --git a/internal/gitutil/rootcommit_test.go b/internal/gitutil/rootcommit_test.go new file mode 100644 index 000000000..c48dae4be --- /dev/null +++ b/internal/gitutil/rootcommit_test.go @@ -0,0 +1,121 @@ +package gitutil_test + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" + "github.com/intentdriven/abcd/internal/gitutil" +) + +// iss-2610051434156079: the root-commit lookup read every failed git call as +// "no root commit", so a caller keying a store on the identity took a real +// repository for none. These pin which answers are definite ("", nil) and +// which are git not answering (an error). They ask RootCommitContext, whose +// signature the change kept, so each was run against the old body too. + +// TestRootCommitOfAMissingDirectoryIsNone: a directory that is not there has +// no root commit, definitely. +func TestRootCommitOfAMissingDirectoryIsNone(t *testing.T) { + gittest.Env(t) + got, err := gitutil.RootCommitContext(context.Background(), filepath.Join(t.TempDir(), "gone")) + if got != "" || err != nil { + t.Errorf("RootCommitContext(missing directory) = %q, %v; want \"\", nil", got, err) + } +} + +// TestRootCommitOutsideAnyRepositoryIsNone: a directory with no .git entry at +// or above it has no root commit, definitely, and git is not asked. +func TestRootCommitOutsideAnyRepositoryIsNone(t *testing.T) { + gittest.Env(t) + gittest.ShimGit(t, []string{"--max-parents=0"}, "", 128) + got, err := gitutil.RootCommitContext(context.Background(), t.TempDir()) + if got != "" || err != nil { + t.Errorf("RootCommitContext(no .git above) = %q, %v; want \"\", nil", got, err) + } +} + +// TestRootCommitOfAnEmptyRepositoryIsNone: a repository with no commits yet +// has no root commit, definitely — git's own exit 128 for an unborn HEAD is +// told apart from git failing. +func TestRootCommitOfAnEmptyRepositoryIsNone(t *testing.T) { + r := gittest.NewRepo(t) + got, err := gitutil.RootCommitContext(context.Background(), r.Root()) + if got != "" || err != nil { + t.Errorf("RootCommitContext(no commits) = %q, %v; want \"\", nil", got, err) + } +} + +// TestRootCommitOfARepositoryIsItsSHA: a repository with history answers +// with its root commit. +func TestRootCommitOfARepositoryIsItsSHA(t *testing.T) { + r := gittest.NewRepo(t) + r.Write("a.txt", "a\n") + r.Commit("one") + r.Write("b.txt", "b\n") + r.Commit("two") + want := r.Git("rev-list", "--max-parents=0", "HEAD") + got, err := gitutil.RootCommitContext(context.Background(), r.Root()) + if got != want || err != nil { + t.Errorf("RootCommitContext = %q, %v; want %q, nil", got, err, want) + } +} + +// TestRootCommitNamesAGitThatFailsForARepository: git exiting 128 for a +// repository that has commits (a fork refused on a loaded machine, a git +// killed early) is ErrNoRootAnswer, never ("", nil). +func TestRootCommitNamesAGitThatFailsForARepository(t *testing.T) { + r := gittest.NewRepo(t) + r.Write("a.txt", "a\n") + r.Commit("one") + gittest.ShimGit(t, []string{"--max-parents=0"}, "", 128) + got, err := gitutil.RootCommitContext(context.Background(), r.Root()) + if got != "" || !errors.Is(err, gitutil.ErrNoRootAnswer) { + t.Errorf("RootCommitContext(git exits 128) = %q, %v; want \"\" and ErrNoRootAnswer", got, err) + } +} + +// TestRootCommitNamesAGitThatCannotStart: a git that never runs — no +// executable git on PATH, or one killed by a signal — is ErrNoRootAnswer for +// a repository-shaped directory. +func TestRootCommitNamesAGitThatCannotStart(t *testing.T) { + shaped := func(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, ".git"), 0o755); err != nil { + t.Fatal(err) + } + return dir + } + t.Run("no executable git on PATH", func(t *testing.T) { + gittest.Env(t) + bin := t.TempDir() + if err := os.WriteFile(filepath.Join(bin, "git"), []byte("#!/bin/sh\nexit 0\n"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin) + got, err := gitutil.RootCommitContext(context.Background(), shaped(t)) + if got != "" || !errors.Is(err, gitutil.ErrNoRootAnswer) { + t.Errorf("RootCommitContext(git not runnable) = %q, %v; want \"\" and ErrNoRootAnswer", got, err) + } + }) + t.Run("a git that kills itself", func(t *testing.T) { + gittest.Env(t) + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("no sh") + } + bin := t.TempDir() + if err := os.WriteFile(filepath.Join(bin, "git"), []byte("#!/bin/sh\nkill -9 $$\n"), 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + got, err := gitutil.RootCommitContext(context.Background(), shaped(t)) + if got != "" || !errors.Is(err, gitutil.ErrNoRootAnswer) { + t.Errorf("RootCommitContext(git killed) = %q, %v; want \"\" and ErrNoRootAnswer", got, err) + } + }) +} diff --git a/internal/gitutil/roots_test.go b/internal/gitutil/roots_test.go index 1e4fa64bf..a473ca3a9 100644 --- a/internal/gitutil/roots_test.go +++ b/internal/gitutil/roots_test.go @@ -41,7 +41,7 @@ func TestDefaultBranchRootsListsEveryRoot(t *testing.T) { t.Run("an ordinary repository lists its one root", func(t *testing.T) { r, root := setup(t) got, err := gitutil.DefaultBranchRoots(r.Root()) - if err != nil || !slices.Equal(got, []string{root}) || root != gitutil.RootCommit(r.Root()) { + if err != nil || !slices.Equal(got, []string{root}) || root != gittest.RootCommit(t, r.Root()) { t.Fatalf("DefaultBranchRoots = %q, %v; want [%s], RootCommit's answer", got, err, root) } }) @@ -49,7 +49,7 @@ func TestDefaultBranchRootsListsEveryRoot(t *testing.T) { r, root := setup(t) other := datedRoot(t, r, "2090-01-01T00:00:00Z", "an unrelated root") r.Git("merge", "-q", "-s", "ours", "--allow-unrelated-histories", "--no-edit", other) - if gitutil.RootCommit(r.Root()) != other { + if gittest.RootCommit(t, r.Root()) != other { t.Fatal("the fixture's rev-list does not meet the merged root first") } got, err := gitutil.DefaultBranchRoots(r.Root()) diff --git a/internal/gitutil/waitdelay_test.go b/internal/gitutil/waitdelay_test.go index 1be1fbe75..0480be339 100644 --- a/internal/gitutil/waitdelay_test.go +++ b/internal/gitutil/waitdelay_test.go @@ -40,8 +40,8 @@ func TestRootCommitWaitsOutAGitThatHoldsItsPipe(t *testing.T) { repo, want, _ := committedRepo(t) gittest.SlowPipeGit(t, "--max-parents=0", -1) - if got := gitutil.RootCommit(repo.Root()); got != want { - t.Errorf("RootCommit = %q; want %q", got, want) + if got, err := gitutil.RootCommit(repo.Root()); got != want || err != nil { + t.Errorf("RootCommit = %q, %v; want %q, nil", got, err, want) } } diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index fbf20454c..18631004a 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -1987,6 +1987,17 @@ and the --json envelope carries the name as session_title.`, } } det, err := ahoy.Detect(cwd) + if err == nil && det.RootSHA == "" && det.RepoIdentity.RootErr() != nil { + // git failing once on a loaded machine is not a directory + // outside any repository: ask again before giving the session + // up, as the sub-agent hook does (iss-2610051434156079). + det, err = retryDetect(cmd.ErrOrStderr(), cwd, det) + } + if err == nil && det.RootSHA == "" { + if rerr := det.RepoIdentity.RootErr(); rerr != nil { + return warn("git gave no root commit for %q (%s); capturing nothing", cwd, fsutil.RedactHome(rerr.Error())) + } + } if err != nil || det.RootSHA == "" { return warn("cannot resolve the repo's root-commit SHA from %q; capturing nothing", cwd) } @@ -2008,12 +2019,17 @@ and the --json envelope carries the name as session_title.`, fmt.Fprintf(cmd.ErrOrStderr(), "abcd %s\n", termsafe.Sanitize(n)) } } - res, err := history.Stage(captureRoot(cwd), det.RootSHA, history.StageMeta{ + res, err := stageWithRetry(cmd.ErrOrStderr(), captureRoot(cwd), det.RootSHA, history.StageMeta{ Lineage: history.CaptureMeta{SessionID: in.SessionID, Kind: "native", LineageSource: "hook"}, SourcePath: in.TranscriptPath, }, raw) if err != nil { - return warn("staging failed (%v); this session was not captured", err) + // The marker is where the person sees the loss: this hook exits + // 0, and its stderr reaches only the host's debug log. + if nerr := history.NoteStageMiss(captureRoot(cwd), det.RootSHA, "session "+in.SessionID, err.Error()); nerr != nil { + diagnosticLine(cmd.ErrOrStderr(), "abcd history: the missed stage could not be recorded either (%v)", nerr) + } + return warn("staging failed (%v); this session was not captured, and `abcd history staged` reports it", err) } staged := hookStageResult{Hook: "session-end", Captured: true, SessionID: res.Staged.SessionID, Bytes: res.Staged.Bytes} if !res.Wrote { @@ -2087,6 +2103,14 @@ and the --json envelope carries the name as session_title.`, // it leaves is said out loud rather than dropped, so a partial pass // never reads as a complete one. det, detErr := ahoy.Detect(cwd) + if rerr := det.RepoIdentity.RootErr(); detErr == nil && rerr != nil { + // git did not answer for this repository: nothing is + // drained or noted this start, and it is said, rather than + // read as a folder with no store (iss-2610051434156079). + notices = append(notices, fmt.Sprintf( + "abcd: git gave no root commit for this repository (%s), so staged transcripts were not drained this time; `abcd history drain` finishes them.", + termsafe.Sanitize(fsutil.RedactHome(rerr.Error())))) + } if detErr == nil && det.RootSHA != "" { currentSHA = det.RootSHA // Record which store this session belongs to while a real @@ -2331,8 +2355,8 @@ func drainWhileLive(cmd *cobra.Command, cwd string) rootLookup { rootSHA, rootErr := boundedRootCommit(cwd) looked := rootLookup{sha: rootSHA, err: rootErr, asked: true} if rootErr != nil { - // git did not answer in time: nothing is drained this turn, and the - // staged transcripts wait for the next one. + // git did not answer, in time or at all: nothing is drained this + // turn, and the staged transcripts wait for the next one. diagnosticLine(cmd.ErrOrStderr(), "abcd history: this repository's root commit: %s; draining no staged transcripts this turn", fsutil.RedactHome(rootErr.Error())) return looked @@ -6724,6 +6748,9 @@ func repoRootSHA() (string, error) { if err != nil { return "", err } + if rerr := det.RepoIdentity.RootErr(); rerr != nil { + return "", fmt.Errorf("history: cannot read the repo's root-commit SHA: %w", rerr) + } if det.RootSHA == "" { return "", fmt.Errorf("history: cannot resolve the repo's root-commit SHA (not a git repo with commits)") } diff --git a/internal/surface/cli/dashboard.go b/internal/surface/cli/dashboard.go index 61080d901..f2d5f5e30 100644 --- a/internal/surface/cli/dashboard.go +++ b/internal/surface/cli/dashboard.go @@ -86,7 +86,11 @@ func newDashboardStartCommand(asJSON *bool) *cobra.Command { if err != nil { return &exitError{Code: 2, Msg: prefix + err.Error() + " (nothing started)"} } - if !ahoy.Managed(root) { + managed, merr := ahoy.ManagedContext(cmd.Context(), root) + if merr != nil { + return &exitError{Code: 1, Msg: prefix + "whether abcd manages this checkout could not be read (" + scrubPaths(merr) + "); try again (nothing started)"} + } + if !managed { return &exitError{Code: 2, Msg: prefix + "this checkout is not one abcd manages; run `abcd ahoy install` first (nothing started)"} } bin, err := tailscale.Resolve(exec.LookPath, tailscale.BundledCommands, root) diff --git a/internal/surface/cli/filing_match_surface_test.go b/internal/surface/cli/filing_match_surface_test.go index 27d7070bf..ee29617c1 100644 --- a/internal/surface/cli/filing_match_surface_test.go +++ b/internal/surface/cli/filing_match_surface_test.go @@ -8,7 +8,7 @@ import ( "github.com/intentdriven/abcd/internal/core/capture" "github.com/intentdriven/abcd/internal/core/issueschema" "github.com/intentdriven/abcd/internal/core/report" - "github.com/intentdriven/abcd/internal/gitutil" + "github.com/intentdriven/abcd/internal/gittest" ) // filing_match_surface_test.go is the wiring proof for the filing-time match @@ -42,7 +42,7 @@ func TestInboxPromoteMatchesTheLedger(t *testing.T) { t.Fatalf("report did not name an id:\n%s", filed) } id := filed[i : i+20] - t.Cleanup(report.SetAbcdRootCommitForTest(gitutil.RootCommit(repo))) + t.Cleanup(report.SetAbcdRootCommitForTest(gittest.RootCommit(t, repo))) out := string(runCLI(t, "inbox", "promote", id)) if !strings.Contains(out, "matched "+held.ID) || !strings.Contains(out, "link written") { diff --git a/internal/surface/cli/history.go b/internal/surface/cli/history.go index 2ca5cf976..d66b76bb9 100644 --- a/internal/surface/cli/history.go +++ b/internal/surface/cli/history.go @@ -251,13 +251,13 @@ func newHistoryCommand(asJSON *bool) *cobra.Command { "abcd history: the sub-agent payload marker is unreadable (%s)\n", termsafe.Sanitize(gapErr.Error())) } - // The stage-miss marker counts sub-agent transcripts a hook read and + // The stage-miss marker counts transcripts a staging hook read and // could not stage (iss-2610051434156079): the hook exits 0, so this // is the only place the person can see such a loss. miss, hasMiss, missErr := history.StageMiss(repoRoot, rootSHA) if missErr != nil { fmt.Fprintf(cmd.ErrOrStderr(), - "abcd history: the sub-agent stage-miss marker is unreadable (%s)\n", + "abcd history: the stage-miss marker is unreadable (%s)\n", termsafe.Sanitize(missErr.Error())) } if staged == nil { @@ -284,9 +284,9 @@ func newHistoryCommand(asJSON *bool) *cobra.Command { gap.FirstSeen.Format("2006-01-02T15:04:05Z")) } if hasMiss { - fmt.Fprintf(w, "\nNOTE: %d sub-agent transcript(s) were read but could not be staged, so they are not in this store (first %s, last %s, agent %s: %s).\n", + fmt.Fprintf(w, "\nNOTE: %d transcript(s) were read but could not be staged, so they are not in this store (first %s, last %s, %s: %s).\n", miss.Count, miss.FirstSeen.Format("2006-01-02T15:04:05Z"), miss.LastSeen.Format("2006-01-02T15:04:05Z"), - termsafe.Sanitize(miss.LastAgent), termsafe.Sanitize(fsutil.RedactHome(miss.LastReason))) + termsafe.Sanitize(miss.LastWho), termsafe.Sanitize(fsutil.RedactHome(miss.LastReason))) } }() if len(staged) == 0 && len(quarantined) == 0 { diff --git a/internal/surface/cli/history_recovery.go b/internal/surface/cli/history_recovery.go index 57572007a..33430b30f 100644 --- a/internal/surface/cli/history_recovery.go +++ b/internal/surface/cli/history_recovery.go @@ -218,6 +218,10 @@ func resolveDestination(into string) (history.Destination, error) { if err != nil { return history.Destination{}, err } + if rerr := det.RepoIdentity.RootErr(); rerr != nil { + return history.Destination{}, fmt.Errorf("history ingest: the root commit of %s could not be read, so it has no store key this run: %w", + termsafe.Sanitize(fsutil.DisplayPath(abs)), rerr) + } if det.RootSHA == "" { return history.Destination{}, fmt.Errorf("history ingest: %s is not a git repository with commits, so it has no store key", termsafe.Sanitize(fsutil.DisplayPath(abs))) diff --git a/internal/surface/cli/hook_subagent.go b/internal/surface/cli/hook_subagent.go index e31747851..a163ab5f9 100644 --- a/internal/surface/cli/hook_subagent.go +++ b/internal/surface/cli/hook_subagent.go @@ -38,7 +38,6 @@ import ( "github.com/intentdriven/abcd/internal/core/ahoy" "github.com/intentdriven/abcd/internal/core/history" "github.com/intentdriven/abcd/internal/fsutil" - "github.com/intentdriven/abcd/internal/gitutil" "github.com/intentdriven/abcd/internal/termsafe" ) @@ -116,7 +115,7 @@ func newSubagentStopCommand() *cobra.Command { return warn("unreadable SubagentStop payload (%v); staging nothing", err) } in = parsed - repoRoot, rootSHA, via := resolveSubagentStore(in) + repoRoot, rootSHA, via, rootErr := resolveSubagentStore(in) // The absent-field case comes before the store check has any // consequence, but after it has been attempted: the marker needs a @@ -132,14 +131,14 @@ func newSubagentStopCommand() *cobra.Command { return warn("this harness fired %s with no agent_transcript_path, so no sub-agent transcript can be captured here; `abcd history staged` reports this", orDefault(in.Event, "SubagentStop")) } - if rootSHA == "" { - // A directory that carries a .git entry but gave no root - // commit is a git that failed this once (a fork refused on a - // loaded machine, a git that exited early), not a directory - // outside any repository. It is asked again, and the session - // note re-read between tries, since a burst's siblings write it - // as they resolve (iss-2610051434156079). - repoRoot, rootSHA, via = retrySubagentStore(cmd.ErrOrStderr(), in) + if rootSHA == "" && rootErr != nil { + // git did not answer for the cwd's root commit (a fork refused + // on a loaded machine, a git that exited early): that is a git + // that failed this once, not a directory outside any + // repository. It is asked again, and the session note re-read + // between tries, since a burst's siblings write it as they + // resolve (iss-2610051434156079). + repoRoot, rootSHA, via = retrySubagentStore(cmd.ErrOrStderr(), in, rootErr) } if rootSHA == "" { return warn("cannot resolve the repository for this sub-agent — the payload's cwd %q did not resolve and no store has seen session %q; staging nothing", @@ -168,7 +167,7 @@ func newSubagentStopCommand() *cobra.Command { // The transcript was read and is lost. The marker is what lets // the person see that: this hook's stderr reaches only a debug // log, since it must exit 0. - if nerr := history.NoteStageMiss(repoRoot, rootSHA, in.AgentID, err.Error()); nerr != nil { + if nerr := history.NoteStageMiss(repoRoot, rootSHA, "sub-agent "+in.AgentID, err.Error()); nerr != nil { diagnosticLine(cmd.ErrOrStderr(), "abcd history: the missed stage could not be recorded either (%v)", nerr) } return warn("staging sub-agent %s failed (%v); this transcript was not captured, and `abcd history staged` reports it", in.AgentID, err) @@ -219,49 +218,65 @@ func newSubagentStopCommand() *cobra.Command { // written into the resolved lane rather than a fixed path — a lane the lookup // cannot enumerate is one it correctly reports as not having seen the session, // instead of staging the transcript into a second store nothing drains. -func resolveSubagentStore(in hookInput) (repoRoot, rootSHA, via string) { +// +// rootErr is why the cwd gave no root commit when git did not answer for it +// (ahoy.RepoIdentity.RootErr), nil when the cwd's answer was definite: it is +// the signal that a second try could find what the first did not. +func resolveSubagentStore(in hookInput) (repoRoot, rootSHA, via string, rootErr error) { cwd := in.Cwd if cwd == "" { if wd, err := os.Getwd(); err == nil { cwd = wd } } - if det, err := ahoy.Detect(cwd); err == nil && det.RootSHA != "" { + det, err := ahoy.Detect(cwd) + if err == nil && det.RootSHA != "" { root := captureRoot(cwd) if in.SessionID != "" { // Best effort: a failure here degrades a fallback, never a capture. _ = history.NoteSessionRepo(root, det.RootSHA, in.SessionID) } - return root, det.RootSHA, "cwd" + return root, det.RootSHA, "cwd", nil + } + if err == nil { + rootErr = det.RepoIdentity.RootErr() } if in.SessionID != "" { if sha, err := history.SessionRepo(in.SessionID); err == nil { - return "", sha, "session" + return "", sha, "session", nil } } - return "", "", "" + return "", "", "", rootErr } -// retrySubagentStore asks again, after resolveSubagentStore found no store, -// when the payload's cwd carries a .git entry: git failing once is a loaded -// machine, not a directory outside any repository, and a burst's siblings -// write the session note as they resolve, so it is re-read between tries. A -// cwd with no .git entry, or one that is gone, is not retried: there is nothing -// a second git call could find. Each retry is said on stderr. -func retrySubagentStore(w io.Writer, in hookInput) (repoRoot, rootSHA, via string) { - if in.Cwd == "" || !gitutil.RepoShaped(in.Cwd) { - return "", "", "" - } +// retrySubagentStore asks again, after resolveSubagentStore found no store +// because git did not answer for the cwd's root commit (first, the failure the +// first try met): git failing once is a loaded machine, not a directory +// outside any repository, and a burst's siblings write the session note as +// they resolve, so it is re-read between tries. A cwd git answered for +// definitely — none, gone, or a repository with no commits — is never retried, +// since a second git call could find nothing there; the caller does not call +// it then. A try whose answer turns definite stops the retries the same way. +// Each retry is said on stderr. +func retrySubagentStore(w io.Writer, in hookInput, first error) (repoRoot, rootSHA, via string) { + last := first for i, wait := range subagentResolveWaits { time.Sleep(wait) - if repoRoot, rootSHA, via = resolveSubagentStore(in); rootSHA != "" { - fmt.Fprintf(w, "abcd history: sub-agent %s: the repository resolved on try %d, after git first gave no root commit for a repository-shaped cwd\n", + var rootErr error + if repoRoot, rootSHA, via, rootErr = resolveSubagentStore(in); rootSHA != "" { + fmt.Fprintf(w, "abcd history: sub-agent %s: the repository resolved on try %d, after git first gave no root commit for the cwd\n", termsafe.Sanitize(in.AgentID), i+2) return repoRoot, rootSHA, via } + if rootErr == nil { + fmt.Fprintf(w, "abcd history: sub-agent %s: on try %d git answered that the cwd has no root commit\n", + termsafe.Sanitize(in.AgentID), i+2) + return "", "", "" + } + last = rootErr } - fmt.Fprintf(w, "abcd history: sub-agent %s: git gave no root commit for the repository-shaped cwd in %d tries\n", - termsafe.Sanitize(in.AgentID), len(subagentResolveWaits)+1) + fmt.Fprintf(w, "abcd history: sub-agent %s: git gave no root commit for the cwd in %d tries (%s)\n", + termsafe.Sanitize(in.AgentID), len(subagentResolveWaits)+1, termsafe.Sanitize(fsutil.RedactHome(last.Error()))) return "", "", "" } @@ -270,15 +285,44 @@ func retrySubagentStore(w io.Writer, in hookInput) (repoRoot, rootSHA, via strin // the transcript (iss-2610051434156079). Every other failure returns at once: // it would fail the same way again. func stageWithRetry(w io.Writer, repoRoot, rootSHA string, meta history.StageMeta, raw []byte) (history.StageResult, error) { + who := "session " + termsafe.Sanitize(meta.Lineage.SessionID) + if meta.Lineage.AgentID != "" { + who = "sub-agent " + termsafe.Sanitize(meta.Lineage.AgentID) + } res, err := history.Stage(repoRoot, rootSHA, meta, raw) for try := 0; err != nil && errors.Is(err, fsutil.ErrLockContention) && try < subagentStageRetries; try++ { - fmt.Fprintf(w, "abcd history: sub-agent %s: the staging lock was held past its timeout; trying again (%d of %d)\n", - meta.Lineage.AgentID, try+1, subagentStageRetries) + fmt.Fprintf(w, "abcd history: %s: the staging lock was held past its timeout; trying again (%d of %d)\n", + who, try+1, subagentStageRetries) res, err = history.Stage(repoRoot, rootSHA, meta, raw) } return res, err } +// retryDetect is the session-end hook's retrySubagentStore: when git gave no +// root commit for a repository-shaped cwd, detection is asked again on the +// same bounded waits, and a try that turns definite (a root, or git answering +// that there is none) ends the retries. Each retry is said on stderr +// (iss-2610051434156079). +func retryDetect(w io.Writer, cwd string, first ahoy.DetectionResult) (ahoy.DetectionResult, error) { + det := first + for i, wait := range subagentResolveWaits { + time.Sleep(wait) + next, err := ahoy.Detect(cwd) + if err != nil { + return next, err + } + det = next + if det.RootSHA != "" { + fmt.Fprintf(w, "abcd history: the repository resolved on try %d, after git first gave no root commit for the cwd\n", i+2) + return det, nil + } + if det.RepoIdentity.RootErr() == nil { + return det, nil + } + } + return det, nil +} + // subagentStageMeta assembles the lineage for one sub-agent stage, running the // attribution ladder's first rung. // diff --git a/internal/surface/cli/hook_subagent_stage_retry_test.go b/internal/surface/cli/hook_subagent_stage_retry_test.go index c4aaeb9c7..72aa71520 100644 --- a/internal/surface/cli/hook_subagent_stage_retry_test.go +++ b/internal/surface/cli/hook_subagent_stage_retry_test.go @@ -192,11 +192,11 @@ func TestHookSubagentStopMarksAStageThatStillFails(t *testing.T) { if err != nil || !ok { t.Fatalf("the missed stage left no marker (%v)\nstderr: %s", err, stderr) } - if miss.Count != 1 || miss.LastAgent != "m1" || !strings.Contains(miss.LastReason, "lock") { + if miss.Count != 1 || miss.LastWho != "sub-agent m1" || !strings.Contains(miss.LastReason, "lock") { t.Errorf("marker = %+v; want one miss by m1 naming the lock", miss) } t.Chdir(repo) - if out := string(runCLI(t, "history", "staged")); !strings.Contains(out, "1 sub-agent transcript(s) were read but could not be staged") { + if out := string(runCLI(t, "history", "staged")); !strings.Contains(out, "1 transcript(s) were read but could not be staged") { t.Errorf("history staged does not report the missed stage:\n%s", out) } } @@ -212,3 +212,30 @@ func notStaged(stderrs []string) string { } return b.String() } + +// TestHookSessionEndRetriesATransientRootFailure: the session's own transcript +// had the same silent loss as a sub-agent's. One failed root-commit lookup at +// session end no longer gives the session up; the hook asks again and stages. +func TestHookSessionEndRetriesATransientRootFailure(t *testing.T) { + repo, rootSHA := sessionEndRepo(t) + tp := filepath.Join(t.TempDir(), "session.jsonl") + if err := os.WriteFile(tp, []byte(`{"role":"user","text":"hello"}`+"\n"), 0o600); err != nil { + t.Fatal(err) + } + failRootCommitGit(t, 1) + + _, stderr, failed := runHookAllowingFailure(endPayload(t, "sess-end-retry", repo, tp), "hook", "session-end") + if failed { + t.Fatalf("the hook exited non-zero: %s", stderr) + } + staged, err := history.ListStaged(repo, rootSHA) + if err != nil { + t.Fatal(err) + } + if len(staged) != 1 { + t.Fatalf("a transient git failure at session end dropped the session: %d staged\nstderr: %s", len(staged), stderr) + } + if !strings.Contains(stderr, "resolved on try") { + t.Errorf("stderr does not say the repository resolved on a retry: %q", stderr) + } +} diff --git a/internal/surface/cli/ideate.go b/internal/surface/cli/ideate.go index 4204a65e3..1c0ea9ad0 100644 --- a/internal/surface/cli/ideate.go +++ b/internal/surface/cli/ideate.go @@ -173,7 +173,11 @@ func ideateStore(repoRoot string, private, public bool) (ideate.Store, error) { scrubPaths(err) + "), and a verdict is filed under the repository's root commit, which a shallow clone does not hold " + "(nothing written)"} } - sha := gitutil.RootCommit(repoRoot) + sha, err := gitutil.RootCommit(repoRoot) + if err != nil { + return ideate.Store{}, &exitError{Code: 2, Msg: "ideate record: this repository's root commit could not be read (" + + scrubPaths(err) + "), and a verdict is filed under it (nothing written)"} + } if sha == "" { return ideate.Store{}, &exitError{Code: 2, Msg: "ideate record: this repository has no commit yet, " + "and a verdict is filed under the repository's root commit; commit once, then record (nothing written)"} diff --git a/internal/surface/cli/ideate_root_test.go b/internal/surface/cli/ideate_root_test.go index 3da94ee05..5f98c1e89 100644 --- a/internal/surface/cli/ideate_root_test.go +++ b/internal/surface/cli/ideate_root_test.go @@ -8,7 +8,7 @@ import ( "github.com/intentdriven/abcd/internal/core/decisions" "github.com/intentdriven/abcd/internal/core/ideate" - "github.com/intentdriven/abcd/internal/gitutil" + "github.com/intentdriven/abcd/internal/gittest" ) // The store an `ideate record` run writes into is the CHECKOUT's research store, @@ -63,7 +63,7 @@ func seedIdeateStore(t *testing.T, root string) { // through decisionTexts. func recordedVerdict(t *testing.T, repo, slug string) (note string) { t.Helper() - dir := filepath.Join(repo, filepath.FromSlash(ideate.ResearchRelDir), gitutil.RootCommit(repo)) + dir := filepath.Join(repo, filepath.FromSlash(ideate.ResearchRelDir), gittest.RootCommit(t, repo)) entries, err := os.ReadDir(dir) if err != nil { t.Fatalf("the checkout's research store holds no verdict: %v", err) diff --git a/internal/surface/cli/ideate_surface_test.go b/internal/surface/cli/ideate_surface_test.go index cb9c10b0d..415cb1116 100644 --- a/internal/surface/cli/ideate_surface_test.go +++ b/internal/surface/cli/ideate_surface_test.go @@ -13,7 +13,6 @@ import ( "github.com/intentdriven/abcd/internal/core/decisions" "github.com/intentdriven/abcd/internal/core/ideate" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // ideateRepo lays out the minimum an `ideate record` run needs, so the surface @@ -101,7 +100,7 @@ func TestIdeateRecordWiredFromTheCLI(t *testing.T) { // The literal directory, not ideate.ResearchRelDir: a check written against // the constant follows the constant wherever it moves, so it can never notice // the record landing outside the home research/README.md gives a dated note. - wantDir := ".abcd/development/research/ideas/" + gitutil.RootCommit(repo) + "/" + wantDir := ".abcd/development/research/ideas/" + gittest.RootCommit(t, repo) + "/" if !strings.HasPrefix(res.Path, wantDir) || !strings.HasSuffix(res.Path, "-ideate-the-ideate-gate.md") { t.Errorf("path = %q; want a dated ideate record under %s", res.Path, wantDir) } @@ -289,7 +288,7 @@ func TestIdeateRecordPrivateFromTheCLI(t *testing.T) { if err := json.Unmarshal(out, &res); err != nil { t.Fatalf("decode result: %v\n%s", err, out) } - sha := gitutil.RootCommit(repo) + sha := gittest.RootCommit(t, repo) wantPrefix := abcdhome.Display("ideas", sha) + "/" if res.Visibility != ideate.VisibilityPrivate || !strings.HasPrefix(res.Path, wantPrefix) || res.DecisionPath != "" { t.Errorf("result = visibility %q path %q decision %q; want private under %s with no pointer", diff --git a/internal/surface/cli/implement.go b/internal/surface/cli/implement.go index e5c34db0d..c45cbf9e9 100644 --- a/internal/surface/cli/implement.go +++ b/internal/surface/cli/implement.go @@ -171,7 +171,12 @@ func implementRun(access runAccess, session string) (*implement.Run, error) { if err != nil { return nil, err } - sha := gitutil.RootCommit(root) + sha, err := gitutil.RootCommit(root) + if err != nil { + // Not the core's "no root commit" refusal: git did not answer, so no + // run is opened, created or reported absent on a guess. + return nil, fmt.Errorf("reading the checkout's root commit, which keys the run: %w", err) + } var run *implement.Run switch access { case runJoin: diff --git a/internal/surface/cli/lab_surface_test.go b/internal/surface/cli/lab_surface_test.go index d47efbd4c..a86fd0535 100644 --- a/internal/surface/cli/lab_surface_test.go +++ b/internal/surface/cli/lab_surface_test.go @@ -9,7 +9,7 @@ import ( "github.com/intentdriven/abcd/internal/abcdhome" "github.com/intentdriven/abcd/internal/core/lab" - "github.com/intentdriven/abcd/internal/gitutil" + "github.com/intentdriven/abcd/internal/gittest" ) // labCheckout is a one-commit repository under a temp HOME, with the process @@ -41,7 +41,7 @@ func runLab(t *testing.T, args ...string) (int, string, string) { // dual-binary gate holds. func asAbcd(t *testing.T, repo string) { t.Helper() - t.Cleanup(lab.SetAbcdRootCommitForTest(gitutil.RootCommit(repo))) + t.Cleanup(lab.SetAbcdRootCommitForTest(gittest.RootCommit(t, repo))) } // The verb is wired: every sub-verb executes from the CLI, a gate refusal exits diff --git a/internal/surface/cli/memory.go b/internal/surface/cli/memory.go index d650a08d4..63de12bf6 100644 --- a/internal/surface/cli/memory.go +++ b/internal/surface/cli/memory.go @@ -87,17 +87,24 @@ const ( // readMemoryLayers reads both layers through the recall's one reader // (recall.ReadLayers), for the repository the rules loader resolved, and // that repository's root commit, the key a personal note is tied by. A layer -// that cannot be read is named, never fatal here. -func readMemoryLayers(home, repoRoot string) (l recall.Layers, root string, unreadable []recall.Refusal) { +// that cannot be read is named, never fatal here. A root commit git did not +// answer for is the error: without it every tied note would read as another +// repository's, a reach the listing would state as fact. +func readMemoryLayers(home, repoRoot string) (l recall.Layers, root string, unreadable []recall.Refusal, err error) { l, unreadable = recall.ReadLayers(home, repoRoot) if unreadable == nil { unreadable = []recall.Refusal{} } - return l, memoryRootCommit(repoRoot), unreadable + root, err = memoryRootCommit(repoRoot) + if err != nil { + return recall.Layers{}, "", nil, fmt.Errorf("abcd memory: reading this repository's root commit, which says which personal notes are tied to it: %w", err) + } + return l, root, unreadable, nil } // memoryRootCommit is the repository's root commit, the key a personal note -// is tied by; "" outside a repository with commits. +// is tied by; "" outside a repository with commits, and an error when git did +// not answer. var memoryRootCommit = gitutil.RootCommit // reachOf names a personal note's reach from the repository root key. @@ -163,7 +170,10 @@ func newMemoryCommand(asJSON *bool) *cobra.Command { if err != nil { return err } - l, root, unreadable := readMemoryLayers(home, repoRoot) + l, root, unreadable, err := readMemoryLayers(home, repoRoot) + if err != nil { + return err + } out := memoryBareOutput{Project: len(l.Project), Unreadable: unreadable} for _, n := range l.Machine { switch reachOf(n, root) { @@ -258,7 +268,10 @@ func newMemoryCommand(asJSON *bool) *cobra.Command { if err != nil { return err } - l, root, unreadable := readMemoryLayers(home, repoRoot) + l, root, unreadable, err := readMemoryLayers(home, repoRoot) + if err != nil { + return err + } out := memoryListOutput{Notes: []memoryNoteView{}, Unreadable: unreadable} for _, n := range l.Machine { out.Notes = append(out.Notes, noteView("machine", reachOf(n, root), n)) diff --git a/internal/surface/cli/memory_test.go b/internal/surface/cli/memory_test.go index 9528f920c..031b9504f 100644 --- a/internal/surface/cli/memory_test.go +++ b/internal/surface/cli/memory_test.go @@ -307,7 +307,9 @@ func TestMemoryAddRefusesAMissingTitleRecallOrWriter(t *testing.T) { // A root the resolver refuses is named on stderr before the verb's own // refusal: rules.Resolution's notes are a front door's to print, and a memory // verb that dropped them left a no-root-commit refusal with no word of why the -// repository was not used. +// repository was not used. The verb's own refusal is git's: git will not +// answer for a checkout another uid owns, which is not a repository with no +// root commit (iss-2610051434156079), so it is exit 1, not ErrNoRoot's 2. func TestMemoryAddPrintsTheOwnershipRefusalBeforeItsOwn(t *testing.T) { home, repo := memoryFixture(t) sub := mustMkdirAll(t, filepath.Join(repo, "pkg")) @@ -318,13 +320,13 @@ func TestMemoryAddPrintsTheOwnershipRefusalBeforeItsOwn(t *testing.T) { var so, se bytes.Buffer code := run([]string{"memory", "add", "--title", "The tests need the race flag", "--recall", "test", "--recall", "race", "--by", "Example:model-1"}, strings.NewReader(raceBody), &so, &se) - if code != 2 { - t.Fatalf("exit = %d, want 2\nstdout:\n%s\nstderr:\n%s", code, so.String(), se.String()) + if code != 1 { + t.Fatalf("exit = %d, want 1\nstdout:\n%s\nstderr:\n%s", code, so.String(), se.String()) } stderr := se.String() - refused, noRoot := strings.Index(stderr, "REFUSED"), strings.Index(stderr, "no root commit") + refused, noRoot := strings.Index(stderr, "REFUSED"), strings.Index(stderr, "git gave no root commit") if refused < 0 || noRoot < 0 || refused > noRoot { - t.Errorf("stderr must name the refused root before the no-root-commit refusal:\n%s", stderr) + t.Errorf("stderr must name the refused root before git's root-commit refusal:\n%s", stderr) } if _, err := os.Stat(abcdhome.Path(home, "memory.json")); !os.IsNotExist(err) { t.Error("a refused add wrote the machine layer") diff --git a/internal/surface/cli/outage_render_test.go b/internal/surface/cli/outage_render_test.go index 777685cff..4452c5335 100644 --- a/internal/surface/cli/outage_render_test.go +++ b/internal/surface/cli/outage_render_test.go @@ -12,7 +12,7 @@ import ( "github.com/intentdriven/abcd/internal/core/implement" "github.com/intentdriven/abcd/internal/core/implement/loop" - "github.com/intentdriven/abcd/internal/gitutil" + "github.com/intentdriven/abcd/internal/gittest" ) // A run's lost connection (iss-2610080620372731) reads in `implement step`, in @@ -85,7 +85,7 @@ func TestImplementStatusNamesAnUnreadableOutage(t *testing.T) { repo := buildRepo(t) mustImplement(t, "build", "itd-10", "--json") mustImplement(t, "implement", "join", "--session", "alpha", "--role", "first", "--json") - run, err := implement.Peek(gitutil.RootCommit(repo.Root())) + run, err := implement.Peek(gittest.RootCommit(t, repo.Root())) if err != nil { t.Fatal(err) } diff --git a/internal/surface/cli/report_surface_test.go b/internal/surface/cli/report_surface_test.go index 33f4fbf9e..0f777c7cb 100644 --- a/internal/surface/cli/report_surface_test.go +++ b/internal/surface/cli/report_surface_test.go @@ -12,7 +12,6 @@ import ( "github.com/intentdriven/abcd/internal/abcdhome" "github.com/intentdriven/abcd/internal/core/report" "github.com/intentdriven/abcd/internal/gittest" - "github.com/intentdriven/abcd/internal/gitutil" ) // fillTemplate fills the issued skeleton the way a reporter does. @@ -214,7 +213,7 @@ func TestInboxListsShowsAndPromotes(t *testing.T) { // The readable one is promoted; which of the two was planted is the // directory order's choice. The repository stands in for abcd's own // checkout, the only place a promotion files. - t.Cleanup(report.SetAbcdRootCommitForTest(gitutil.RootCommit(repo))) + t.Cleanup(report.SetAbcdRootCommitForTest(gittest.RootCommit(t, repo))) first = promotable out := string(runCLI(t, "inbox", "promote", first)) if !strings.Contains(out, "promoted "+first+" to iss-") { @@ -352,7 +351,7 @@ func TestInboxPromoteCaptureRefusalExitsTwo(t *testing.T) { } t.Setenv("HOME", home) t.Chdir(repo) - t.Cleanup(report.SetAbcdRootCommitForTest(gitutil.RootCommit(repo))) + t.Cleanup(report.SetAbcdRootCommitForTest(gittest.RootCommit(t, repo))) id := fileOneReport(t, "a finding about abcd") elsewhere := t.TempDir() if err := os.MkdirAll(filepath.Join(repo, ".abcd", "work"), 0o755); err != nil { diff --git a/internal/surface/cli/source.go b/internal/surface/cli/source.go index 4870db88c..17fd65e53 100644 --- a/internal/surface/cli/source.go +++ b/internal/surface/cli/source.go @@ -429,7 +429,10 @@ func ledgerRepo(flag string) (string, error) { if err != nil { return "", &exitError{Code: 2, Msg: "abcd library ledger: " + err.Error() + " (or name the ledger with --repo)"} } - sha := gitutil.RootCommit(root) + sha, err := gitutil.RootCommit(root) + if err != nil { + return "", &exitError{Code: 1, Msg: "abcd library ledger: this checkout's root commit could not be read (" + scrubPaths(err) + "), and the ledger is named by it; try again, or name it with --repo"} + } if !gitutil.IsFullSHA(sha) { return "", &exitError{Code: 2, Msg: "abcd library ledger: this checkout has no commit to name its ledger by; name it with --repo"} } diff --git a/internal/surface/cli/statusline.go b/internal/surface/cli/statusline.go index 1384eb411..453804684 100644 --- a/internal/surface/cli/statusline.go +++ b/internal/surface/cli/statusline.go @@ -13,6 +13,7 @@ import ( "github.com/intentdriven/abcd/internal/abcdhome" "github.com/intentdriven/abcd/internal/core/ahoy" "github.com/intentdriven/abcd/internal/core/statusline" + "github.com/intentdriven/abcd/internal/gitutil" "github.com/intentdriven/abcd/internal/procgroup" "github.com/intentdriven/abcd/internal/termsafe" "github.com/spf13/cobra" @@ -243,7 +244,11 @@ func decideStatus(gitCtx context.Context, gitBudget time.Duration, stdin io.Read if rootErr == nil { var mErr error if managed, mErr = ahoy.ManagedContext(gitCtx, root); mErr != nil { - d.notes = append(d.notes, fmt.Sprintf("git did not say within %s whether abcd manages this checkout, so the status line is empty for this refresh", gitBudget)) + note := fmt.Sprintf("git did not say within %s whether abcd manages this checkout, so the status line is empty for this refresh", gitBudget) + if errors.Is(mErr, gitutil.ErrNoRootAnswer) { + note = "git gave no root commit for this checkout, so whether abcd manages it is unknown and the status line is empty for this refresh" + } + d.notes = append(d.notes, note) d.blank = true return d } From 46e6217f7e76c305897efd1365c526e515065181 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:38:43 +0100 Subject: [PATCH 04/10] fix: keep the session-end hook's retries inside its 1.5-second budget The host gives a SessionEnd hook 1.5 seconds unless it asks for more, so the sub-agent hook's three and a half seconds of resolution waits and its stage retries would have been cut off there, leaving neither the stage nor the marker. Session end now pauses twice, briefly, for the repository and does not retry the stage. Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- .../brief/04-surfaces/11-history.md | 4 +++- internal/surface/cli/cli.go | 4 ++-- internal/surface/cli/hook_subagent.go | 24 ++++++++++++------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/11-history.md b/.abcd/development/brief/04-surfaces/11-history.md index 7c9925b69..11134d13d 100644 --- a/.abcd/development/brief/04-surfaces/11-history.md +++ b/.abcd/development/brief/04-surfaces/11-history.md @@ -297,7 +297,9 @@ any repository and staged nothing. When the payload's working directory carries a `.git` entry but git gave no root commit, the entrypoint asks again for a few seconds, re-reading the session note between tries. A stage refused because its lock was held past the lock's timeout is tried again, up to three -more times. Each retry is said on stderr. A transcript read and still not +more times. The session-end entrypoint asks again too, within the 1.5 seconds +the host gives a session-end hook: two short pauses for the repository, and no +further try of the stage. Each retry is said on stderr. A transcript read and still not staged, with the store known, is counted in a marker beside the staging directory, and the staged listing reports it: the entrypoint must exit 0, and its stderr reaches only the host's debug log, so the marker is where the person diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 18631004a..70999c16b 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -1991,7 +1991,7 @@ and the --json envelope carries the name as session_title.`, // git failing once on a loaded machine is not a directory // outside any repository: ask again before giving the session // up, as the sub-agent hook does (iss-2610051434156079). - det, err = retryDetect(cmd.ErrOrStderr(), cwd, det) + det, err = retryDetect(cmd.ErrOrStderr(), sessionEndResolveWaits, cwd, det) } if err == nil && det.RootSHA == "" { if rerr := det.RepoIdentity.RootErr(); rerr != nil { @@ -2019,7 +2019,7 @@ and the --json envelope carries the name as session_title.`, fmt.Fprintf(cmd.ErrOrStderr(), "abcd %s\n", termsafe.Sanitize(n)) } } - res, err := stageWithRetry(cmd.ErrOrStderr(), captureRoot(cwd), det.RootSHA, history.StageMeta{ + res, err := stageWithRetry(cmd.ErrOrStderr(), 0, captureRoot(cwd), det.RootSHA, history.StageMeta{ Lineage: history.CaptureMeta{SessionID: in.SessionID, Kind: "native", LineageSource: "hook"}, SourcePath: in.TranscriptPath, }, raw) diff --git a/internal/surface/cli/hook_subagent.go b/internal/surface/cli/hook_subagent.go index a163ab5f9..3876fd66e 100644 --- a/internal/surface/cli/hook_subagent.go +++ b/internal/surface/cli/hook_subagent.go @@ -91,6 +91,14 @@ var subagentResolveWaits = []time.Duration{100 * time.Millisecond, 400 * time.Mi // transcript is marked as missed rather than dropped unseen. var subagentStageRetries = 3 +// sessionEndResolveWaits are retryDetect's pauses for the session-end hook, +// which the host gives 1.5 seconds in all unless the hook asks for more +// (SessionEnd's default budget, per the host's hooks reference): two short +// pauses cover a git that failed once and leave the stage its time. Its stage +// is not retried for the same reason; a session's own lock is contended only +// by that session re-firing. +var sessionEndResolveWaits = []time.Duration{100 * time.Millisecond, 300 * time.Millisecond} + // newSubagentStopCommand builds `abcd hook subagent-stop`. func newSubagentStopCommand() *cobra.Command { return &cobra.Command{ @@ -162,7 +170,7 @@ func newSubagentStopCommand() *cobra.Command { } meta := subagentStageMeta(in) - res, err := stageWithRetry(cmd.ErrOrStderr(), repoRoot, rootSHA, meta, raw) + res, err := stageWithRetry(cmd.ErrOrStderr(), subagentStageRetries, repoRoot, rootSHA, meta, raw) if err != nil { // The transcript was read and is lost. The marker is what lets // the person see that: this hook's stderr reaches only a debug @@ -284,28 +292,28 @@ func retrySubagentStore(w io.Writer, in hookInput, first error) (repoRoot, rootS // timeout, so a stage that cannot take the lock waits longer instead of losing // the transcript (iss-2610051434156079). Every other failure returns at once: // it would fail the same way again. -func stageWithRetry(w io.Writer, repoRoot, rootSHA string, meta history.StageMeta, raw []byte) (history.StageResult, error) { +func stageWithRetry(w io.Writer, retries int, repoRoot, rootSHA string, meta history.StageMeta, raw []byte) (history.StageResult, error) { who := "session " + termsafe.Sanitize(meta.Lineage.SessionID) if meta.Lineage.AgentID != "" { who = "sub-agent " + termsafe.Sanitize(meta.Lineage.AgentID) } res, err := history.Stage(repoRoot, rootSHA, meta, raw) - for try := 0; err != nil && errors.Is(err, fsutil.ErrLockContention) && try < subagentStageRetries; try++ { + for try := 0; err != nil && errors.Is(err, fsutil.ErrLockContention) && try < retries; try++ { fmt.Fprintf(w, "abcd history: %s: the staging lock was held past its timeout; trying again (%d of %d)\n", - who, try+1, subagentStageRetries) + who, try+1, retries) res, err = history.Stage(repoRoot, rootSHA, meta, raw) } return res, err } // retryDetect is the session-end hook's retrySubagentStore: when git gave no -// root commit for a repository-shaped cwd, detection is asked again on the -// same bounded waits, and a try that turns definite (a root, or git answering +// root commit for a repository-shaped cwd, detection is asked again after +// each of waits, and a try that turns definite (a root, or git answering // that there is none) ends the retries. Each retry is said on stderr // (iss-2610051434156079). -func retryDetect(w io.Writer, cwd string, first ahoy.DetectionResult) (ahoy.DetectionResult, error) { +func retryDetect(w io.Writer, waits []time.Duration, cwd string, first ahoy.DetectionResult) (ahoy.DetectionResult, error) { det := first - for i, wait := range subagentResolveWaits { + for i, wait := range waits { time.Sleep(wait) next, err := ahoy.Detect(cwd) if err != nil { From 17e6cd8156570f13a46d20eac54c0f1e99ad03a1 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:02:41 +0100 Subject: [PATCH 05/10] chore: capture that the partial-clone guard's probes ignore a caller's deadline Refs: iss-2610181802344551 Assisted-by: Claude:claude-opus-5-5 --- ...551-a-context-bound-git-command-can-outlive.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .abcd/work/issues/open/iss-2610181802344551-a-context-bound-git-command-can-outlive.md diff --git a/.abcd/work/issues/open/iss-2610181802344551-a-context-bound-git-command-can-outlive.md b/.abcd/work/issues/open/iss-2610181802344551-a-context-bound-git-command-can-outlive.md new file mode 100644 index 000000000..24c45f671 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610181802344551-a-context-bound-git-command-can-outlive.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610181802344551" +slug: "a-context-bound-git-command-can-outlive" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "fixing iss-2610051434156079" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/gitutil/lazyfetch.go" +remedy: "Thread the caller's context into guardObjectRead, lazyFetchGuard, readFloorConfig and the version probe when the command is context-bound, so every git the call starts ends at the caller's deadline; test with a hanging git that answers the version probe" +--- + +A context-bound git command can outlive its caller's deadline: isolatedGitContext runs the partial-clone guard (lazyFetchGuard) first, and the guard's own git version probe and its floor-config read run with no context, so a git that hangs holds a caller on a deadline (the status verb, the prompt hook's reads) for as long as git hangs, though the command itself is killed at the deadline. Found when TestRootCommitContextKillsGitAtTheDeadline, run alone, took 20 s against a 500 ms deadline because its hanging fake git answered the guard's probes. From 1c706f85fa39e67d05402812e6bc57314efad697 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:02:42 +0100 Subject: [PATCH 06/10] fix: ask git's version again after a probe that could not run The partial-clone guard probed git's version once per process and kept the answer even when the probe failed, so a git that could not run once on a loaded machine read as a git too old for the rest of the process, refusing every object read in a partial clone. A failed probe is no longer kept; a line git answered with still is. The deadline test now warms the probe with the real git first, so it no longer depends on test order. Refs: iss-2610051434156079 Refs: iss-2610181802344551 Assisted-by: Claude:claude-opus-5-5 --- internal/gitutil/context_test.go | 4 ++++ internal/gitutil/lazyfetch.go | 14 +++++++++++--- internal/gitutil/version_floor_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/internal/gitutil/context_test.go b/internal/gitutil/context_test.go index 4d0df3cab..4e635bf3f 100644 --- a/internal/gitutil/context_test.go +++ b/internal/gitutil/context_test.go @@ -38,6 +38,10 @@ func hangingGit(t *testing.T) (pidFile string) { // context's own error rather than an empty answer, and the git it started is // gone — killed, not abandoned to finish on its own. func TestRootCommitContextKillsGitAtTheDeadline(t *testing.T) { + // The partial-clone guard asks the real git for its version once per + // process; asked here first, so the hanging git below is only the + // root-commit lookup and not the version probe the guard makes before it. + gitutil.GitAtLeast(2, 0) pidFile := hangingGit(t) // Repository-shaped, so the lookup asks git: a directory with no .git // entry is answered without it. diff --git a/internal/gitutil/lazyfetch.go b/internal/gitutil/lazyfetch.go index f0b17e788..fd41de2eb 100644 --- a/internal/gitutil/lazyfetch.go +++ b/internal/gitutil/lazyfetch.go @@ -70,15 +70,23 @@ func (v *versionCache) honoursNoLazyFetch() (bool, string) { } // atLeast reports whether the git on PATH is at least major.minor, and the -// version line it read, probing once per process. A probe that fails, or a -// line that does not parse, is below every floor. +// version line it read, probing until one probe runs and then never again in +// the process. A probe that fails, or a line that does not parse, is below +// every floor. A failed probe is not kept: git that could not run once on a +// loaded machine would otherwise be read as a git too old for the rest of +// the process, refusing every object read in a partial clone +// (iss-2610051434156079). A line that does not parse is git's own answer, and +// is kept. func (v *versionCache) atLeast(major, minor int) (bool, string) { v.mu.Lock() defer v.mu.Unlock() if !v.done { raw, err := v.src() + if err != nil { + return false, strings.TrimSpace(raw) + } v.raw = strings.TrimSpace(raw) - v.ok = err == nil + v.ok = true v.done = true } return v.ok && versionAtLeast(v.raw, major, minor), v.raw diff --git a/internal/gitutil/version_floor_test.go b/internal/gitutil/version_floor_test.go index 86e64cdbe..7e7ac23d9 100644 --- a/internal/gitutil/version_floor_test.go +++ b/internal/gitutil/version_floor_test.go @@ -32,3 +32,27 @@ func TestGitAtLeastNamesTheVersionInDigitsAlone(t *testing.T) { } } } + +// TestAFailedVersionProbeIsAskedAgain: a git that could not run once (a fork +// refused on a loaded machine) is not kept as a git below every floor for the +// rest of the process; the next question probes again (iss-2610051434156079). +func TestAFailedVersionProbeIsAskedAgain(t *testing.T) { + calls := 0 + restore := gitutil.SwapGitVersionForTest(func() (string, error) { + calls++ + if calls == 1 { + return "", errors.New("fork/exec git: resource temporarily unavailable") + } + return "git version 2.52.0", nil + }) + defer restore() + if ok, _ := gitutil.GitAtLeast(2, 44); ok { + t.Fatal("a failed probe read as a git at the floor") + } + if ok, v := gitutil.GitAtLeast(2, 44); !ok || v != "2.52" { + t.Fatalf("the probe after a failure = %v %q; want it asked again and at the floor", ok, v) + } + if ok, _ := gitutil.GitAtLeast(2, 44); !ok || calls != 2 { + t.Fatalf("a probe that ran was asked again (%d calls); want it kept", calls) + } +} From 83d0e8751c3480396db4bdb580583b71582fd429 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:27:24 +0100 Subject: [PATCH 07/10] fix: ask git before the shape, and show a lost transcript under --json The root-commit lookup took a path with no .git entry on it for no repository without asking git, so a directory reached through a symlink, or a bare repository, read as none. git is now asked first. A failure is read as "not a repository" only when git says so in its pinned C locale, or when neither the path nor its resolved form carries a .git entry. history staged --json printed only the array, so an agent told to report a lost transcript saw none. Both notes now also go to stderr there, and the text render no longer says every session is stored above a loss. Install no longer asks to adopt a repository git did not answer for; an explicit --adopt still stands. ahoy's status names the failure instead of "unmanaged". The hooks do not retry the partial-clone floor, which answers the same every time. A marker that no longer parses is replaced with a reason saying earlier misses were counted there. Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- commands/history.md | 3 +- internal/core/ahoy/apply.go | 12 +++++ internal/core/history/stagemiss.go | 12 ++++- internal/core/history/stagemiss_test.go | 25 +++++++++++ internal/gitutil/repo.go | 44 ++++++++++++++++--- internal/gitutil/rootcommit_test.go | 33 ++++++++++++++ internal/surface/cli/cli.go | 11 +++-- internal/surface/cli/history.go | 40 ++++++++++------- internal/surface/cli/hook_subagent.go | 9 ++++ .../cli/hook_subagent_stage_retry_test.go | 9 ++++ 10 files changed, 171 insertions(+), 27 deletions(-) diff --git a/commands/history.md b/commands/history.md index dbf245ba9..1ba8cbef6 100644 --- a/commands/history.md +++ b/commands/history.md @@ -171,7 +171,8 @@ at all, so an empty sub-agent corpus is the host and not the sessions. It carries a second note when transcripts, a session's or a sub-agent's, were read but could not be staged, with how many, when, and the last one's reason: those transcripts are not in the store, and the hook that lost them exited 0, -so this note is the only place the loss shows. Tell the user how many were lost and why. An entry +so this note is the only place the loss shows. Under `--json` both notes go to +stderr, beside the array, so read stderr as well. Tell the user how many were lost and why. An entry past the seven-day limit renders as OVERDUE; a quarantined transcript is listed in its own block, and is not awaiting anything — nothing will retry it. diff --git a/internal/core/ahoy/apply.go b/internal/core/ahoy/apply.go index 7822ce7c4..76831ac4a 100644 --- a/internal/core/ahoy/apply.go +++ b/internal/core/ahoy/apply.go @@ -120,6 +120,18 @@ func install(cwd string, opts InstallOptions, p Prompter) (res InstallResult, er p = waiting.prompter(p) } + // git not answering is not an unmanaged repository: the repository may be + // registered, so install never ASKS to adopt it on that reading + // (iss-2610051434156079). An explicit --adopt is the person's own word and + // stands; without one, nothing is installed until git answers. + gitUnanswered := det.RepoIdentity.RootErr() + if det.RootSHA != "" { + gitUnanswered = nil + } + if gitUnanswered != nil && det.FolderKind == UnmanagedRepo && (opts.Adopt == nil || !*opts.Adopt) { + return InstallResult{}, fmt.Errorf("git gave no root commit for this repository, so abcd cannot tell whether it already manages it; run the install again once git answers, or pass --adopt to adopt it anyway: %w", gitUnanswered) + } + // Adoption gate for an unmanaged repo. adopted := false if det.FolderKind == UnmanagedRepo { diff --git a/internal/core/history/stagemiss.go b/internal/core/history/stagemiss.go index 7496e9aa2..49d717375 100644 --- a/internal/core/history/stagemiss.go +++ b/internal/core/history/stagemiss.go @@ -69,7 +69,17 @@ func NoteStageMiss(repoRoot, rootSHA, who, reason string) error { return fsutil.WithFileLock(path+".lock", stageMissLockTimeout, func() error { now := time.Now().UTC() note := StageMissNote{Schema: 1, FirstSeen: now} - if prior, ok, err := readStageMiss(path); err == nil && ok { + prior, ok, rerr := readStageMiss(path) + switch { + case rerr != nil: + // A marker that cannot be read is replaced, and the reason says + // earlier misses were counted there, so the count restarting is + // never read as the first miss. + reason = "an earlier, unreadable stage-miss marker was replaced, so earlier misses are not counted here; " + reason + if len(reason) > stageMissReasonMax { + reason = reason[:stageMissReasonMax] + } + case ok: note.FirstSeen, note.Count = prior.FirstSeen, prior.Count } note.LastSeen, note.LastWho, note.LastReason = now, who, reason diff --git a/internal/core/history/stagemiss_test.go b/internal/core/history/stagemiss_test.go index 927769c2e..d3adbbc2b 100644 --- a/internal/core/history/stagemiss_test.go +++ b/internal/core/history/stagemiss_test.go @@ -1,6 +1,7 @@ package history import ( + "os" "strings" "sync" "testing" @@ -60,3 +61,27 @@ func TestStageMissKeepsTheReasonOneBoundedLine(t *testing.T) { t.Errorf("reason not one bounded line: %q (%d bytes)", note.LastReason, len(note.LastReason)) } } + +// TestStageMissSaysAReplacedMarkerHeldEarlierMisses: a marker that no longer +// parses is replaced, and the reason says earlier misses were counted there, +// so the restarted count is never read as the first miss. +func TestStageMissSaysAReplacedMarkerHeldEarlierMisses(t *testing.T) { + repoRoot, _ := setupStore(t) + path, err := stageMissPath(repoRoot, testRootSHA) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + if err := NoteStageMiss(repoRoot, testRootSHA, "sub-agent a1", "lock held"); err != nil { + t.Fatal(err) + } + note, ok, err := StageMiss(repoRoot, testRootSHA) + if err != nil || !ok { + t.Fatalf("marker not readable after replacement: ok=%v err=%v", ok, err) + } + if note.Count != 1 || !strings.Contains(note.LastReason, "earlier, unreadable stage-miss marker was replaced") { + t.Errorf("marker = %+v; want a count of 1 that says earlier misses were lost", note) + } +} diff --git a/internal/gitutil/repo.go b/internal/gitutil/repo.go index 0f0438b02..619149983 100644 --- a/internal/gitutil/repo.go +++ b/internal/gitutil/repo.go @@ -374,9 +374,10 @@ var ErrNoRootAnswer = errors.New("git gave no root commit for a repository") // histories); the first `rev-list` reports is the canonical one, the same // choice the history registry and the lifeboat probe make. // -// It returns ("", nil) only for the two definite answers: root names no -// directory or no repository (nothing at or above it carries a .git entry), -// or the repository has no commits yet. Every other failure is an error a +// It returns ("", nil) only for the definite answers: root names no +// directory; git exited 128 and nothing at or above root, as written or with +// its symlinks resolved, carries a .git entry; or the repository has no +// commits yet. Every other failure is an error a // caller must handle: ErrNoRootAnswer for a git that did not answer for a // repository-shaped directory, ErrGitTimedOut for one whose output missed the // runner's wait twice. A failed git call is never read as an answer, since a @@ -399,9 +400,10 @@ func RootCommitContext(ctx context.Context, root string) (string, error) { if _, err := os.Stat(root); errors.Is(err, os.ErrNotExist) { return "", nil } - if !RepoShaped(root) { - return "", nil - } + // git is always asked first: a directory reached through a symlink, or a + // bare repository, carries no .git entry on the path as written, and only + // git can say it is a repository. The shape is consulted after a failure, + // to tell "not a repository" from "a repository git did not answer for". out, err := RunLimitedContext(ctx, root, 4096, "rev-list", "-n", "1", "--max-parents=0", "HEAD") if ctxErr := ctx.Err(); ctxErr != nil { return "", ctxErr @@ -410,7 +412,7 @@ func RootCommitContext(ctx context.Context, root string) (string, error) { if errors.Is(err, ErrGitTimedOut) { return "", err } - if emptyHistory(ctx, root, err) { + if notARepository(root, err) || emptyHistory(ctx, root, err) { return "", nil } // The cause stays in the chain (%w), so a caller can still tell @@ -424,6 +426,34 @@ func RootCommitContext(ctx context.Context, root string) (string, error) { return fields[0], nil } +// notARepository reports whether a lookup git exited 128 on failed because +// root is in no repository: git said so itself, or neither the path as +// written nor its resolved form has a .git entry at or above it. git exits +// 128 for that and for a repository it will not answer for alike, so its +// message and the shape decide; a git that did not run to an exit is never +// "not a repository". +func notARepository(root string, lookupErr error) bool { + var ee *exec.ExitError + if !errors.As(lookupErr, &ee) || ee.ExitCode() != 128 { + return false + } + // git's own answer, in the C locale gitEnv pins: a .git entry that holds + // no repository (an empty folder, a stale gitfile) is still not one. The + // match is anchored on runBoundedCmd's stderr quote, so a path naming the + // words cannot pass for it; a refusal ("detected dubious ownership") is + // not this answer and stays an error. + if strings.Contains(lookupErr.Error(), `(stderr: "fatal: not a git repository`) { + return true + } + if RepoShaped(root) { + return false + } + if real, err := filepath.EvalSymlinks(root); err == nil && RepoShaped(real) { + return false + } + return true +} + // emptyHistory reports whether a failed root-commit lookup failed because the // repository has no commits yet. git exits 128 for that and for every other // fatal error alike, so it is asked directly: `rev-parse --verify -q HEAD` diff --git a/internal/gitutil/rootcommit_test.go b/internal/gitutil/rootcommit_test.go index c48dae4be..70a297046 100644 --- a/internal/gitutil/rootcommit_test.go +++ b/internal/gitutil/rootcommit_test.go @@ -119,3 +119,36 @@ func TestRootCommitNamesAGitThatCannotStart(t *testing.T) { } }) } + +// TestRootCommitThroughASymlinkIsItsSHA: a directory reached through a symlink +// into a repository's subdirectory carries no .git entry on the path as +// written, and is still a repository; git is asked, not the path's shape. +func TestRootCommitThroughASymlinkIsItsSHA(t *testing.T) { + r := gittest.NewRepo(t) + r.Write("svc/api/main.txt", "x\n") + r.Commit("one") + want := r.Git("rev-list", "--max-parents=0", "HEAD") + link := filepath.Join(t.TempDir(), "api") + if err := os.Symlink(filepath.Join(r.Root(), "svc", "api"), link); err != nil { + t.Skipf("symlink: %v", err) + } + got, err := gitutil.RootCommit(link) + if got != want || err != nil { + t.Errorf("RootCommit through a symlink = %q, %v; want %q, nil", got, err, want) + } +} + +// TestRootCommitOfABareRepositoryIsItsSHA: a bare repository has no .git +// entry at all, and still has a root commit. +func TestRootCommitOfABareRepositoryIsItsSHA(t *testing.T) { + r := gittest.NewRepo(t) + r.Write("a.txt", "a\n") + r.Commit("one") + want := r.Git("rev-list", "--max-parents=0", "HEAD") + bare := filepath.Join(t.TempDir(), "bare.git") + gittest.CloneBare(t, r.Root(), bare) + got, err := gitutil.RootCommit(bare) + if got != want || err != nil { + t.Errorf("RootCommit of a bare repository = %q, %v; want %q, nil", got, err, want) + } +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 70999c16b..5d0721203 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -4022,10 +4022,15 @@ func newAhoyCommand(asJSON *bool) *cobra.Command { } // Classification is read-only; the human report names the // next step per folder kind (itd-40 AC2/AC3). - switch res.FolderKind { - case ahoy.UnmanagedRepo: + switch rerr := res.RepoIdentity.RootErr(); { + case res.RootSHA == "" && rerr != nil: + // git not answering is not a repository abcd does not + // manage: it may be registered (iss-2610051434156079). + fmt.Fprintf(w, " git gave no root commit for this repository (%s), so abcd cannot tell whether it manages it; run this again.\n", + termsafe.Sanitize(fsutil.RedactHome(rerr.Error()))) + case res.FolderKind == ahoy.UnmanagedRepo: fmt.Fprintf(w, " unmanaged git repo — run `/abcd:ahoy install` to adopt it.\n") - case ahoy.UnmanagedFolder: + case res.FolderKind == ahoy.UnmanagedFolder: fmt.Fprintf(w, " not a git repository — nothing to act on.\n") } }) diff --git a/internal/surface/cli/history.go b/internal/surface/cli/history.go index d66b76bb9..c6d70e6c3 100644 --- a/internal/surface/cli/history.go +++ b/internal/surface/cli/history.go @@ -273,24 +273,34 @@ func newHistoryCommand(asJSON *bool) *cobra.Command { staged[k].SourcePath = fsutil.RedactHome(staged[k].SourcePath) } // The JSON envelope stays the array it has always been: a - // consumer that iterates it must keep working. The marker is a - // per-repo fact rather than a staged entry, so it is reported in - // the human render, where the reader who needs it is. + // consumer that iterates it must keep working. The markers are + // per-repo facts rather than staged entries, so the human render + // carries them after the list, and under --json they go to stderr, + // where an agent reading the array still sees a lost transcript + // (iss-2610051434156079). + notes := func(w io.Writer) { + if hasGap { + fmt.Fprintf(w, "\nNOTE: this harness fired %s %d time(s) without an agent_transcript_path (first %s). No sub-agent transcript can be captured on it, so an empty sub-agent corpus here is the harness, not the sessions.\n", + termsafe.Sanitize(orDefault(gap.Event, "SubagentStop")), gap.Count, + gap.FirstSeen.Format("2006-01-02T15:04:05Z")) + } + if hasMiss { + fmt.Fprintf(w, "\nNOTE: %d transcript(s) were read but could not be staged, so they are not in this store (first %s, last %s, %s: %s).\n", + miss.Count, miss.FirstSeen.Format("2006-01-02T15:04:05Z"), miss.LastSeen.Format("2006-01-02T15:04:05Z"), + termsafe.Sanitize(miss.LastWho), termsafe.Sanitize(fsutil.RedactHome(miss.LastReason))) + } + } + if *asJSON { + notes(cmd.ErrOrStderr()) + } return render(cmd.OutOrStdout(), *asJSON, staged, func(w io.Writer) { - defer func() { - if hasGap { - fmt.Fprintf(w, "\nNOTE: this harness fired %s %d time(s) without an agent_transcript_path (first %s). No sub-agent transcript can be captured on it, so an empty sub-agent corpus here is the harness, not the sessions.\n", - termsafe.Sanitize(orDefault(gap.Event, "SubagentStop")), gap.Count, - gap.FirstSeen.Format("2006-01-02T15:04:05Z")) - } + defer notes(w) + if len(staged) == 0 && len(quarantined) == 0 { if hasMiss { - fmt.Fprintf(w, "\nNOTE: %d transcript(s) were read but could not be staged, so they are not in this store (first %s, last %s, %s: %s).\n", - miss.Count, miss.FirstSeen.Format("2006-01-02T15:04:05Z"), miss.LastSeen.Format("2006-01-02T15:04:05Z"), - termsafe.Sanitize(miss.LastWho), termsafe.Sanitize(fsutil.RedactHome(miss.LastReason))) + fmt.Fprintln(w, "abcd history — nothing staged") + } else { + fmt.Fprintln(w, "abcd history — nothing staged; every ended session is stored") } - }() - if len(staged) == 0 && len(quarantined) == 0 { - fmt.Fprintln(w, "abcd history — nothing staged; every ended session is stored") return } for _, s := range staged { diff --git a/internal/surface/cli/hook_subagent.go b/internal/surface/cli/hook_subagent.go index 3876fd66e..ec63c4d45 100644 --- a/internal/surface/cli/hook_subagent.go +++ b/internal/surface/cli/hook_subagent.go @@ -38,6 +38,7 @@ import ( "github.com/intentdriven/abcd/internal/core/ahoy" "github.com/intentdriven/abcd/internal/core/history" "github.com/intentdriven/abcd/internal/fsutil" + "github.com/intentdriven/abcd/internal/gitutil" "github.com/intentdriven/abcd/internal/termsafe" ) @@ -267,6 +268,11 @@ func resolveSubagentStore(in hookInput) (repoRoot, rootSHA, via string, rootErr // it then. A try whose answer turns definite stops the retries the same way. // Each retry is said on stderr. func retrySubagentStore(w io.Writer, in hookInput, first error) (repoRoot, rootSHA, via string) { + if errors.Is(first, gitutil.ErrLazyFetchFloor) { + // abcd's own refusal to read a partial clone's objects on an old git + // answers the same every time: there is nothing a retry could find. + return "", "", "" + } last := first for i, wait := range subagentResolveWaits { time.Sleep(wait) @@ -313,6 +319,9 @@ func stageWithRetry(w io.Writer, retries int, repoRoot, rootSHA string, meta his // (iss-2610051434156079). func retryDetect(w io.Writer, waits []time.Duration, cwd string, first ahoy.DetectionResult) (ahoy.DetectionResult, error) { det := first + if errors.Is(det.RepoIdentity.RootErr(), gitutil.ErrLazyFetchFloor) { + return det, nil + } for i, wait := range waits { time.Sleep(wait) next, err := ahoy.Detect(cwd) diff --git a/internal/surface/cli/hook_subagent_stage_retry_test.go b/internal/surface/cli/hook_subagent_stage_retry_test.go index 72aa71520..30627ec61 100644 --- a/internal/surface/cli/hook_subagent_stage_retry_test.go +++ b/internal/surface/cli/hook_subagent_stage_retry_test.go @@ -199,6 +199,15 @@ func TestHookSubagentStopMarksAStageThatStillFails(t *testing.T) { if out := string(runCLI(t, "history", "staged")); !strings.Contains(out, "1 transcript(s) were read but could not be staged") { t.Errorf("history staged does not report the missed stage:\n%s", out) } + // Under --json the array stays an array, and the note reaches the agent + // reading it on stderr. + jsonOut, jsonErr, _ := runHookAllowingFailure("", "history", "staged", "--json") + if !strings.HasPrefix(strings.TrimSpace(jsonOut), "[") || strings.Contains(jsonOut, "NOTE") { + t.Errorf("history staged --json stdout is not the plain array:\n%s", jsonOut) + } + if !strings.Contains(jsonErr, "1 transcript(s) were read but could not be staged") { + t.Errorf("history staged --json does not report the missed stage on stderr:\n%s", jsonErr) + } } // notStaged lists the stderr of every hook in a burst that did not report From a47754aea5fc4167e576ebc7aab2a5acfc9d88c9 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:34:39 +0100 Subject: [PATCH 08/10] fix: refuse a partial clone's read as retryable when git's version was unread A git version probe that could not run is not a git below the floor. Refusing it as ErrLazyFetchFloor made both hooks skip their retries, so a transcript in a partial clone was lost to one failed fork after all. The read is still refused, now as ErrGitVersionUnread, which a later try may clear. An explicit --refuse-adopt with git unanswered reports the abort it asked for. Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- internal/core/ahoy/apply.go | 2 +- internal/gitutil/lazyfetch.go | 18 ++++++++++++++ internal/gitutil/lazyfetch_floor_test.go | 31 ++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/internal/core/ahoy/apply.go b/internal/core/ahoy/apply.go index 76831ac4a..c3f565926 100644 --- a/internal/core/ahoy/apply.go +++ b/internal/core/ahoy/apply.go @@ -128,7 +128,7 @@ func install(cwd string, opts InstallOptions, p Prompter) (res InstallResult, er if det.RootSHA != "" { gitUnanswered = nil } - if gitUnanswered != nil && det.FolderKind == UnmanagedRepo && (opts.Adopt == nil || !*opts.Adopt) { + if gitUnanswered != nil && det.FolderKind == UnmanagedRepo && opts.Adopt == nil { return InstallResult{}, fmt.Errorf("git gave no root commit for this repository, so abcd cannot tell whether it already manages it; run the install again once git answers, or pass --adopt to adopt it anyway: %w", gitUnanswered) } diff --git a/internal/gitutil/lazyfetch.go b/internal/gitutil/lazyfetch.go index fd41de2eb..20e026323 100644 --- a/internal/gitutil/lazyfetch.go +++ b/internal/gitutil/lazyfetch.go @@ -53,6 +53,18 @@ func SwapGitVersionForTest(src func() (string, error)) (restore func()) { return func() { gitVersion.set(probeGitVersion) } } +// known reports whether a version probe has run and its answer is kept. +func (v *versionCache) known() bool { + v.mu.Lock() + defer v.mu.Unlock() + return v.done +} + +// ErrGitVersionUnread is a partial clone's object read refused because git's +// version could not be read this time: unlike ErrLazyFetchFloor, a later try +// may clear it. +var ErrGitVersionUnread = errors.New("git's version could not be read") + // set replaces the source and clears the cached answer. func (v *versionCache) set(src func() (string, error)) { v.mu.Lock() @@ -162,6 +174,12 @@ func lazyFetchGuard(root string, args []string) error { if !cfg.promisor && !cmdPromisor { return nil } + if !gitVersion.known() { + // The version could not be read this time (a probe that did not run + // is not kept): that is git not answering, which a later try may + // clear, not a git known to be below the floor (iss-2610051434156079). + return fmt.Errorf("%w: git's version could not be read, so abcd cannot tell whether it honours GIT_NO_LAZY_FETCH in this partial clone", ErrGitVersionUnread) + } _, raw := gitVersion.honoursNoLazyFetch() if reads { return fmt.Errorf("%w (git on PATH: %q)", ErrLazyFetchFloor, raw) diff --git a/internal/gitutil/lazyfetch_floor_test.go b/internal/gitutil/lazyfetch_floor_test.go index 23bfa3bd5..377b68299 100644 --- a/internal/gitutil/lazyfetch_floor_test.go +++ b/internal/gitutil/lazyfetch_floor_test.go @@ -81,9 +81,13 @@ func TestIsolatedObjectReadRefusesAPartialCloneBelowTheLazyFetchFloor(t *testing r.Git("config", "extensions.partialClone", "origin") wantRefused("extensions.partialClone", read()) - // An unreadable version is treated as below the floor. + // An unreadable version still refuses the read, as git not answering + // rather than as the floor, so a caller may try again + // (iss-2610051434156079). gitutil.SetGitVersionSource(t, func() (string, error) { return "", errors.New("no git version") }) - wantRefused("unreadable version", read()) + if err := read(); !errors.Is(err, gitutil.ErrGitVersionUnread) { + t.Fatalf("unreadable version: want the read refused with ErrGitVersionUnread, got %v", err) + } // At the floor the same promisor repository reads. gitutil.SetGitVersionSource(t, func() (string, error) { return "git version 2.44.0", nil }) @@ -289,3 +293,26 @@ func TestSparseListingsRefuseAPartialCloneBelowTheLazyFetchFloor(t *testing.T) { gitutil.SetGitVersionSource(t, func() (string, error) { return "git version 2.44.0", nil }) allRun("sparse partial clone on git 2.44", listings) } + +// TestAPartialCloneReadWhoseVersionProbeFailedIsNotTheFloor: a git version +// probe that could not run is not "below the floor": the refusal is +// ErrGitVersionUnread, which a later try may clear, so a caller retrying a +// transient failure does retry it (iss-2610051434156079). +func TestAPartialCloneReadWhoseVersionProbeFailedIsNotTheFloor(t *testing.T) { + r := gittest.NewRepo(t) + r.Write("kept.txt", "kept\n") + r.Commit("c0") + root := r.Root() + r.Git("config", "remote.origin.promisor", "true") + + gitutil.SetGitVersionSource(t, func() (string, error) { return "", errors.New("fork/exec git: resource temporarily unavailable") }) + _, err := gitutil.Run(root, "cat-file", "blob", "HEAD:kept.txt") + if errors.Is(err, gitutil.ErrLazyFetchFloor) || !errors.Is(err, gitutil.ErrGitVersionUnread) { + t.Fatalf("a failed version probe = %v; want ErrGitVersionUnread, not the floor", err) + } + + gitutil.SetGitVersionSource(t, func() (string, error) { return "git version 2.39.5 (Apple Git-154)", nil }) + if _, err := gitutil.Run(root, "cat-file", "blob", "HEAD:kept.txt"); !errors.Is(err, gitutil.ErrLazyFetchFloor) { + t.Fatalf("a version known to be below the floor = %v; want ErrLazyFetchFloor", err) + } +} From 0c7ed6d70566f63b3465fc65dab4a1b7bea8c5d8 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:40:45 +0100 Subject: [PATCH 09/10] fix: read a plain folder with no git as none, and bound session end by time A directory with no .git entry, as written or resolved, is in no repository whatever git did, so git missing from PATH no longer turns a plain folder into one git did not answer for. Session end's retries are now bounded by time from the hook's start, each costed at what the first detection took, since detection takes most of a second on the loaded machine where git fails. The stage-miss marker keeps, through later misses, when an unreadable earlier marker was replaced. Refs: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- .../brief/04-surfaces/11-history.md | 5 ++-- internal/core/history/stagemiss.go | 14 +++++----- internal/core/history/stagemiss_test.go | 9 ++++++- internal/gitutil/repo.go | 26 ++++++++++--------- internal/gitutil/rootcommit_test.go | 11 ++++++++ internal/surface/cli/cli.go | 5 +++- internal/surface/cli/history.go | 4 +++ internal/surface/cli/hook_subagent.go | 24 +++++++++++++---- 8 files changed, 70 insertions(+), 28 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/11-history.md b/.abcd/development/brief/04-surfaces/11-history.md index 11134d13d..b12c2236e 100644 --- a/.abcd/development/brief/04-surfaces/11-history.md +++ b/.abcd/development/brief/04-surfaces/11-history.md @@ -298,8 +298,9 @@ carries a `.git` entry but git gave no root commit, the entrypoint asks again for a few seconds, re-reading the session note between tries. A stage refused because its lock was held past the lock's timeout is tried again, up to three more times. The session-end entrypoint asks again too, within the 1.5 seconds -the host gives a session-end hook: two short pauses for the repository, and no -further try of the stage. Each retry is said on stderr. A transcript read and still not +the host gives a session-end hook: up to two short pauses for the repository, +each begun only if it and another detection, costed at what the first took, +end within 1.2 seconds of the hook's start, and no further try of the stage. Each retry is said on stderr. A transcript read and still not staged, with the store known, is counted in a marker beside the staging directory, and the staged listing reports it: the entrypoint must exit 0, and its stderr reaches only the host's debug log, so the marker is where the person diff --git a/internal/core/history/stagemiss.go b/internal/core/history/stagemiss.go index 49d717375..5b623ae9b 100644 --- a/internal/core/history/stagemiss.go +++ b/internal/core/history/stagemiss.go @@ -38,6 +38,9 @@ type StageMissNote struct { Count int `json:"count"` LastWho string `json:"last_who"` LastReason string `json:"last_reason"` + // EarlierLostAt is when an unreadable marker was replaced: misses before + // it were counted there and are not in Count. Zero when none was. + EarlierLostAt time.Time `json:"earlier_lost_at,omitzero"` } // stageMissPath returns the marker's path beside the staging directory. @@ -72,15 +75,12 @@ func NoteStageMiss(repoRoot, rootSHA, who, reason string) error { prior, ok, rerr := readStageMiss(path) switch { case rerr != nil: - // A marker that cannot be read is replaced, and the reason says - // earlier misses were counted there, so the count restarting is + // A marker that cannot be read is replaced, and the replacement + // says when, for as long as it stands, so the count restarting is // never read as the first miss. - reason = "an earlier, unreadable stage-miss marker was replaced, so earlier misses are not counted here; " + reason - if len(reason) > stageMissReasonMax { - reason = reason[:stageMissReasonMax] - } + note.EarlierLostAt = now case ok: - note.FirstSeen, note.Count = prior.FirstSeen, prior.Count + note.FirstSeen, note.Count, note.EarlierLostAt = prior.FirstSeen, prior.Count, prior.EarlierLostAt } note.LastSeen, note.LastWho, note.LastReason = now, who, reason note.Count++ diff --git a/internal/core/history/stagemiss_test.go b/internal/core/history/stagemiss_test.go index d3adbbc2b..7583b36a5 100644 --- a/internal/core/history/stagemiss_test.go +++ b/internal/core/history/stagemiss_test.go @@ -81,7 +81,14 @@ func TestStageMissSaysAReplacedMarkerHeldEarlierMisses(t *testing.T) { if err != nil || !ok { t.Fatalf("marker not readable after replacement: ok=%v err=%v", ok, err) } - if note.Count != 1 || !strings.Contains(note.LastReason, "earlier, unreadable stage-miss marker was replaced") { + if note.Count != 1 || note.EarlierLostAt.IsZero() { t.Errorf("marker = %+v; want a count of 1 that says earlier misses were lost", note) } + // The flag stands through later misses, not just the one that replaced it. + if err := NoteStageMiss(repoRoot, testRootSHA, "sub-agent a2", "lock held"); err != nil { + t.Fatal(err) + } + if again, _, _ := StageMiss(repoRoot, testRootSHA); again.Count != 2 || !again.EarlierLostAt.Equal(note.EarlierLostAt) { + t.Errorf("after a second miss, marker = %+v; want a count of 2 still saying when earlier misses were lost", again) + } } diff --git a/internal/gitutil/repo.go b/internal/gitutil/repo.go index 619149983..7aad6c6c5 100644 --- a/internal/gitutil/repo.go +++ b/internal/gitutil/repo.go @@ -426,25 +426,27 @@ func RootCommitContext(ctx context.Context, root string) (string, error) { return fields[0], nil } -// notARepository reports whether a lookup git exited 128 on failed because +// notARepository reports whether a failed root-commit lookup failed because // root is in no repository: git said so itself, or neither the path as // written nor its resolved form has a .git entry at or above it. git exits // 128 for that and for a repository it will not answer for alike, so its -// message and the shape decide; a git that did not run to an exit is never -// "not a repository". +// message and the shape decide. func notARepository(root string, lookupErr error) bool { var ee *exec.ExitError - if !errors.As(lookupErr, &ee) || ee.ExitCode() != 128 { - return false - } - // git's own answer, in the C locale gitEnv pins: a .git entry that holds - // no repository (an empty folder, a stale gitfile) is still not one. The - // match is anchored on runBoundedCmd's stderr quote, so a path naming the - // words cannot pass for it; a refusal ("detected dubious ownership") is - // not this answer and stays an error. - if strings.Contains(lookupErr.Error(), `(stderr: "fatal: not a git repository`) { + if errors.As(lookupErr, &ee) && ee.ExitCode() == 128 && + strings.Contains(lookupErr.Error(), `(stderr: "fatal: not a git repository`) { + // git's own answer, in the C locale gitEnv pins: a .git entry that + // holds no repository (an empty folder, a stale gitfile) is still not + // one. The match is anchored on runBoundedCmd's stderr quote, which + // escapes every quote a path could carry, so a path naming the words + // cannot pass for it; a refusal ("detected dubious ownership") is not + // this answer and stays an error. return true } + // Whatever git did (exited, never started, was absent from PATH), a + // directory with no .git entry at or above it, as written or resolved, is + // in no repository. Only a repository-shaped directory makes a failure an + // unanswered repository. if RepoShaped(root) { return false } diff --git a/internal/gitutil/rootcommit_test.go b/internal/gitutil/rootcommit_test.go index 70a297046..05adc65aa 100644 --- a/internal/gitutil/rootcommit_test.go +++ b/internal/gitutil/rootcommit_test.go @@ -152,3 +152,14 @@ func TestRootCommitOfABareRepositoryIsItsSHA(t *testing.T) { t.Errorf("RootCommit of a bare repository = %q, %v; want %q, nil", got, err, want) } } + +// TestRootCommitOutsideAnyRepositoryWithNoGitIsNone: a plain folder is no +// repository whether or not git can run there, so git missing from PATH does +// not turn it into a repository git did not answer for. +func TestRootCommitOutsideAnyRepositoryWithNoGitIsNone(t *testing.T) { + dir := t.TempDir() + t.Setenv("PATH", t.TempDir()) + if got, err := gitutil.RootCommit(dir); got != "" || err != nil { + t.Errorf("RootCommit of a plain folder with no git = %q, %v; want \"\", nil", got, err) + } +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 5d0721203..548bbbd5d 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -1957,6 +1957,7 @@ and the --json envelope carries the name as session_title.`, Short: "SessionEnd: stage the raw transcript for the next session to redact and store", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + started := time.Now() // Diagnostics go to stderr, out of band; stdout stays empty, since a // Stop hook's stdout is not a place to speak to the model — unless // the caller asked for --json, when it carries one result line on @@ -1986,12 +1987,14 @@ and the --json envelope carries the name as session_title.`, cwd = wd } } + detectStart := time.Now() det, err := ahoy.Detect(cwd) + detectCost := time.Since(detectStart) if err == nil && det.RootSHA == "" && det.RepoIdentity.RootErr() != nil { // git failing once on a loaded machine is not a directory // outside any repository: ask again before giving the session // up, as the sub-agent hook does (iss-2610051434156079). - det, err = retryDetect(cmd.ErrOrStderr(), sessionEndResolveWaits, cwd, det) + det, err = retryDetect(cmd.ErrOrStderr(), sessionEndResolveWaits, started.Add(sessionEndRetryBudget), detectCost, cwd, det) } if err == nil && det.RootSHA == "" { if rerr := det.RepoIdentity.RootErr(); rerr != nil { diff --git a/internal/surface/cli/history.go b/internal/surface/cli/history.go index c6d70e6c3..1855ffa22 100644 --- a/internal/surface/cli/history.go +++ b/internal/surface/cli/history.go @@ -288,6 +288,10 @@ func newHistoryCommand(asJSON *bool) *cobra.Command { fmt.Fprintf(w, "\nNOTE: %d transcript(s) were read but could not be staged, so they are not in this store (first %s, last %s, %s: %s).\n", miss.Count, miss.FirstSeen.Format("2006-01-02T15:04:05Z"), miss.LastSeen.Format("2006-01-02T15:04:05Z"), termsafe.Sanitize(miss.LastWho), termsafe.Sanitize(fsutil.RedactHome(miss.LastReason))) + if !miss.EarlierLostAt.IsZero() { + fmt.Fprintf(w, "NOTE: the count starts at %s, when an unreadable record of earlier misses was replaced; misses before then are not counted.\n", + miss.EarlierLostAt.Format("2006-01-02T15:04:05Z")) + } } } if *asJSON { diff --git a/internal/surface/cli/hook_subagent.go b/internal/surface/cli/hook_subagent.go index ec63c4d45..4dee09abb 100644 --- a/internal/surface/cli/hook_subagent.go +++ b/internal/surface/cli/hook_subagent.go @@ -95,11 +95,20 @@ var subagentStageRetries = 3 // sessionEndResolveWaits are retryDetect's pauses for the session-end hook, // which the host gives 1.5 seconds in all unless the hook asks for more // (SessionEnd's default budget, per the host's hooks reference): two short -// pauses cover a git that failed once and leave the stage its time. Its stage -// is not retried for the same reason; a session's own lock is contended only -// by that session re-firing. +// pauses cover a git that failed once. Detection itself takes most of a +// second on a loaded machine, which is when git fails, so the pauses are +// bounded by sessionEndRetryBudget from the hook's start rather than counted: +// a retry whose pause and detection, costed at the first detection's time, +// cannot end before it is not begun, leaving the stage its time. Its stage is not retried for the same reason; a session's own lock is +// contended only by that session re-firing. var sessionEndResolveWaits = []time.Duration{100 * time.Millisecond, 300 * time.Millisecond} +// sessionEndRetryBudget is how long after the session-end hook starts a +// detection retry, its pause and its detection both (costed at what the first +// detection took), must have ended, leaving the rest of the host's 1.5 +// seconds to the stage. +var sessionEndRetryBudget = 1200 * time.Millisecond + // newSubagentStopCommand builds `abcd hook subagent-stop`. func newSubagentStopCommand() *cobra.Command { return &cobra.Command{ @@ -314,15 +323,20 @@ func stageWithRetry(w io.Writer, retries int, repoRoot, rootSHA string, meta his // retryDetect is the session-end hook's retrySubagentStore: when git gave no // root commit for a repository-shaped cwd, detection is asked again after -// each of waits, and a try that turns definite (a root, or git answering +// each of waits whose pause and detection (costed at cost, the first +// detection's time) end before deadline, and a try that turns definite (a root, or git answering // that there is none) ends the retries. Each retry is said on stderr // (iss-2610051434156079). -func retryDetect(w io.Writer, waits []time.Duration, cwd string, first ahoy.DetectionResult) (ahoy.DetectionResult, error) { +func retryDetect(w io.Writer, waits []time.Duration, deadline time.Time, cost time.Duration, cwd string, first ahoy.DetectionResult) (ahoy.DetectionResult, error) { det := first if errors.Is(det.RepoIdentity.RootErr(), gitutil.ErrLazyFetchFloor) { return det, nil } for i, wait := range waits { + if time.Now().Add(wait + cost).After(deadline) { + fmt.Fprintf(w, "abcd history: git gave no root commit for the cwd, and no time is left to ask again\n") + return det, nil + } time.Sleep(wait) next, err := ahoy.Detect(cwd) if err != nil { From dba100ecbbde769f8a324922a1d04766b0742f66 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:40:48 +0100 Subject: [PATCH 10/10] chore: resolve the sub-agent stage-loss record Resolves: iss-2610051434156079 Assisted-by: Claude:claude-opus-5-5 --- ...0051434156079-a-burst-of-subagent-stops-still-loses.md | 8 ++++++++ 1 file changed, 8 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md (82%) diff --git a/.abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md b/.abcd/work/issues/resolved/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md similarity index 82% rename from .abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md rename to .abcd/work/issues/resolved/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md index 110b92a5e..5441b4a93 100644 --- a/.abcd/work/issues/open/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md +++ b/.abcd/work/issues/resolved/iss-2610051434156079-a-burst-of-subagent-stops-still-loses.md @@ -12,6 +12,10 @@ found_at: "internal/core/history/staging.go" remedy: "Make a stage that cannot take the shared lock within its bound wait longer or queue the transcript for the next drain instead of dropping it, and have the hook say on stderr when a transcript was not staged; prove it with the 12-way burst test under -race on a deliberately slowed lock." deferred_after: v0.13.1 deferral_reason: "Second deferral, made by the 2026-10-07 autonomous run cutting v0.13.2: that patch ships the hook-message fix and the change-guard hardening and touches no staging code. The loss has appeared only on a slow, race-instrumented macOS runner, so its cause is not yet established, and the remedy needs the staging lock reworked and proved under a deliberately slowed lock. That is an investigation of its own, owed before the next minor release; a third deferral should go to the product thinker." +resolution: "A git call that failed once on a loaded machine read as no root commit, so a burst's sub-agent hooks, running before any sibling had noted the session, took the repository for none and dropped transcripts with exit 0. gitutil.RootCommit now returns an error for a failed call and never reads one as an answer; all twenty callers handle it. Both staging hooks retry a transient failure, within their host budgets, and mark a transcript they still cannot stage in a counted marker abcd history staged reports, under --json on stderr. The 12-way burst with failing root lookups stages all 12 under -race; on main it lost exactly the 6 whose lookups failed." +impact: fix +resolved_by: + commit: "46993aee0" --- On the v0.13.1 release PR's merge-queue run, the macOS race lane failed TestHookSubagentStopConcurrentCompletionsAllStage: 12 simultaneous subagent-stop hooks all exited 0 but only 10 transcripts were staged (internal/surface/cli/hook_subagent_stop_test.go:336). The same suite passed on the PR's own run and on every queue run earlier that day, so the loss appears only under a slow, race-instrumented host, but it is a silent loss: the hook reports success and the transcript never reaches the store. It is the defect iss-2609090828371674 resolved (the shared lock's backoff ceiling capping staging throughput), shown not to hold under load. @@ -33,3 +37,7 @@ Recurred in the merge queue: PR #869 (records only) was dropped at 06:59Z when i Reproduced deterministically, so the cause no longer rests on the slow macOS runner alone. The staging lock is not it: since iss-2609090828371674 each agent stages behind its own lock, so the twelve completions of the burst never contend for one, and 25 rounds of the burst under -race here staged every transcript. Slowing all seventeen git calls the hook makes (gittest.SlowPipeGit) still stages, only slowly. What drops a transcript is a git call that fails once rather than slowly. `gitutil.RootCommit` reads any git failure other than a timeout as "no root commit", so `resolveSubagentStore` takes the payload's cwd for a directory that is not a repository and falls back to the session note. A burst's first completions run before any sibling has written that note, so the hook prints "cannot resolve the repository for this sub-agent … staging nothing" on stderr, exits 0, and the transcript is gone. A git shim that fails the root-commit lookup once reproduces it: the hook exits 0 and nothing is staged. A loaded runner produces the same failure when a fork or a git process fails, and the burst test never printed the hooks' stderr, which is why the cause went unseen. The other path to the same silent loss is a stage that still fails, such as a lock held past its timeout, with the store known. + +## Grounds + +- pursued: a burst of sub-agent completions on a loaded machine stages every transcript or names each it lost; a transcript missing from the store with no marker counting it would show this wrong