From 5e8c5be85fe8efbf71507b54c026de91de87bde8 Mon Sep 17 00:00:00 2001 From: Arjen Schwarz Date: Sun, 10 May 2026 22:11:28 +1000 Subject: [PATCH 1/4] docs(T-1197): Add smolspec for finalize show-and-verify Plan the enhancement to `orbit finalize` so it (a) shows the agent (alias, type, model) used for the variant being finalized in the confirmation preamble, and (b) warns when --variant disagrees with the most recent chosen_variant_id recorded in consolidation-log.json. The mismatch warning is informational: the user can still proceed via the existing y/N prompt or --force. A missing or unreadable consolidation log is treated as "no verification possible" and skipped silently, since consolidation is optional. Adds specs/finalize-show-verify/{smolspec.md,tasks.md,decision_log.md} with 5 implementation tasks (distributed test coverage plus a lint/test gate). No code changes in this commit. --- specs/finalize-show-verify/decision_log.md | 68 ++++++++++++++++++++++ specs/finalize-show-verify/smolspec.md | 52 +++++++++++++++++ specs/finalize-show-verify/tasks.md | 48 +++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 specs/finalize-show-verify/decision_log.md create mode 100644 specs/finalize-show-verify/smolspec.md create mode 100644 specs/finalize-show-verify/tasks.md diff --git a/specs/finalize-show-verify/decision_log.md b/specs/finalize-show-verify/decision_log.md new file mode 100644 index 0000000..7b6e6e5 --- /dev/null +++ b/specs/finalize-show-verify/decision_log.md @@ -0,0 +1,68 @@ +# Decision Log: Finalize Show and Verify + +## Decision 1: Mismatch produces a warning, not a hard error + +**Date**: 2026-05-10 +**Status**: accepted + +### Context + +When `orbit finalize --variant N` is invoked after `orbit consolidate` was run on a different variant, the user has likely lost track of which variant they intended to ship. The finalize command currently has no awareness of the consolidation log, so this footgun goes undetected. T-1197 asks for verification. + +### Decision + +When the variant passed to `--variant` does not match the most recent `chosen_variant_id` in `consolidation-log.json`, finalize prints a `Warning:` line naming both IDs and the consolidation timestamp, then proceeds through the existing `y/N` confirmation. No new flag is introduced and finalize never refuses to run on this basis. + +### Rationale + +Hard-failing or requiring a `--force-mismatch` flag would block legitimate workflows where the user intentionally finalizes a different variant than the one consolidated (e.g., they reviewed the consolidated diff and decided to ship the un-consolidated original instead). A warning surfaces the discrepancy without removing user agency. The existing `y/N` prompt — and `--force` for CI — already give the user a clear acknowledge-or-abort decision point. + +### Alternatives Considered + +- **Hard error**: Refuse to finalize on mismatch, requiring the user to consolidate or pick the matching variant - Rejected because it blocks legitimate "I changed my mind" workflows. +- **Warn + require `--force-mismatch`**: Print warning and require a new flag to proceed - Rejected because the existing `y/N` prompt already serves this purpose; adding a flag duplicates UX without adding safety. + +### Consequences + +**Positive:** +- No new flag surface area to document or maintain. +- Users who intentionally finalize a different variant are not blocked. +- The warning still appears under `--force` (printed before the prompt-skip path), so CI logs surface mismatches. + +**Negative:** +- A user who proceeds through the `y/N` prompt without reading the warning could still finalize the wrong variant. Acceptable given the prompt already requires deliberate `y` input. + +--- + +## Decision 2: Skip verification silently when no consolidation log exists + +**Date**: 2026-05-10 +**Status**: accepted + +### Context + +Consolidation is optional in the Orbit workflow — a user can run `orbit finalize` without ever running `orbit consolidate`. In that case there is no `consolidation-log.json` to read. + +### Decision + +When `consolidation-log.json` is missing, unreadable, or has no entries, finalize prints no warning and continues normally. Read errors are not surfaced to the user. + +### Rationale + +A "no consolidation log found" warning would fire on every finalize that wasn't preceded by consolidation, which is a normal and supported workflow. The verification feature exists to catch user error in the consolidate-then-finalize path; it should be silent when consolidation simply wasn't part of the workflow. + +### Alternatives Considered + +- **Print "no consolidation log found, skipping verification"**: Always tell the user what happened - Rejected as noise; the absence of a log is the default state for non-consolidation workflows. +- **Surface JSON parse errors**: Help the user notice a corrupt log - Rejected because it conflates "log absent" and "log corrupt" into a verification failure that blocks finalize for an unrelated reason. + +### Consequences + +**Positive:** +- Workflows that skip consolidation see no behavioural change. +- No false alarms for users who have never run consolidation. + +**Negative:** +- A genuinely corrupt `consolidation-log.json` is silently ignored. Acceptable: the consolidate command itself will surface the corruption next time it runs, and finalize is not the right place to diagnose log integrity. + +--- diff --git a/specs/finalize-show-verify/smolspec.md b/specs/finalize-show-verify/smolspec.md new file mode 100644 index 0000000..753e300 --- /dev/null +++ b/specs/finalize-show-verify/smolspec.md @@ -0,0 +1,52 @@ +# Finalize Show and Verify (T-1197) + +## Overview + +The `orbit finalize` command currently announces that it will rebase a variant's branch but tells the user nothing about which agent produced that variant or whether the variant matches the one consolidation was just run on. This change adds two display/verification steps to `cmd/orbit/finalize.go`: it shows the agent (and model, when known) for the variant being finalized, and — when a consolidation log exists — warns if the requested `--variant` does not match the most recent `chosen_variant_id` recorded in `consolidation-log.json`. The warning is informational; the user can still proceed via the existing `y/N` confirmation. + +## Requirements + +- The system MUST display the agent used for the target variant in the finalize preamble (the `This will:` block printed before the confirmation prompt). +- The system MUST include the variant's agent alias, the underlying agent type, and the model when those values are recorded on the variant; absent individual fields MUST be omitted from the rendered string so no empty parens, "model: ", or trailing punctuation appear. +- The system MUST render `Agent: unknown` when none of `Agent`, `AgentType`, or `Model` are populated on the variant. +- The system MUST read `specs//.orbit/consolidation-log.json` (when it exists) and compare the most recent entry's `chosen_variant_id` against the `--variant` flag value. +- The system MUST print a warning before the confirmation prompt when the most recent consolidation entry's `chosen_variant_id` differs from `--variant`. The warning MUST name both variant IDs and the consolidation timestamp formatted as RFC3339 (the format already used in the JSON log). +- The system MUST print the mismatch warning regardless of whether `--force` is set, so the warning is visible in logs/CI output even when the confirmation prompt is skipped. +- The system MUST allow finalize to proceed after the warning via the existing `y/N` prompt (or `--force`); no new flag is introduced. +- The system MUST treat a missing or unreadable consolidation log as "no verification possible" and continue silently without warning. +- The system MUST NOT change the existing finalize success/failure behaviour, exit codes, or rebase logic. + +## Implementation Approach + +**Key files to modify:** + +1. `cmd/orbit/finalize.go` — insert new logic between the `if targetVariant == nil { ... }` guard and the `// Show what will happen` block. Add (a) an agent-info line printed in the preamble using `targetVariant.Agent` / `targetVariant.AgentType` / `targetVariant.Model`, and (b) a consolidation-log lookup that prints a warning when the latest entry's `ChosenVariantID` differs from `*variantID`. The warning lookup MUST run before the `if !*force { ... }` block so it executes in `--force` runs too. +2. `cmd/orbit/finalize_test.go` (new) — table-driven tests covering: agent-info rendering with all/partial/no agent fields populated, mismatch warning, matching entry produces no warning, missing log file produces no warning, corrupt log file produces no warning, warning prints under `--force`. + +**Approach:** + +- Reuse the existing `consolidation.NewLogger(orbitDir)` constructor and call `Read()` to obtain the full log; the latest entry is the last element of `log.Entries`. This mirrors `cmd/orbit/consolidate.go:240`, which constructs the logger as `consolidation.NewLogger(filepath.Join(specDir, ".orbit"))`. +- Format the agent info as a single line in the preamble. Suggested rendering when all fields are present: `Agent: (, model: )`. Build the parenthetical conditionally so any of `` or `` being empty produces a clean string with no empty parens, dangling commas, or `model: ` with no value. When all three fields are empty, print `Agent: unknown`. +- Format the mismatch warning to standard output (not stderr) so it appears inline with the rest of the preamble, prefixed with `Warning:` to match the existing `Error:` formatting style used later in the file. Include the prior `chosen_variant_id`, the requested `--variant`, and the prior consolidation `timestamp` formatted as RFC3339. +- Treat any error from `Read()` (file missing, JSON parse failure, empty `Entries` slice) as "no verification" and skip silently. Do not surface log read errors to the user — consolidation is optional and absence of a log is normal. +- For tests, construct a temp repo + spec dir in `t.TempDir()` (the established pattern in `cmd/orbit/subdir_test.go`) and capture stdout using the inline `os.Pipe` / `os.Stdout = w` pattern used in `cmd/orbit/status_test.go:508-517`. Write fixture `consolidation-log.json` files directly into the temp `.orbit` directory. + +**Dependencies:** + +- `internal/consolidation` — `Logger`, `LogEntry`, `ConsolidationLog`, `NewLogger`, `Read` (all already exported and stable). +- `internal/variants` — `Variant.Agent`, `Variant.AgentType`, `Variant.Model` (already populated during runs by `Manager.UpdateAgentInfo`, see `manager.go:365`). + +**Out of Scope:** + +- Adding agent/model info to other commands (`status`, `compare`, `cleanup`). +- Persisting any new fields to `variants.json` or `consolidation-log.json`. +- Refusing to finalize on mismatch, prompting an extra confirmation, or adding a `--force-mismatch` flag (user opted for warn-only behaviour). +- Verifying anything beyond the most recent consolidation entry (older entries are not consulted). +- Auto-detecting the consolidated variant ID when `--variant` is omitted. + +## Risks and Assumptions + +- **Risk:** A user runs consolidation, then re-runs consolidation against a different variant, then finalizes the original variant — the warning will fire even though both consolidations are legitimate. **Mitigation:** Acceptable; the warning is informational and the user can still proceed. The message names the timestamp so the user can see which consolidation is being referenced. +- **Risk:** `consolidation-log.json` exists but is locked by a concurrent consolidation. **Mitigation:** `Logger.Read()` does not acquire the flock (only `Append` does), so a concurrent `Append` will not block the read; a partially-written file would fail JSON parse and be silently skipped per the missing-log handling. +- **Assumption:** `targetVariant.Agent` and `AgentType` are populated for any variant produced by a recent run; older variants from before `UpdateAgentInfo` was added may have empty fields, which is handled by the conditional rendering above. +- **Assumption:** The most recent entry in the consolidation log corresponds to the consolidation the user intends to verify against. Multiple consolidation entries are possible (rollback then re-consolidate), so "most recent" is the only correct choice — checking earlier entries would produce false negatives. diff --git a/specs/finalize-show-verify/tasks.md b/specs/finalize-show-verify/tasks.md new file mode 100644 index 0000000..237083a --- /dev/null +++ b/specs/finalize-show-verify/tasks.md @@ -0,0 +1,48 @@ +--- +references: + - specs/finalize-show-verify/smolspec.md +--- +# Finalize Show and Verify (T-1197) + +- [ ] 1. Render agent info in finalize preamble + - Implement conditional formatting in cmd/orbit/finalize.go so the preamble shows `Agent: (, model: )` when all fields are present. + - Omit any individual missing field cleanly (no empty parens, dangling commas, or `model: ` with no value). + - Fall back to `Agent: unknown` when all three of Agent, AgentType, Model are empty on the variant. + - Verify by running orbit finalize against a fixture variant manually or by running the tests added in task 2. + - Reference: smolspec.md Requirements 1-3, Implementation Approach point 1 and Format the agent info bullet. + - References: specs/finalize-show-verify/smolspec.md + +- [ ] 2. Verify agent info display behaviour with table-driven tests + - Add cmd/orbit/finalize_test.go with table-driven tests. + - Cases: all three fields populated; only Agent populated; Agent + AgentType (no Model); only Model populated; all empty (Agent: unknown). + - Use the temp repo + spec dir pattern from cmd/orbit/subdir_test.go and the inline os.Pipe / os.Stdout = w stdout-capture pattern from cmd/orbit/status_test.go:508-517. + - All tests must pass. + - Reference: smolspec.md Implementation Approach For tests bullet. + - Blocked-by: pghg6mg (Render agent info in finalize preamble) + - References: specs/finalize-show-verify/smolspec.md + +- [ ] 3. Verify chosen variant against consolidation log + - In cmd/orbit/finalize.go, read consolidation-log.json via consolidation.NewLogger(filepath.Join(specDir, ".orbit")) and Read() (mirroring cmd/orbit/consolidate.go:240). + - When the last entrys ChosenVariantID differs from the requested --variant, print a `Warning:` line to stdout naming both IDs and the prior consolidation timestamp formatted as RFC3339. + - Place the lookup BEFORE the `if !*force { ... }` block so the warning prints in --force mode. + - Treat missing file, JSON parse failure, or empty Entries slice as no-verification: print nothing and continue. + - Verify manually or via task 4. + - Reference: smolspec.md Requirements 4-7, Implementation Approach point 1 (placement note) and Format the mismatch warning / Treat any error from Read() bullets. + - Blocked-by: pghg6mh (Verify agent info display behaviour with table-driven tests) + - References: specs/finalize-show-verify/smolspec.md + +- [ ] 4. Verify mismatch warning behaviour with table-driven tests + - Extend cmd/orbit/finalize_test.go with table-driven cases. + - Cases: mismatch fires warning naming both IDs and RFC3339 timestamp; matching entry produces no warning; missing log file produces no warning; corrupt JSON log produces no warning; empty Entries slice produces no warning; warning prints when --force is passed. + - Write fixture consolidation-log.json files into a temp .orbit directory. + - All tests must pass. + - Reference: smolspec.md Implementation Approach point 2 (test scope) and For tests bullet. + - Blocked-by: pghg6mi (Verify chosen variant against consolidation log) + - References: specs/finalize-show-verify/smolspec.md + +- [ ] 5. Lint and full package test suite pass + - Run `make lint` and `make test` from the repo root; both must exit zero. + - Fix any issues introduced by tasks 1-4 (for example modernize warnings or formatting). + - No production code changes outside what tasks 1-4 require. + - Reference: project Makefile. + - Blocked-by: pghg6mj (Verify mismatch warning behaviour with table-driven tests) From ff94a493a3f85199925f239a06431d029de1c59b Mon Sep 17 00:00:00 2001 From: Arjen Schwarz Date: Sun, 10 May 2026 22:44:46 +1000 Subject: [PATCH 2/4] docs(T-1197): Add specs/OVERVIEW.md catalog Generate a chronological catalog of all feature specs under specs/, with status (Done/In Progress/Planned/No Tasks) derived from each spec's tasks.md, plus per-spec file listings linking to the underlying documents. Excludes specs/bugfixes/ (tracked separately) and the empty specs/gear-mode and specs/general placeholder directories. Includes the new finalize-show-verify entry from the previous commit on this branch. --- specs/OVERVIEW.md | 349 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 349 insertions(+) create mode 100644 specs/OVERVIEW.md diff --git a/specs/OVERVIEW.md b/specs/OVERVIEW.md new file mode 100644 index 0000000..2a7706d --- /dev/null +++ b/specs/OVERVIEW.md @@ -0,0 +1,349 @@ +# Specs Overview + +| Name | Creation Date | Status | Summary | +|------|---------------|--------|---------| +| [Custom Commands](#custom-commands) | 2025-12-22 | Done | Configure custom prompts for Orbit task orchestration instead of the hardcoded `/next-task --phase` command. | +| [Session Management](#session-management) | 2025-12-24 | Done | Improve log storage and Claude session handling with three targeted enhancements. | +| [Apsis](#apsis) | 2025-12-26 | Done | Standalone CLI converting Claude Code JSONL session transcripts into readable Markdown. | +| [HTML Export](#html-export) | 2025-12-27 | No Tasks | Render Apsis transcripts and Orbit logs as styled HTML in addition to Markdown. | +| [Collapsible Details](#collapsible-details) | 2025-12-28 | Done | Wrap long tool outputs in `
` blocks for readable Apsis transcripts. | +| [Multi Spec Comparison](#multi-spec-comparison) | 2025-12-28 | Done | Run parallel implementations of one spec in worktrees, then compare and recommend a winner. | +| [Log Improvements](#log-improvements) | 2025-12-30 | No Tasks | Tracking notes for incremental improvements to apsis/orbit session log output. | +| [Orbit UX Improvements](#orbit-ux-improvements) | 2026-01-04 | Done | Two UX enhancements to make orbit runs friendlier and clearer. | +| [Web Interface](#web-interface) | 2026-01-05 | Done | Web UI for browsing Orbit runs, transcripts, and summaries from any device. | +| [Codex Support](#codex-support) | 2026-01-07 | Done | Extend Apsis to convert OpenAI Codex CLI session transcripts alongside Claude Code. | +| [Multi Agent](#multi-agent) | 2026-01-21 | Done | Unified abstraction layer letting Orbit orchestrate sessions across multiple AI coding agents. | +| [Variant Consolidation](#variant-consolidation) | 2026-01-24 | Done | Add consolidation and rollback capabilities to Orbit's multi-variant comparison workflow. | +| [Per Variant Model Selection](#per-variant-model-selection) | 2026-01-25 | Done | Run variants on the same agent but different models via named agent aliases. | +| [Apsis Follow](#apsis-follow) | 2026-01-26 | Done | Live-tail JSONL transcripts via `apsis -F` as agent sessions progress. | +| [Enhanced Status](#enhanced-status) | 2026-01-27 | Done | Detailed real-time visibility into running and failed variants in `orbit status`. | +| [OpenCode Agent](#opencode-agent) | 2026-01-28 | Done | Add OpenCode as a supported agent for Orbit orchestration. | +| [Centralized Logging](#centralized-logging) | 2026-01-29 | Done | Write Orbit debug logs as structured JSONL into `~/.orbit/logs/`. | +| [Kiro SQLite Logs](#kiro-sqlite-logs) | 2026-01-31 | Done | Parse Kiro CLI sessions directly from SQLite, replacing the `/chat save` workaround. | +| [Orbit Command Hooks](#orbit-command-hooks) | 2026-01-31 | Done | Separate shell commands from AI prompts and add agent-level command hooks. | +| [Apsis Copilot Support](#apsis-copilot-support) | 2026-02-01 | Done | Bring Apsis GitHub Copilot session support to parity with Claude/Codex/Kiro. | +| [Auto Consolidate](#auto-consolidate) | 2026-02-01 | Done | Auto-run consolidation on the recommended variant after `orbit run --variants`. | +| [Kiro Transcript Improvements](#kiro-transcript-improvements) | 2026-02-01 | Done | Show Kiro session credits and parse Json-variant tool results in Apsis. | +| [Copilot Usage Tracking](#copilot-usage-tracking) | 2026-02-02 | Done | Parse Copilot usage stats and fix multi-unit cost display across agents. | +| [Integration Test Framework](#integration-test-framework) | 2026-02-03 | Done | Mock-agent test framework for Orbit's orchestration without invoking real CLIs. | +| [Comparison Learnings](#comparison-learnings) | 2026-02-05 | Done | Add a Learnings section to variant comparison reports with code references. | +| [Legacy Claude Removal](#legacy-claude-removal) | 2026-02-05 | Done | Remove the legacy `claudeRunner` interface and migrate remaining tests to `TestAgent`. | +| [Apsis Kiro IDE Support](#apsis-kiro-ide-support) | 2026-02-08 | Done | Support Kiro IDE `.chat` session files alongside the existing CLI sources. | +| [Apsis Serve](#apsis-serve) | 2026-02-09 | Done | Local web server for browsing Apsis sessions across all supported agents. | +| [Vibes](#vibes) | 2026-02-10 | No Tasks | Three improvements to variant comparison: timeout, JSON persistence, offline reports. | +| [Apsis Latest](#apsis-latest) | 2026-02-11 | Done | Open the most recent project session via `apsis latest`, no ID required. | +| [Shared Agent Execution](#shared-agent-execution) | 2026-02-17 | No Tasks | Extract the shared exec scaffolding duplicated across all five agent `Run()` methods. | +| [Shared Retry Executor](#shared-retry-executor) | 2026-02-18 | No Tasks | Consolidate four near-identical retry executors into a single shared implementation. | +| [Message Metadata](#message-metadata) | 2026-03-12 | Done | Show timestamps and model identifiers inline in rendered Apsis transcripts. | +| [Finalize Show Verify](#finalize-show-verify) | 2026-05-10 | Planned | Show variant agent and warn on consolidation mismatch in `orbit finalize`. | + +--- + +## Custom Commands + +Configure custom prompts for Orbit task orchestration instead of the hardcoded `/next-task --phase` command. + +- [decision_log.md](custom-commands/decision_log.md) +- [design.md](custom-commands/design.md) +- [requirements.md](custom-commands/requirements.md) +- [tasks.md](custom-commands/tasks.md) + +## Session Management + +Improve log storage and Claude session handling with three targeted enhancements. + +- [decision_log.md](session-management/decision_log.md) +- [design.md](session-management/design.md) +- [requirements.md](session-management/requirements.md) +- [tasks.md](session-management/tasks.md) + +## Apsis + +Standalone CLI converting Claude Code JSONL session transcripts into readable Markdown. + +- [decision_log.md](apsis/decision_log.md) +- [design.md](apsis/design.md) +- [requirements.md](apsis/requirements.md) +- [tasks.md](apsis/tasks.md) + +## HTML Export + +Render Apsis transcripts and Orbit logs as styled HTML in addition to Markdown. + +- [PLAN.md](html-export/PLAN.md) + +## Collapsible Details + +Wrap long tool outputs in `
` blocks for readable Apsis transcripts. + +- [decision_log.md](collapsible-details/decision_log.md) +- [design.md](collapsible-details/design.md) +- [plan.md](collapsible-details/plan.md) +- [requirements.md](collapsible-details/requirements.md) +- [tasks.md](collapsible-details/tasks.md) + +## Multi Spec Comparison + +Run parallel implementations of one spec in worktrees, then compare and recommend a winner. + +- [decision_log.md](multi-spec-comparison/decision_log.md) +- [design-2026-01-11.md](multi-spec-comparison/design-2026-01-11.md) +- [design.md](multi-spec-comparison/design.md) +- [plan.md](multi-spec-comparison/plan.md) +- [requirements.md](multi-spec-comparison/requirements.md) +- [tasks.md](multi-spec-comparison/tasks.md) + +## Log Improvements + +Tracking notes for incremental improvements to apsis/orbit session log output. + +- [session-log.md](log-improvements/session-log.md) + +## Orbit UX Improvements + +Two UX enhancements to make orbit runs friendlier and clearer. + +- [decision_log.md](orbit-ux-improvements/decision_log.md) +- [design.md](orbit-ux-improvements/design.md) +- [requirements.md](orbit-ux-improvements/requirements.md) +- [tasks.md](orbit-ux-improvements/tasks.md) + +## Web Interface + +Web UI for browsing Orbit runs, transcripts, and summaries from any device. + +- [decision_log.md](web-interface/decision_log.md) +- [design.md](web-interface/design.md) +- [PLAN.md](web-interface/PLAN.md) +- [requirements.md](web-interface/requirements.md) +- [tasks.md](web-interface/tasks.md) + +## Codex Support + +Extend Apsis to convert OpenAI Codex CLI session transcripts alongside Claude Code. + +- [decision_log.md](codex-support/decision_log.md) +- [design.md](codex-support/design.md) +- [requirements.md](codex-support/requirements.md) +- [tasks.md](codex-support/tasks.md) + +## Multi Agent + +Unified abstraction layer letting Orbit orchestrate sessions across multiple AI coding agents. + +- [decision_log.md](multi-agent/decision_log.md) +- [design.md](multi-agent/design.md) +- [plan.md](multi-agent/plan.md) +- [requirements.md](multi-agent/requirements.md) +- [tasks.md](multi-agent/tasks.md) + +## Variant Consolidation + +Add consolidation and rollback capabilities to Orbit's multi-variant comparison workflow. + +- [decision_log.md](variant-consolidation/decision_log.md) +- [design.md](variant-consolidation/design.md) +- [requirements.md](variant-consolidation/requirements.md) +- [review-fixes-1.md](variant-consolidation/review-fixes-1.md) +- [review-overview-1.md](variant-consolidation/review-overview-1.md) +- [tasks.md](variant-consolidation/tasks.md) + +## Per Variant Model Selection + +Run variants on the same agent but different models via named agent aliases. + +- [decision_log.md](per-variant-model-selection/decision_log.md) +- [design.md](per-variant-model-selection/design.md) +- [requirements.md](per-variant-model-selection/requirements.md) +- [tasks.md](per-variant-model-selection/tasks.md) + +## Apsis Follow + +Live-tail JSONL transcripts via `apsis -F` as agent sessions progress. + +- [decision_log.md](apsis-follow/decision_log.md) +- [design.md](apsis-follow/design.md) +- [requirements.md](apsis-follow/requirements.md) +- [review-fixes-1.md](apsis-follow/review-fixes-1.md) +- [review-overview-1.md](apsis-follow/review-overview-1.md) +- [tasks.md](apsis-follow/tasks.md) + +## Enhanced Status + +Detailed real-time visibility into running and failed variants in `orbit status`. + +- [decision_log.md](enhanced-status/decision_log.md) +- [design.md](enhanced-status/design.md) +- [requirements.md](enhanced-status/requirements.md) +- [review-fixes-1.md](enhanced-status/review-fixes-1.md) +- [review-overview-1.md](enhanced-status/review-overview-1.md) +- [tasks.md](enhanced-status/tasks.md) + +## OpenCode Agent + +Add OpenCode as a supported agent for Orbit orchestration. + +- [review-fixes-1.md](opencode-agent/review-fixes-1.md) +- [review-overview-1.md](opencode-agent/review-overview-1.md) +- [smolspec.md](opencode-agent/smolspec.md) +- [tasks.md](opencode-agent/tasks.md) + +## Centralized Logging + +Write Orbit debug logs as structured JSONL into `~/.orbit/logs/`. + +- [decision_log.md](centralized-logging/decision_log.md) +- [design.md](centralized-logging/design.md) +- [requirements.md](centralized-logging/requirements.md) +- [review-fixes-1.md](centralized-logging/review-fixes-1.md) +- [review-overview-1.md](centralized-logging/review-overview-1.md) +- [tasks.md](centralized-logging/tasks.md) + +## Kiro SQLite Logs + +Parse Kiro CLI sessions directly from SQLite, replacing the `/chat save` workaround. + +- [decision_log.md](kiro-sqlite-logs/decision_log.md) +- [design.md](kiro-sqlite-logs/design.md) +- [requirements.md](kiro-sqlite-logs/requirements.md) +- [tasks.md](kiro-sqlite-logs/tasks.md) + +## Orbit Command Hooks + +Separate shell commands from AI prompts and add agent-level command hooks. + +- [decision_log.md](orbit-command-hooks/decision_log.md) +- [design.md](orbit-command-hooks/design.md) +- [requirements.md](orbit-command-hooks/requirements.md) +- [review-fixes-1.md](orbit-command-hooks/review-fixes-1.md) +- [review-overview-1.md](orbit-command-hooks/review-overview-1.md) +- [tasks.md](orbit-command-hooks/tasks.md) + +## Apsis Copilot Support + +Bring Apsis GitHub Copilot session support to parity with Claude/Codex/Kiro. + +- [smolspec.md](apsis-copilot-support/smolspec.md) +- [tasks.md](apsis-copilot-support/tasks.md) + +## Auto Consolidate + +Auto-run consolidation on the recommended variant after `orbit run --variants`. + +- [smolspec.md](auto-consolidate/smolspec.md) +- [tasks.md](auto-consolidate/tasks.md) + +## Kiro Transcript Improvements + +Show Kiro session credits and parse Json-variant tool results in Apsis. + +- [smolspec.md](kiro-transcript-improvements/smolspec.md) +- [tasks.md](kiro-transcript-improvements/tasks.md) + +## Copilot Usage Tracking + +Parse Copilot usage stats and fix multi-unit cost display across agents. + +- [decision_log.md](copilot-usage-tracking/decision_log.md) +- [design.md](copilot-usage-tracking/design.md) +- [requirements.md](copilot-usage-tracking/requirements.md) +- [tasks.md](copilot-usage-tracking/tasks.md) + +## Integration Test Framework + +Mock-agent test framework for Orbit's orchestration without invoking real CLIs. + +- [decision_log.md](integration-test-framework/decision_log.md) +- [design.md](integration-test-framework/design.md) +- [requirements.md](integration-test-framework/requirements.md) +- [tasks.md](integration-test-framework/tasks.md) +- [test-migration-completion.md](integration-test-framework/test-migration-completion.md) + +## Comparison Learnings + +Add a Learnings section to variant comparison reports with code references. + +- [decision_log.md](comparison-learnings/decision_log.md) +- [design.md](comparison-learnings/design.md) +- [implementation.md](comparison-learnings/implementation.md) +- [requirements.md](comparison-learnings/requirements.md) +- [tasks.md](comparison-learnings/tasks.md) + +## Legacy Claude Removal + +Remove the legacy `claudeRunner` interface and migrate remaining tests to `TestAgent`. + +- [decision_log.md](legacy-claude-removal/decision_log.md) +- [design.md](legacy-claude-removal/design.md) +- [implementation.md](legacy-claude-removal/implementation.md) +- [requirements.md](legacy-claude-removal/requirements.md) +- [tasks.md](legacy-claude-removal/tasks.md) +- [verification-report.md](legacy-claude-removal/verification-report.md) + +## Apsis Kiro IDE Support + +Support Kiro IDE `.chat` session files alongside the existing CLI sources. + +- [decision_log.md](apsis-kiro-ide-support/decision_log.md) +- [design.md](apsis-kiro-ide-support/design.md) +- [implementation.md](apsis-kiro-ide-support/implementation.md) +- [requirements.md](apsis-kiro-ide-support/requirements.md) +- [review-fixes-1.md](apsis-kiro-ide-support/review-fixes-1.md) +- [review-overview-1.md](apsis-kiro-ide-support/review-overview-1.md) +- [tasks.md](apsis-kiro-ide-support/tasks.md) + +## Apsis Serve + +Local web server for browsing Apsis sessions across all supported agents. + +- [decision_log.md](apsis-serve/decision_log.md) +- [design.md](apsis-serve/design.md) +- [implementation.md](apsis-serve/implementation.md) +- [requirements.md](apsis-serve/requirements.md) +- [review-fixes-1.md](apsis-serve/review-fixes-1.md) +- [review-overview-1.md](apsis-serve/review-overview-1.md) +- [tasks.md](apsis-serve/tasks.md) + +## Vibes + +Three improvements to variant comparison: timeout, JSON persistence, offline reports. + +- [compare-improvements.md](vibes/compare-improvements.md) + +## Apsis Latest + +Open the most recent project session via `apsis latest`, no ID required. + +- [implementation.md](apsis-latest/implementation.md) +- [smolspec.md](apsis-latest/smolspec.md) +- [tasks.md](apsis-latest/tasks.md) + +## Shared Agent Execution + +Extract the shared exec scaffolding duplicated across all five agent `Run()` methods. + +- [smolspec.md](shared-agent-execution/smolspec.md) + +## Shared Retry Executor + +Consolidate four near-identical retry executors into a single shared implementation. + +- [smolspec.md](shared-retry-executor/smolspec.md) + +## Message Metadata + +Show timestamps and model identifiers inline in rendered Apsis transcripts. + +- [decision_log.md](message-metadata/decision_log.md) +- [design.md](message-metadata/design.md) +- [implementation.md](message-metadata/implementation.md) +- [requirements.md](message-metadata/requirements.md) +- [tasks.md](message-metadata/tasks.md) + +## Finalize Show Verify + +Show variant agent and warn on consolidation mismatch in `orbit finalize`. + +- [decision_log.md](finalize-show-verify/decision_log.md) +- [smolspec.md](finalize-show-verify/smolspec.md) +- [tasks.md](finalize-show-verify/tasks.md) From 8da1ef445e5cfa66c69d25da15f8935dcad02323 Mon Sep 17 00:00:00 2001 From: Arjen Schwarz Date: Sun, 10 May 2026 22:51:17 +1000 Subject: [PATCH 3/4] feat(T-1197): Show variant agent and warn on consolidation mismatch in orbit finalize Render the variant's agent alias, agent type, and model in the finalize preamble, falling back to "Agent: unknown" when no fields are populated. Build the parenthetical conditionally so missing fields produce a clean string with no empty parens, dangling commas, or stray "model: " labels. Read consolidation-log.json (when present) and warn before the confirmation prompt when the requested --variant differs from the most recent consolidation entry's chosen_variant_id. The lookup runs before the --force gate so the warning is visible in CI output. Missing log, parse failure, or empty Entries are treated as "no verification" and emit nothing. Tests cover all/partial/no agent fields, mismatch firing, matching entries producing no warning, missing/corrupt/empty logs, and the --force path. --- CHANGELOG.md | 5 + cmd/orbit/finalize.go | 55 +++++ cmd/orbit/finalize_test.go | 311 ++++++++++++++++++++++++++++ specs/OVERVIEW.md | 2 +- specs/finalize-show-verify/tasks.md | 10 +- 5 files changed, 377 insertions(+), 6 deletions(-) create mode 100644 cmd/orbit/finalize_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 950d2e4..3097dcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `orbit finalize` now displays the variant's agent (alias, type, model) in its preamble, falling back to "Agent: unknown" when none of the fields are populated. +- `orbit finalize` now reads the spec's `consolidation-log.json` and prints a warning before the confirmation prompt when the requested `--variant` differs from the most recent consolidation entry. The warning fires under `--force` as well so it remains visible in CI output. + ## [0.9.0] - 2026-04-11 Initial public release of Orbit and Apsis. diff --git a/cmd/orbit/finalize.go b/cmd/orbit/finalize.go index 7791158..192353b 100644 --- a/cmd/orbit/finalize.go +++ b/cmd/orbit/finalize.go @@ -8,7 +8,9 @@ import ( "os" "path/filepath" "strings" + "time" + "github.com/arjenschwarz/orbit/internal/consolidation" "github.com/arjenschwarz/orbit/internal/variants" ) @@ -96,12 +98,17 @@ func finalizeCommand(args []string) error { // Show what will happen fmt.Printf("Finalize spec: %s\n\n", specName) + fmt.Printf("%s\n", formatVariantAgentInfo(targetVariant)) fmt.Printf("This will:\n") fmt.Printf(" 1. Rebase variant %d (%s) onto %s\n", targetVariant.ID, targetVariant.Branch, metadata.OriginalBranch) fmt.Printf(" 2. Remove all variant worktrees\n") fmt.Printf(" 3. Delete all variant branches\n") fmt.Println() + // Verify against the most recent consolidation log entry, if any. + // Runs before the force-gate so the warning is visible in --force / CI runs. + printConsolidationMismatchWarning(filepath.Join(specDir, ".orbit"), *variantID) + // Confirm unless force flag is set if !*force { fmt.Print("Proceed with finalize? [y/N] ") @@ -145,3 +152,51 @@ func finalizeCommand(args []string) error { return nil } + +// printConsolidationMismatchWarning reads consolidation-log.json under orbitDir +// and prints a warning when the most recent entry's ChosenVariantID differs +// from the requested variantID. Missing log, parse failure, or empty entries +// are treated as "no verification possible" and print nothing. +func printConsolidationMismatchWarning(orbitDir string, variantID int) { + log, err := consolidation.NewLogger(orbitDir).Read() + if err != nil { + return + } + if len(log.Entries) == 0 { + return + } + latest := log.Entries[len(log.Entries)-1] + if latest.ChosenVariantID == variantID { + return + } + fmt.Printf("Warning: variant %d does not match the most recent consolidation (variant %d, %s)\n\n", + variantID, latest.ChosenVariantID, latest.Timestamp.Format(time.RFC3339)) +} + +// formatVariantAgentInfo renders an "Agent: ..." line for the finalize preamble. +// When all of Agent, AgentType, and Model are empty, returns "Agent: unknown". +// Otherwise, builds "Agent: (, model: )" with each parenthetical +// piece omitted cleanly when its source field is empty. +func formatVariantAgentInfo(v *variants.Variant) string { + if v.Agent == "" && v.AgentType == "" && v.Model == "" { + return "Agent: unknown" + } + + var parens []string + if v.AgentType != "" { + parens = append(parens, v.AgentType) + } + if v.Model != "" { + parens = append(parens, "model: "+v.Model) + } + + var parts []string + if v.Agent != "" { + parts = append(parts, v.Agent) + } + if len(parens) > 0 { + parts = append(parts, "("+strings.Join(parens, ", ")+")") + } + + return "Agent: " + strings.Join(parts, " ") +} diff --git a/cmd/orbit/finalize_test.go b/cmd/orbit/finalize_test.go new file mode 100644 index 0000000..98d9301 --- /dev/null +++ b/cmd/orbit/finalize_test.go @@ -0,0 +1,311 @@ +package main + +import ( + "encoding/json" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/arjenschwarz/orbit/internal/consolidation" + "github.com/arjenschwarz/orbit/internal/variants" +) + +// setupFinalizeRepo creates a git repo with a single completed variant whose +// agent fields are configurable. It returns the repo root. +func setupFinalizeRepo(t *testing.T, specName string, agent, agentType, model string) string { + t.Helper() + tmpDir := t.TempDir() + + for _, args := range [][]string{ + {"init"}, + {"config", "user.email", "test@test.com"}, + {"config", "user.name", "Test"}, + } { + cmd := exec.Command("git", args...) + cmd.Dir = tmpDir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %s failed: %s", args[0], out) + } + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README.md"), []byte("# Test"), 0644)) + cmd := exec.Command("git", "add", ".") + cmd.Dir = tmpDir + require.NoError(t, cmd.Run()) + cmd = exec.Command("git", "commit", "-m", "init") + cmd.Dir = tmpDir + require.NoError(t, cmd.Run()) + + cmd = exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = tmpDir + out, err := cmd.Output() + require.NoError(t, err) + baseCommit := strings.TrimSpace(string(out)) + + specOrbitDir := filepath.Join(tmpDir, "specs", specName, ".orbit") + require.NoError(t, os.MkdirAll(specOrbitDir, 0755)) + worktreePath := filepath.Join(specOrbitDir, "worktrees", "orbit-impl-1-"+specName) + require.NoError(t, os.MkdirAll(worktreePath, 0755)) + + metadata := variants.VariantsMetadata{ + RunID: "test-run", + BaseCommit: baseCommit, + OriginalBranch: "main", + StartedAt: time.Now(), + Variants: []*variants.Variant{ + { + ID: 1, + Branch: "orbit-impl-1-" + specName, + WorktreePath: worktreePath, + Status: variants.StatusCompleted, + Agent: agent, + AgentType: agentType, + Model: model, + }, + }, + } + + data, err := json.MarshalIndent(metadata, "", " ") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(specOrbitDir, "variants.json"), data, 0644)) + + return tmpDir +} + +// runFinalizeCapture runs finalizeCommand with the given args, redirecting +// stdin to "n\n" so the confirmation prompt cancels (avoiding any rebase), +// and returns captured stdout plus any returned error. +func runFinalizeCapture(t *testing.T, args []string) (string, error) { + t.Helper() + + originalStdin := os.Stdin + stdinR, stdinW, err := os.Pipe() + require.NoError(t, err) + os.Stdin = stdinR + _, _ = stdinW.WriteString("n\n") + _ = stdinW.Close() + t.Cleanup(func() { os.Stdin = originalStdin }) + + originalStdout := os.Stdout + stdoutR, stdoutW, err := os.Pipe() + require.NoError(t, err) + os.Stdout = stdoutW + + cmdErr := finalizeCommand(args) + + _ = stdoutW.Close() + os.Stdout = originalStdout + + captured, readErr := io.ReadAll(stdoutR) + require.NoError(t, readErr) + + return string(captured), cmdErr +} + +func TestFormatVariantAgentInfo(t *testing.T) { + tests := map[string]struct { + agent string + agentType string + model string + want string + }{ + "all three populated": { + agent: "primary", + agentType: "claude-code", + model: "claude-opus-4-7", + want: "Agent: primary (claude-code, model: claude-opus-4-7)", + }, + "only agent populated": { + agent: "primary", + want: "Agent: primary", + }, + "agent and type, no model": { + agent: "primary", + agentType: "claude-code", + want: "Agent: primary (claude-code)", + }, + "only model populated": { + model: "claude-opus-4-7", + want: "Agent: (model: claude-opus-4-7)", + }, + "all empty": { + want: "Agent: unknown", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + v := &variants.Variant{ + Agent: tc.agent, + AgentType: tc.agentType, + Model: tc.model, + } + got := formatVariantAgentInfo(v) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestFinalizeCommand_AgentInfoPreamble(t *testing.T) { + tests := map[string]struct { + agent string + agentType string + model string + wantLine string + }{ + "all three populated": { + agent: "primary", + agentType: "claude-code", + model: "claude-opus-4-7", + wantLine: "Agent: primary (claude-code, model: claude-opus-4-7)", + }, + "only agent populated": { + agent: "primary", + wantLine: "Agent: primary", + }, + "agent and type, no model": { + agent: "primary", + agentType: "claude-code", + wantLine: "Agent: primary (claude-code)", + }, + "only model populated": { + model: "claude-opus-4-7", + wantLine: "Agent: (model: claude-opus-4-7)", + }, + "all empty": { + wantLine: "Agent: unknown", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + specName := "agent-info-" + strings.ReplaceAll(name, " ", "-") + repoRoot := setupFinalizeRepo(t, specName, tc.agent, tc.agentType, tc.model) + + originalWd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(originalWd) }) + require.NoError(t, os.Chdir(repoRoot)) + + out, _ := runFinalizeCapture(t, []string{"--variant", "1", specName}) + assert.Contains(t, out, tc.wantLine) + }) + } +} + +// writeConsolidationLog writes a fixture consolidation-log.json containing the +// given entries to the spec's .orbit directory. +func writeConsolidationLog(t *testing.T, repoRoot, specName string, entries []consolidation.LogEntry) { + t.Helper() + orbitDir := filepath.Join(repoRoot, "specs", specName, ".orbit") + log := consolidation.ConsolidationLog{ + SchemaVersion: consolidation.SchemaVersion, + Entries: entries, + } + data, err := json.MarshalIndent(log, "", " ") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(orbitDir, "consolidation-log.json"), data, 0644)) +} + +// writeRawConsolidationLog writes raw bytes to consolidation-log.json (for +// corrupt-JSON test fixtures). +func writeRawConsolidationLog(t *testing.T, repoRoot, specName string, data []byte) { + t.Helper() + orbitDir := filepath.Join(repoRoot, "specs", specName, ".orbit") + require.NoError(t, os.WriteFile(filepath.Join(orbitDir, "consolidation-log.json"), data, 0644)) +} + +func TestFinalizeCommand_ConsolidationMismatchWarning(t *testing.T) { + priorTimestamp := time.Date(2026, 5, 1, 12, 30, 0, 0, time.UTC) + priorRFC3339 := priorTimestamp.Format(time.RFC3339) + + mismatchEntry := consolidation.LogEntry{ + Timestamp: priorTimestamp, + ChosenVariantID: 2, + Agent: "claude-code", + } + matchingEntry := consolidation.LogEntry{ + Timestamp: priorTimestamp, + ChosenVariantID: 1, + Agent: "claude-code", + } + + tests := map[string]struct { + setupLog func(t *testing.T, repoRoot, specName string) + args []string + wantWarning bool + }{ + "mismatch fires warning": { + setupLog: func(t *testing.T, repoRoot, specName string) { + writeConsolidationLog(t, repoRoot, specName, []consolidation.LogEntry{mismatchEntry}) + }, + args: []string{"--variant", "1"}, + wantWarning: true, + }, + "matching entry produces no warning": { + setupLog: func(t *testing.T, repoRoot, specName string) { + writeConsolidationLog(t, repoRoot, specName, []consolidation.LogEntry{matchingEntry}) + }, + args: []string{"--variant", "1"}, + wantWarning: false, + }, + "missing log file produces no warning": { + setupLog: func(t *testing.T, repoRoot, specName string) {}, + args: []string{"--variant", "1"}, + wantWarning: false, + }, + "corrupt json log produces no warning": { + setupLog: func(t *testing.T, repoRoot, specName string) { + writeRawConsolidationLog(t, repoRoot, specName, []byte("{not valid json")) + }, + args: []string{"--variant", "1"}, + wantWarning: false, + }, + "empty entries slice produces no warning": { + setupLog: func(t *testing.T, repoRoot, specName string) { + writeConsolidationLog(t, repoRoot, specName, []consolidation.LogEntry{}) + }, + args: []string{"--variant", "1"}, + wantWarning: false, + }, + "warning prints under force": { + setupLog: func(t *testing.T, repoRoot, specName string) { + writeConsolidationLog(t, repoRoot, specName, []consolidation.LogEntry{mismatchEntry}) + }, + args: []string{"--variant", "1", "--force"}, + wantWarning: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + specName := "mismatch-" + strings.ReplaceAll(name, " ", "-") + repoRoot := setupFinalizeRepo(t, specName, "primary", "claude-code", "claude-opus-4-7") + tc.setupLog(t, repoRoot, specName) + + originalWd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(originalWd) }) + require.NoError(t, os.Chdir(repoRoot)) + + args := append([]string{}, tc.args...) + args = append(args, specName) + out, _ := runFinalizeCapture(t, args) + + if tc.wantWarning { + assert.Contains(t, out, "Warning:", "expected warning in output") + assert.Contains(t, out, "variant 1", "expected requested variant ID in warning") + assert.Contains(t, out, "variant 2", "expected prior chosen variant ID in warning") + assert.Contains(t, out, priorRFC3339, "expected RFC3339 timestamp in warning") + } else { + assert.NotContains(t, out, "Warning:", "expected no warning in output") + } + }) + } +} diff --git a/specs/OVERVIEW.md b/specs/OVERVIEW.md index 2a7706d..aacf8b8 100644 --- a/specs/OVERVIEW.md +++ b/specs/OVERVIEW.md @@ -35,7 +35,7 @@ | [Shared Agent Execution](#shared-agent-execution) | 2026-02-17 | No Tasks | Extract the shared exec scaffolding duplicated across all five agent `Run()` methods. | | [Shared Retry Executor](#shared-retry-executor) | 2026-02-18 | No Tasks | Consolidate four near-identical retry executors into a single shared implementation. | | [Message Metadata](#message-metadata) | 2026-03-12 | Done | Show timestamps and model identifiers inline in rendered Apsis transcripts. | -| [Finalize Show Verify](#finalize-show-verify) | 2026-05-10 | Planned | Show variant agent and warn on consolidation mismatch in `orbit finalize`. | +| [Finalize Show Verify](#finalize-show-verify) | 2026-05-10 | Done | Show variant agent and warn on consolidation mismatch in `orbit finalize`. | --- diff --git a/specs/finalize-show-verify/tasks.md b/specs/finalize-show-verify/tasks.md index 237083a..ef84d3d 100644 --- a/specs/finalize-show-verify/tasks.md +++ b/specs/finalize-show-verify/tasks.md @@ -4,7 +4,7 @@ references: --- # Finalize Show and Verify (T-1197) -- [ ] 1. Render agent info in finalize preamble +- [x] 1. Render agent info in finalize preamble - Implement conditional formatting in cmd/orbit/finalize.go so the preamble shows `Agent: (, model: )` when all fields are present. - Omit any individual missing field cleanly (no empty parens, dangling commas, or `model: ` with no value). - Fall back to `Agent: unknown` when all three of Agent, AgentType, Model are empty on the variant. @@ -12,7 +12,7 @@ references: - Reference: smolspec.md Requirements 1-3, Implementation Approach point 1 and Format the agent info bullet. - References: specs/finalize-show-verify/smolspec.md -- [ ] 2. Verify agent info display behaviour with table-driven tests +- [x] 2. Verify agent info display behaviour with table-driven tests - Add cmd/orbit/finalize_test.go with table-driven tests. - Cases: all three fields populated; only Agent populated; Agent + AgentType (no Model); only Model populated; all empty (Agent: unknown). - Use the temp repo + spec dir pattern from cmd/orbit/subdir_test.go and the inline os.Pipe / os.Stdout = w stdout-capture pattern from cmd/orbit/status_test.go:508-517. @@ -21,7 +21,7 @@ references: - Blocked-by: pghg6mg (Render agent info in finalize preamble) - References: specs/finalize-show-verify/smolspec.md -- [ ] 3. Verify chosen variant against consolidation log +- [x] 3. Verify chosen variant against consolidation log - In cmd/orbit/finalize.go, read consolidation-log.json via consolidation.NewLogger(filepath.Join(specDir, ".orbit")) and Read() (mirroring cmd/orbit/consolidate.go:240). - When the last entrys ChosenVariantID differs from the requested --variant, print a `Warning:` line to stdout naming both IDs and the prior consolidation timestamp formatted as RFC3339. - Place the lookup BEFORE the `if !*force { ... }` block so the warning prints in --force mode. @@ -31,7 +31,7 @@ references: - Blocked-by: pghg6mh (Verify agent info display behaviour with table-driven tests) - References: specs/finalize-show-verify/smolspec.md -- [ ] 4. Verify mismatch warning behaviour with table-driven tests +- [x] 4. Verify mismatch warning behaviour with table-driven tests - Extend cmd/orbit/finalize_test.go with table-driven cases. - Cases: mismatch fires warning naming both IDs and RFC3339 timestamp; matching entry produces no warning; missing log file produces no warning; corrupt JSON log produces no warning; empty Entries slice produces no warning; warning prints when --force is passed. - Write fixture consolidation-log.json files into a temp .orbit directory. @@ -40,7 +40,7 @@ references: - Blocked-by: pghg6mi (Verify chosen variant against consolidation log) - References: specs/finalize-show-verify/smolspec.md -- [ ] 5. Lint and full package test suite pass +- [x] 5. Lint and full package test suite pass - Run `make lint` and `make test` from the repo root; both must exit zero. - Fix any issues introduced by tasks 1-4 (for example modernize warnings or formatting). - No production code changes outside what tasks 1-4 require. From e8a744f7dc1a4441b27687599b255390e9d43049 Mon Sep 17 00:00:00 2001 From: Arjen Schwarz Date: Sun, 10 May 2026 22:57:12 +1000 Subject: [PATCH 4/4] docs(T-1197): Add implementation explanation for finalize show-and-verify Three-level explanation (beginner / intermediate / expert) of the cmd/orbit/finalize.go changes plus a Completeness Assessment table mapping each numbered requirement in smolspec.md to the code that satisfies it. Generated as part of pre-push-review validation; all nine requirements explained cleanly with no gaps. --- specs/finalize-show-verify/implementation.md | 117 +++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 specs/finalize-show-verify/implementation.md diff --git a/specs/finalize-show-verify/implementation.md b/specs/finalize-show-verify/implementation.md new file mode 100644 index 0000000..2dfeb18 --- /dev/null +++ b/specs/finalize-show-verify/implementation.md @@ -0,0 +1,117 @@ +# Implementation: Finalize Show and Verify (T-1197) + +This note explains the changes that landed under `specs/finalize-show-verify/` — what was added, how it works, and how each numbered requirement in `smolspec.md` maps to code. + +## Beginner Level + +### What Changed / What This Does + +`orbit finalize` is the command you run when you have several variant implementations of a spec sitting in side-by-side worktrees and you've picked a winner. It rebases the winner onto your original branch and tidies up the rest. Before this change, the preamble it printed only said *which* variant you'd picked — not *who* implemented it (which AI agent, which model) or whether the variant you picked is actually the one you just consolidated against. + +Two things were added to that preamble: + +1. An **"Agent: ..." line** that names the agent alias, agent type (e.g., `claude-code`), and model used to produce the variant. +2. A **"Warning: ..." line** that fires when the variant ID you passed to `--variant` doesn't match the most recent consolidation entry. It only appears when there's a real mismatch; if you never ran `orbit consolidate`, or the variants line up, nothing extra is printed. + +### Why It Matters + +When you've been comparing three or four variants from different agents, it's easy to lose track. Showing the agent up front confirms you're finalizing what you think you are. The warning catches a specific footgun: running `orbit consolidate --variant 2` and then accidentally `orbit finalize --variant 1` ships the un-consolidated branch. + +### Key Concepts + +- **Variant**: A standalone implementation attempt living in its own git worktree. Multiple variants run side-by-side so you can compare. +- **Consolidation**: A separate step where the chosen variant absorbs improvements from the others. Each consolidation is logged in `consolidation-log.json`. +- **Finalize**: Adopt one variant as the final implementation, rebase it, delete the rest. + +--- + +## Intermediate Level + +### Changes Overview + +Production change is contained to `cmd/orbit/finalize.go`: + +- New imports: `time` and `internal/consolidation`. +- New helper `formatVariantAgentInfo(v *variants.Variant) string` (`finalize.go:180`). +- New helper `printConsolidationMismatchWarning(orbitDir string, variantID int)` (`finalize.go:160`). +- Two call sites in `finalizeCommand`: + - `finalize.go:101` — print agent info inside the existing preamble. + - `finalize.go:110` — print mismatch warning, deliberately placed *before* the `if !*force` block. + +Tests live in the new `cmd/orbit/finalize_test.go` (table-driven, ~310 lines) and use the established temp-repo + stdout-capture patterns from `cmd/orbit/subdir_test.go` and `cmd/orbit/status_test.go`. + +`CHANGELOG.md` gained two `Unreleased > Added` entries; `specs/OVERVIEW.md` registers the spec in the catalog. No CLAUDE.md changes were needed (no new flags, env vars, or config keys). + +### Implementation Approach + +- **Agent line.** `formatVariantAgentInfo` builds the string conditionally. If all three of `Agent`, `AgentType`, `Model` are empty it returns `Agent: unknown`. Otherwise it appends the alias, then a parenthetical containing whichever of `` and `model: ` are populated. Empty fields produce no empty parens, no dangling commas, no `model: ` with nothing after it. +- **Mismatch warning.** Reuses `consolidation.NewLogger(filepath.Join(specDir, ".orbit"))` and `Logger.Read()` (mirrors `cmd/orbit/consolidate.go:240`). The latest entry is `log.Entries[len(log.Entries)-1]`. If `Read()` returns an error, or `Entries` is empty, the helper returns silently — that's the documented "no verification possible" case. When the entry exists and `ChosenVariantID != variantID`, it prints `Warning: variant N does not match the most recent consolidation (variant M, )\n\n` to stdout. +- **Placement.** The warning lookup runs *before* the `--force` short-circuit so the line is visible in CI / non-interactive runs. + +### Trade-offs + +- **Warn vs. hard error.** Decision log entry 1 records the choice: a hard error would block legitimate "I changed my mind" workflows where the user deliberately ships the un-consolidated original. The existing `y/N` prompt already serves as the deliberate acknowledgement. +- **Silent on missing/corrupt log.** Decision log entry 2: most finalize runs aren't preceded by consolidation, so a "no log found" line would be noise. Surfacing JSON parse errors was rejected because it conflates absence with corruption and blocks finalize for an unrelated reason. +- **No `Logger.Latest()` helper.** The reach into `log.Entries[len-1]` is duplicated between `internal/consolidation/logger.go:137` (`GetLatestCommitSHA`) and the new code. Encapsulation could be improved, but it's a two-call surface — out of scope for this spec. + +--- + +## Expert Level + +### Technical Deep Dive + +- **Lock-free read.** `Logger.Read()` does not take the flock that `Append` uses, so a concurrent consolidation does not block the finalize preamble. A partially-written log fails JSON parse and is treated as "no verification possible" — same path as a missing file. +- **Stdout vs. stderr.** The warning is intentionally on stdout, matching the rest of the preamble formatting and the existing `Error:`-style messages later in the file. CI consumers reading stdout get the warning naturally. +- **Force semantics.** The placement of `printConsolidationMismatchWarning` at `finalize.go:110`, before `:113`, means automated `--force` runs still emit the warning to logs. The function is not gated on interactivity at all. +- **All-empty fallback.** `formatVariantAgentInfo` checks all three fields up front and returns `Agent: unknown` only when all are empty. With at least one populated, it never emits an empty-paren `()`, a dangling `, `, or a bare `model: `. +- **Sequencing constraint.** Tests mutate `os.Stdout` and `os.Chdir` globally and therefore can't use `t.Parallel()`. This is consistent with `status_test.go` and `subdir_test.go`. A future contributor who adds `t.Parallel()` here would silently corrupt other tests' captures — worth keeping in mind if the test surface grows. + +### Architecture Impact + +- The change is additive and contained to the finalize command surface. No exported API change. No schema change to `variants.json` or `consolidation-log.json`. +- The `consolidation.Logger.Read()` API is now consumed by two callers (`consolidate.go` and `finalize.go`). Adding an explicit `Latest() (LogEntry, bool)` method would consolidate the empty-check + last-element pattern; today it's open-coded twice. Worth doing the next time the consolidation package is touched, but not warranted on this branch. +- Out-of-scope items the spec called out are honoured: no changes to `status` / `compare` / `cleanup`; no `--force-mismatch` flag; no auto-detection of the consolidated variant when `--variant` is omitted. + +### Potential Issues + +- **Quiet corrupt log.** A genuinely corrupt `consolidation-log.json` is silently ignored. Decision log entry 2 accepts this: the next `consolidate` run will surface the corruption, and finalize is not the right diagnostic site. +- **Repeat consolidation, then finalize the original.** Documented risk in `smolspec.md`. The warning fires; the user can still proceed via `y/N` or `--force`. The warning includes the consolidation timestamp so the user can match it against shell history. +- **Older variants without agent fields.** `targetVariant.Agent`/`AgentType`/`Model` may be empty for variants produced before `Manager.UpdateAgentInfo` was added. `formatVariantAgentInfo` handles each empty subset, including all-empty, so no crash and no garbage output. + +--- + +## Completeness Assessment + +Each numbered requirement in `smolspec.md` mapped to the code that implements it. + +| # | Requirement | Implementation | +|---|-------------|----------------| +| 1 | Display the agent used for the target variant in the finalize preamble | `cmd/orbit/finalize.go:101` calls `formatVariantAgentInfo(targetVariant)` between `Finalize spec:` and the `This will:` block. | +| 2 | Include alias, type, and model when recorded; absent fields omitted cleanly | `formatVariantAgentInfo` (`finalize.go:180-202`) builds the parenthetical from `AgentType` and `model: ` only when populated, then joins with the alias. Test cases at `finalize_test.go:118-148` cover all-three, only-agent, agent+type, and only-model. | +| 3 | `Agent: unknown` when none of `Agent`, `AgentType`, `Model` are populated | `finalize.go:181-183` — explicit early return when all three are empty. Test at `finalize_test.go:144-147`. | +| 4 | Read `specs//.orbit/consolidation-log.json` (when it exists) and compare its most recent `chosen_variant_id` against `--variant` | `printConsolidationMismatchWarning` (`finalize.go:160-174`) constructs `consolidation.NewLogger(orbitDir).Read()`; latest entry via `log.Entries[len(log.Entries)-1]`. | +| 5 | Print warning before the confirmation prompt naming both IDs and the consolidation timestamp as RFC3339 | `finalize.go:172-173` — `Warning: variant does not match the most recent consolidation (variant , )`. Asserted by `finalize_test.go:303-305`. | +| 6 | Warning prints regardless of `--force` | Call placed at `finalize.go:110`, before `if !*force` at `:113`. Asserted by the `warning prints under force` test case (`finalize_test.go:289-294`). | +| 7 | Allow finalize to proceed via the existing `y/N` prompt or `--force`; no new flag | No flag added. Existing `bufio.NewReader(os.Stdin).ReadString('\n')` flow at `finalize.go:114-124` is untouched. | +| 8 | Treat missing or unreadable consolidation log as "no verification possible" — silent | `finalize.go:162-167` — early returns on `Read()` error or empty `Entries`. `missing log file produces no warning`, `corrupt json log produces no warning`, and `empty entries slice produces no warning` test cases all assert no `Warning:` string. | +| 9 | No change to existing finalize success/failure behaviour, exit codes, or rebase logic | Diff is purely additive (two helper functions and three lines in `finalizeCommand`). Rebase path at `finalize.go:127-148` is byte-identical to `origin/main`. | + +### Out-of-Scope Compliance + +The smolspec listed five out-of-scope items. None were implemented: + +- No agent/model display added to `status`, `compare`, or `cleanup`. +- No new fields persisted to `variants.json` or `consolidation-log.json`. +- No `--force-mismatch` flag; no refusal-to-finalize on mismatch; no extra confirmation prompt. +- Only the most recent consolidation entry is consulted. +- `--variant` is still required; no auto-detection. + +### Validation Findings + +**Gaps identified:** none. Every numbered requirement has a clean explanation grounded in specific code. + +**Logic issues:** none. The "only model populated" path produces `Agent: (model: )` — a slightly awkward leading space before `(` because the alias slot is empty. The spec's wording (no empty parens, no dangling `model: `, no trailing punctuation) is satisfied; the leading-space reading is locked into `finalize_test.go:142`. Treated as intentional, not a defect. + +**Questions raised:** the `log.Entries[len-1]` indexing is duplicated in `internal/consolidation/logger.go:137`. A `Logger.Latest()` helper would be cleaner; deferring to a future package touch. + +**Recommendations:** none for this branch. Both items above are intentionally out of scope per the smolspec and decision log.