Repository navigation
Restore retrospective artifact copying parity #1031
Description
Problem
Restore retrospective artifact copying parity after #331. The current Go implementation copies and counts a dangling extra link although the frozen shell warns and skips it (F1), recursively copies an extra of . into its own output although the shell refuses it (F2), and discards a failed report's output although shell redirection truncates the destination first and preserves partial output (F3).
This is CHILD-A, "Restore retrospective artifact copying parity". It owns parent AC6 and the extra-artifact behavior already merged from parent AC13. The base retains shared copying code; this child corrects its observable behavior without removing any existing test.
Acceptance criteria
- AC1: Given a readable transcript, when creation succeeds, then the artifact directory contains an independent transcript copy and the four canned reports with the expected contents; a linked transcript is copied as content.
- AC2: Given extra files, directories and links to existing artifacts, when creation succeeds, then their contents and nested link behavior match the frozen shell and the summary counts only copied extras. Given a nonexistent path or a dangling top-level link, creation warns "skipping missing artifact", leaves no copy for that extra, excludes it from the count and continues successfully.
AC3: GivenDropped: the frozen shell copies with.or an extra directory that contains the retrospective destination, when creation runs, then it exits nonzero without recursively reproducing its output tree or printing a success summary, matching the frozen shell's refusal outcome.cp -R, and stock macOScpand GNUcpdisagree on a copy into itself, so the shell cannot define the expected result. AC6 states the outcome directly.- AC4: Given any canned report that fails before output or after partial output, including an existing report destination, when creation runs, then that destination contains only the bytes emitted by the failing invocation, including an empty file when nothing was emitted. Previously completed reports remain available, later reports are not refreshed, creation exits nonzero, and no success summary is printed.
- AC5: Non-functional: given the child diff, when it is measured and verified with all existing and added tests, then it stays strictly below 800 code lines including generated copies, passes the existing coverage floors, and changes no gate, coverage exclusion or shell entry point.
- AC6: Given
., a directory that contains the retrospective destination, or a path reaching such a directory through a symlinked component, when creation runs, then it exits nonzero, prints a diagnostic that the artifact cannot be copied into itself, prints no success summary, and leaves exactly onetranscript.jsonlin the artifact tree and noRETRO.md. The outcome is asserted directly, without comparing against shell output.
Scope and non-goals
In scope: parent AC6, merged AC13 and F1-F3, including regression tests for dangling extras and failed report files, and the AC6 test for self-containing directories. Preserve transcript-link dereferencing and the already-fixed absence of a redundant child-error diagnostic.
Non-goal: preflight rendering and subprocess-status parity, which belong to CHILD-B, "Restore retrospective preflight and exit-status parity"; activation #1002; removal #1003; report-engine changes; test removal or gate changes.
Approach
Correct missing-extra detection, reject recursive destination containment, and open each report destination before running that report so failure leaves its actual evidence. Reuse shared test fixtures and add artifact regression tests without moving or shortening the base tests. Shared reporting setup may change within this child; metadata buffering remains separate from report-file output.
The measured scratch candidate changes copyArtifact, preserve, and the report-command setup in internal/retro/new.go, and adds internal/retro/artifact_parity_test.go. Production delivery must establish the complete acceptance criteria, including the ordered effects on other reports; the sizing candidate is evidence of fit, not a published fix.
Dependencies
This work is blocked by the BASE-1 slice of #331, which carries the copying code this child corrects. Delivery order is BASE-1, then this artifact child, with CHILD-B after BASE-2 and this child. CHILD-B is blocked by both preceding deliveries. Activation #1002 must wait for all three and launcher #932. Removal #1003 follows activation and its N-2 warning window. Blocked on: the BASE-1 slice of #331. When a route exists, record that as a native blocked-by relation, not prose only. Filing step: once this child and CHILD-B both exist, add native blocked-by links from #1002 to each of them, since #1002 currently has native blockers only for #331 and #932. No new issue or dependency relationship is represented as already filed.
Size
S: measured scratch candidate 113 code lines, consisting of 108 added and 5 deleted code lines, including the complete new regression-test file; generated-copy changes: 0. Strictly below 800. No test was removed or shortened to fit. Remeasure the final diff after completing all acceptance checks. These scratch-candidate counts are planning figures and must not be copied into pull request descriptions.
Verify plan
- AC1: retain
TestRetroNewandTestRetroSymlinkedTranscript; compare transcript and canned-report contents against the unchanged shell on isolated fixtures. - AC2: retain file, directory, nested-link and missing-path assertions in
TestRetroNew; addTestRetroDanglingExtraand comparisons for readable top-level links, warning text and counts. - AC3: struck, see AC6.
- AC6:
TestRetroSelfContainingExtraruns the Go engine for., a containing directory, and a path through a symlinked component, and asserts nonzero status, the diagnostic, empty standard output, a singletranscript.jsonlunder the artifact tree and noRETRO.md. It does not run the shell for these cases. - AC4: add
TestRetroReportFailureEvidencefor all four reports with empty and partial output over existing files; assert exact retained bytes, nonzero status and no success summary. The candidate also asserts completed reports are refreshed and later destinations remain unchanged; compare those effects with the shell before delivery. - AC5: run
go vet ./internal/retro ./internal/agentrepo,go test ./internal/retro ./internal/agentrepo,mise run lint-go,mise run check-generated-skillsandmise run cover-go, which writes a fresh coverage profile on the delivery revision and then enforces the unchanged thresholds; AC5 is met only when all five pass on the exact delivery revision. A threshold check against an older profile does not count. Measure added and deleted code with scc 4.1.0, counting new files in full and any generated copies. Require the complete original test set and no gate changes. - Parent crosswalk: parent AC6 is covered by child AC2, AC6 and AC4. Parent AC13 is covered through child AC2 and AC6. Parent IDs stay unchanged. The base keeps parent AC11 for the artifact totals of a normal run, and this child adds only the extra-artifact counts under child AC2.
- Record each AC as met, not met or deferred on the exact delivery revision. The scratch candidate passes the targeted Go tests; full acceptance, coverage and delivery gates are not claimed by this draft.
Risks
Containment checks must reject recursive copies without changing valid nested-link handling. Report failures intentionally leave truncated or partial files, so verification must inspect evidence on disk rather than only exit status. Undo: revert this child before activation; if CHILD-B has landed, revert it first. Retain the base and frozen entry points while acceptance remains incomplete.
Operator authority
None. No gate, owner path, coverage exclusion or shell entry point changes. The edit removes a shell comparison that cannot be deterministic across cp flavors and states the required outcome directly.
Activity
tbhb-agent commentedon Nov 1, 2026
Refinement: READY — artifact-parity child is bounded, with fresh coverage verification
Checked
originidentifiestbhb-dev/devtools; read parent Port analyze-session's shell scripts to Python #331’s body and comments once. Port analyze-session's shell scripts to Python #331, The binary resolver, permanent launcher and configuration bridges are resolved in Go #932, Switch analyze-session to the Go retro command with the frozen scripts as fallback #1002, Remove the analyze-session shell scripts after the N-2 window #1003, and Replace complex shell with tested Go, starting with gates and merging #588 are open..devtools/tasks/go.toml:162-172showsmise run cover-gowrites a freshcoverage.outwithgo test -coverprofile=coverage.outbefore enforcing thresholds.cover-go-checkonly checks an existing profile (.devtools/tasks/go.toml:228-230).internal/retropackage is not excluded:.testcoverage.yml:17-21excludes only commandmain.gofiles. Its files and package are subject to the file and package floors, and its coverage contributes to the total thresholds (.testcoverage.yml:12-15).mise run cover-goand requires unchanged coverage floors, tests, gates and size measurement. This directly addresses the stale-profile risk.Noneis appropriate operator authority.Gaps
None.
Split
The delivery order is base #331, CHILD-A, then CHILD-B; CHILD-B is blocked by both prior deliveries. Activation #1002 waits for the base, both children and #932; removal #1003 follows activation and the N-2 window. The draft correctly treats future dependency links as not yet filed and calls for native blocked-by links where the issues exist.
Loosens: no
Definition of done
Planning coverage only; these checkboxes describe what the draft’s ACs and Verify plan cover, not completed delivery work.
Assisted-bytrailers and PR provenance label are present.Round: 3 of 3
tbhb-agent commentedon Nov 1, 2026
Refinement: READY — artifact corrections remain distinct from the creation base
Checked
the proposed bodyagainstthe live body; the live timestamp matches.packages/agents/source/skills/analyze-session/shared/scripts/new-retro.sh:109,packages/agents/source/skills/analyze-session/shared/scripts/new-retro.sh:117andpackages/agents/source/skills/analyze-session/shared/scripts/new-retro.sh:112.Gaps
None.
Split
BASE-1 supplies the copying implementation; this issue corrects dangling extras, recursive containment and failed-report evidence. #1032 follows both this issue and BASE-2. This is compatible with the other bodies’ delivery order.
The approach changes the relevant copying and report-output operations directly, without speculative machinery.
Loosens: no — only duplicate summary ownership is removed; BASE-1 AC11 and its Verify entry retain that guarantee.
Owner-tier: no — the stated implementation and test paths require none of the listed protected paths.
Definition of done
The definition of done for #1031 (Feature). Tick each item and give its evidence.
check-leaks.shwith the repository's private-strings list, and the leak review of each post.docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.provenance:agent-assistedlabel.Status:comment names the merge commit.mise run checkis green, and so are the coverage floorsmise run cover-go-checkandmise run cover-py-check, whichcheckdoesn't run.packages/agents/source/commits whatmise run generate-skills, thenmise run skill-tokens, thenapm installrewrite, in that order, andcheck-generated-skills(part oflint) passes.docs/agent-skills-migration.mdor the adoption docs, or the PR's Verification says "No consumer impact" and why.Round: 1 of 3
tbhb-agent commentedon Nov 1, 2026
Refinement: READY — deterministic self-copy outcomes replace the platform-dependent shell oracle
Checked
originidentifiestbhb-dev/devtools. Diffed the proposed body against the live body. Earlier comments contain two READY verdicts and no NOT READY verdicts.transcript.jsonl, and noRETRO.md. Its Verify entry tests the Go engine directly for., a containing directory, and a path through a symlinked component, without shell comparison.origin/maintree does not containinternal/retro/new.goor the proposed test file, so there is no implementation there to inspect. The Verify plan keeps shell comparisons for AC1, AC2, and AC4, where shell behavior defines the stated parity outcomes.Operator authority: Noneremains valid.Gaps
None.
Split
Linux and macOS support symlinked paths; Windows can represent them with symlinks or junctions, subject to fixture setup. The
.and symlinked-component cases are well-defined by AC6’s direct Go outcome, independent ofcpbehavior. The stated outcomes can hold on all three operating systems if tests create and resolve the symlinked component there.Loosens: no — AC6 removes the shell-parity requirement but specifies deterministic refusal and additional observable outcomes, preserving AC3’s safety property.
Owner-tier: no
Definition of done
The definition of done for #1031 (Feature). Tick each item and give its evidence.
check-leaks.shwith the repository's private-strings list, and the leak review of each post.docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.provenance:agent-assistedlabel.Status:comment names the merge commit.mise run checkis green, and so are the coverage floorsmise run cover-go-checkandmise run cover-py-check, whichcheckdoesn't run.packages/agents/source/commits whatmise run generate-skills, thenmise run skill-tokens, thenapm installrewrite, in that order, andcheck-generated-skills(part oflint) passes.docs/agent-skills-migration.mdor the adoption docs, or the PR's Verification says "No consumer impact" and why.Round: 1 of 3