* fix: fail send when killed session delivery is not confirmed
* fix(core): drop requireConfirmation throw that re-introduced duplicate-message bug
The PR's sendWithConfirmation added a throw when confirmation heuristics
did not flip within SEND_CONFIRMATION_ATTEMPTS on a restored session.
But runtimePlugin.sendMessage had already fired, so the throw bubbled up
to the lifecycle manager's catch-all, leaving lastCIFailureDispatchHash
(and its merge-conflict twin) unset. Next poll re-dispatched the same
message — exactly the duplicate-message bug that commit 77685a5 removed.
Real fix for #1074 is preserved: restoreForDelivery still throws when
waitForRestoredSession returns false, which happens before sendMessage
fires. Killed sessions that cannot be revived are still reported as
failures, with no duplicate-send risk.
- sendWithConfirmation no longer takes requireConfirmation; unconfirmed
delivery always returns (soft success).
- prepareSession returns Session (the tuple only existed to drive the
removed throw).
- send's retry predicate reverts to prepared.restoredAt === undefined
&& isRestorable(prepared).
- Adds regression test: restored session + sendMessage fires +
confirmation never flips → send() resolves.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* feat(cli): filter terminated sessions from ao session ls / ao status by default
Closes#1310. Terminated sessions (killed/terminated/done/merged/errored/cleanup,
plus lifecycle-driven terminal states) are now hidden from `ao session ls` and
`ao status` by default. A dim footer reports how many were hidden and how to
surface them. Pass `--include-terminated` to restore the full list.
JSON output wraps into `{ data: [...], meta: { hiddenTerminatedCount } }` on
both commands so text and machine-readable views tell the same story. This is a
breaking change for script consumers of `--json`; `--include-terminated` is the
escape hatch.
Orthogonal to `-a, --all` (orchestrator visibility, unchanged). Restore of
terminated sessions by id is unaffected — that path goes through `sm.get`, not
`sm.list`.
Docs (`SETUP.md`, `docs/CLI.md`) updated to match. Tests cover both the legacy
status branch and the canonical lifecycle branch of `isTerminalSession`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): drop unused `lc` param in lifecycle-alive test case
ESLint's no-unused-vars rejects unprefixed unused args. The "alive — should
remain visible" branch of the new lifecycle-driven filter test in
`session.test.ts` took `lc` but never touched it. Switch to `()`. Matches the
equivalent case in `status.test.ts`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(core): preserve pr.state=merged when legacy metadata lacks pr= URL
Review blocker on PR #1340: a metadata file with `status=merged` but no `pr=`
URL was still showing as active in `ao session ls` / `ao status` by default.
Root cause: `synthesizePRState()` in lifecycle-state.ts short-circuited to
`{ state: "none" }` whenever no PR URL was present, ignoring the fact that the
legacy `status` column already encodes terminal truth. Once lifecycle was
synthesized as `session.state="idle"` + `pr.state="none"`, `deriveLegacyStatus`
returned `"idle"` and `isTerminalSession()` (lifecycle branch) returned false.
The new CLI filter then let the session through.
Fix: when legacy `status === "merged"` and no URL is available, synthesize
`pr.state="merged", reason="merged"` with `number: null, url: null`. The
terminal signal survives the flat-metadata → canonical-lifecycle round trip.
Also:
- Export `sessionFromMetadata` from the core barrel. CLI tests and external
consumers need it to round-trip metadata through the canonical lifecycle.
- Update CLI `buildSessionsFromDir` helpers to route through `sessionFromMetadata`
so mocked `sm.list()` reflects production reconstruction (the old shortcut
bypassed synthesis entirely and was the reason the bug slipped past the
original test suite).
- Add regression tests: one at the core level (`parseCanonicalLifecycle` for
merged-without-URL) and one integration-style test per CLI command asserting
the reviewer's exact repro produces the expected filtered output.
- One pre-existing test expectation updated: when metadata has `status=working`
and `pr=<url>`, the reconstructed status is `pr_open`, not `working`. That's
what production `sm.list()` has always returned; the test was previously
hiding behind the reconstruction shortcut.
Changeset bumped to include ao-core (patch).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(cli): route review-check helper through sessionFromMetadata
Last remaining test-fidelity shortcut flagged by codex on PR #1340. The
`buildSessionsFromDir` helper in review-check.test.ts fabricated Session
objects by hand, bypassing the canonical lifecycle reconstruction that
production `sm.list()` runs. Doesn't affect review-check's actual behavior
(which reads `session.metadata["pr"]` directly), but aligns this test with
the equivalent helpers in session.test.ts and status.test.ts so future
lifecycle changes don't silently skip this surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes#1048. ao start used to allocate a fresh `{prefix}-orchestrator-N`
on every invocation instead of reattaching to the previous session, and
the dashboard's orchestrator link pointed at a different id than the
CLI just printed.
Changes:
runStartup (packages/cli/src/commands/start.ts):
- On startup, list existing orchestrators for the project, partition
them into live (runtime still running) and restorable (terminal but
sm.restore()-able) buckets, pick the most-recently-active from the
chosen bucket, and reuse/restore that id instead of spawning a new
one. Only spawn fresh when both buckets are empty.
- Live is preferred UNCONDITIONALLY over restorable — a newer killed
record can never beat an older-but-running one. Without this, a
cross-bucket sort could resurrect a killed record via sm.restore()
while the live orchestrator kept running, leaving two alive.
- Restored sessions get an explicit "(restored)" marker in the CLI
summary so the resurfaced id isn't a surprise.
- The phantom `${prefix}-orchestrator` id constant is removed from
every URL print, browser-open target, and summary line. Everything
now uses the real selected id.
registerStop (same file):
- ao stop now resolves the real orchestrator via sm.list(projectId)
+ isOrchestratorSession filter + most-recently-active sort, then
calls sm.kill on that id. The old phantom `${prefix}-orchestrator`
target never matched a real numbered record, so ao stop was a
silent no-op and the orchestrator kept running on disk between
start cycles. sm.list-failure warning no longer duplicates with the
generic "no orchestrator found" message.
isOrchestratorSession (packages/core/src/types.ts):
- Tightened: legacy bare-id records (`{projectId}-orchestrator` with
no role metadata) are no longer recognized as orchestrators by the
public predicate. This was the source of the dashboard/CLI id
divergence — stale bare records with a different prefix than the
numbered form were leaking into the dashboard's orchestrator list.
session-manager repair (packages/core/src/session-manager.ts):
- Split `isOrchestratorSessionRecord` (permissive, used by cleanup
protection) from a new `isRepairableOrchestratorRecord` (stricter,
used only by repairSingleSessionMetadataOnRead and
repairSessionMetadataOnRead).
- The strict repair predicate accepts role-stamped records, the bare
`{sessionPrefix}-orchestrator` correct-prefix legacy shape, and the
numbered `{sessionPrefix}-orchestrator-N` worktree shape. It
rejects foreign bare names like `{projectId}-orchestrator`, so
those records never get `role: orchestrator` backfilled on read
and therefore can no longer pass `isOrchestratorSession()` in real
`sm.list()` output via the role-metadata branch.
Tests added (~12):
- runStartup: live reuse, restore-on-killed, ignore-stale-bare
legacy records, live-beats-restorable regression, multi-live
reuse, URL fallback when --no-orchestrator.
- ao stop: kills the actual numbered id (not the phantom), handles
multiple orchestrators, tolerates sm.list throwing.
- isOrchestratorSession: rejects stale bare ids without role
metadata; accepts bare ids with role metadata stamped.
- listDashboardOrchestrators: stale bare excluded, numbered live
included, role-stamped legacy included.
- session-manager repair: does not backfill role onto foreign bare-id
records (issue #1048 regression guard).
Unblocks: review comments from cursor[bot] (dead else-if branch,
double messaging, redundant isTerminalSession check) and illegalcall
(cross-bucket sort, repair-backfill bypass of predicate tightening) —
all addressed in-place with the multi-orchestrator model preserved.
Verified: core 606/606, cli 450/450, typecheck clean across core/cli/web.
Represent missing activity probes as first-class signal states so lifecycle inference only treats valid idle evidence as proof. This prevents false stuck transitions, keeps API/UI lifecycle truth aligned, and makes root monorepo verification deterministic by serializing recursive build and typecheck.
Introduce an explicit reporting channel so worker agents can self-declare
their workflow phase (started/working/waiting/needs-input/fixing-ci/
addressing-reviews/completed). Fresh reports are trusted over weak inference
but runtime death, activity-based waiting_input, and SCM ground truth still
take precedence.
- Add `applyAgentReport`, validator, canonical mapping, and freshness helper
in `packages/core/src/agent-report.ts`.
- Wire the fallback into `determineStatus` just before the idle-beyond-
threshold promotion, skipping orchestrator and terminal sessions.
- Add `ao acknowledge` and `ao report <state>` CLI commands. Both resolve
the session from `AO_SESSION_ID` when no argument is passed.
- Teach the base agent prompt and orchestrator prompt about the new
reporting commands.
- Ship unit tests covering normalization, mapping, transition validation,
metadata persistence, freshness, and first-start behavior.
Stage 3 of the state-machine redesign (see aa-2/state-machine-redesign-
rollout-plan.md).
- New repo-utils.ts with extractOwnerRepo() and isValidRepoString()
shared across detectEnvironment, autoCreateConfig, addProjectToConfig
- Remote regex now supports GitLab subgroup paths (group/subgroup/repo)
- Repo validation accepts multi-segment paths (owner/repo and deeper)
- Updated prompt text to mention group/subgroup/repo format
- Updated ProjectConfig.repo docstring to be provider-neutral
- Tests import from shared helpers instead of duplicating regex
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clack/prompts text() can return undefined when submitted with just
the placeholder value. Guard with typeof check in promptText and
defensive (entered || "") in both callers to prevent .trim() crash.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- detect-env: test GitHub/GitLab HTTPS/SSH remote extraction, unknown
hosts returning null, missing remote, and non-git directories
- repo-validation: test the anchored regex accepts owner/repo, rejects
empty, lone slash, missing segments, whitespace, and nested paths
- config-validation: test SCM/tracker inference skipped when repo is
missing or has no slash, and inferred correctly with owner/repo
- scm-webhooks: test eventMatchesProject returns false when project
has no repo configured
- orchestrator-prompt: existing test covers repo:undefined → "not configured"
- prompt-builder: existing test covers BASE_AGENT_PROMPT_NO_REPO selection
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Anchor repo validation regex with $ to reject trailing junk like
"acme/repo extra" or "acme/repo#frag" — both prompt locations
- Widen detectEnvironment and addProjectToConfig remote regex to
match gitlab.com in addition to github.com, so GitLab repos get
auto-detected owner/repo instead of silently skipping it
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
afterEach calls vi.restoreAllMocks() which restores the top-level mock
to the real function. Use vi.spyOn on the module namespace instead of
vi.mocked on the destructured import so the mock survives restoration.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test has no ownerRepo detected, which triggers the interactive
repo prompt. Mock isHumanCaller to false so the prompt is skipped
in the non-interactive test environment.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Fix ao start <path> regression: when target path differs from cwd,
run autoCreateConfig on the target (which is a git repo) instead of
cwd (which may not be). Removes the double-create pattern that added
a second project entry.
2. Gate remaining PR/CI sections in orchestrator-prompt: PR Takeover,
PR Review Flow, Bulk Issue Processing, and Monitoring Progress
details are now wrapped in project.repo checks so repo-less projects
don't get contradictory instructions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move ensureGit() after path dedup check so already-registered paths
return early without requiring git
- Use realpathSync for canonical path comparison (resolves symlinks,
case variants, trailing slashes)
- Bail out with error when both SIGTERM and SIGKILL fail to stop AO
instead of unconditionally unregistering a live instance
- Rewrite path-dedup test to exercise the path-arg branch via
AO_CONFIG_PATH, covering addProjectToConfig's dedup lines
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test was mocking detectEnvironment with isGitRepo: false, which now
correctly triggers the fail-fast error for non-git directories. Updated
to isGitRepo: true since the test is verifying config generation
defaults, not non-git behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. prompt-builder: gate PR/CI instructions on project.repo — use
trimmed BASE_AGENT_PROMPT_NO_REPO when no remote configured
2. orchestrator-prompt: gate Quick Start and Available Commands
sections on project.repo — omit issue/PR commands when absent
3. types.ts: add changeset (minor for ao-core) with migration note
4. config.ts: consistent includes("/") guard for tracker inference
5. start.ts: fail fast with clear error when run in non-git directory
6. start.ts: stricter repo validation regex (requires non-empty
segments on both sides of slash)
7. start.ts: addProjectToConfig now prompts for repo like
autoCreateConfig does, with matching warning message
8. scm-webhooks.ts: pre-filter projects without repo in
findWebhookProjects to skip unnecessary SCM plugin lookups
9. lifecycle-manager.ts: fix GitLab subgroup split — use lastIndexOf
to correctly handle group/subgroup/repo paths
Closes#1154
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Replace SIGINT/SIGTERM handlers with a process `exit` handler to avoid
conflicting with the shutdown handler that flushes lifecycle state and
exits with the correct code (130 for SIGINT).
2. Expand dashboard process pattern to match dev mode (next dev, ao-web)
in addition to production (next-server, start-all.js).
3. Only kill dashboard-matching PIDs from lsof output, leaving unrelated
co-listeners (sidecars, SO_REUSEPORT) untouched.
4. Use killDashboardOnPort for the first port attempt too, preventing
blind kills on the configured port when running.json is stale.
5. Add test for mixed-PID filtering on a single port.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove SIGINT/SIGTERM/exit listeners from process when the dashboard
child exits, preventing listener accumulation if runStartup is called
multiple times in the same process.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add test for orchestrator-prompt with repo: undefined (verifies
"not configured" fallback instead of showing literal "undefined")
- Add c8 ignore for interactive prompt code in autoCreateConfig and
promptText (same pattern as existing promptConfirm/promptSelect)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of silently omitting the repo field when no GitHub remote is
found, interactively ask the user to enter their owner/repo. This way
users on GitLab or other hosts can provide the correct value during
first-run setup rather than having to manually edit the yaml afterward.
If the user skips the prompt, the config is still valid (repo remains
optional) with a clear warning about what features are unavailable.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use ?? instead of || for ownerRepo fallback (semantically correct for
null-to-undefined conversion)
- Extract requireRepo() result into a local variable in tracker-gitlab's
updateIssue and issueUrl to avoid redundant validation calls
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Make `repo` field optional in ProjectConfig and Zod schema so projects
without a detected GitHub remote can still load and run
- Remove placeholder `repo: "owner/repo"` from autoCreateConfig() and
addProjectToConfig() — omit the field entirely when no remote is found
- Always use actual workingDir for `path` instead of unreliable `~/<projectId>`
fallback for non-git directories
- Add null guards for `project.repo` across SCM plugins, tracker plugins,
lifecycle manager, webhooks, and prompt builders to prevent crashes when
repo is not configured
Closes#1154
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Port scan now uses killDashboardOnPort() which checks `ps -p <pid> -o args=`
for next-server/start-all.js before killing, avoiding collateral damage to
unrelated services on nearby ports.
2. SIGINT/SIGTERM handlers now call process.exit() after killing the child,
restoring default exit behavior so Ctrl+C doesn't leave the parent hung
if the child is unresponsive.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds test for stopDashboard finding an orphaned dashboard on a reassigned
port via the port-range scan fallback. Marks signal handler body with
c8 ignore since it only fires on process termination.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs caused `ao stop` to fail when the dashboard port was auto-reassigned:
1. Ctrl+C did not propagate to the dashboard child process because Node.js
doesn't guarantee signal forwarding. Added SIGINT/SIGTERM/exit handlers
in runStartup() to explicitly kill the dashboard child.
2. stopDashboard() only checked the configured port, missing orphaned
dashboards on reassigned ports. Refactored into killOnPort() helper
and added a port-range scan fallback (up to MAX_PORT_SCAN ports).
Closes#645
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The "errors when multiple projects and no arg" test expects the error
path (not the prompt path) in resolveProject. Set mockIsHumanCaller
to false so the test reliably hits the non-interactive error branch.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover the moved isAlreadyRunning() check (non-TTY exit, quit, open,
restart, new orchestrator) and the path-based dedup guard in
addProjectToConfig(). Uses hoisted mocks for isAlreadyRunning,
isHumanCaller, promptSelect, unregister, and waitForExit to ensure
proper test isolation.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the `isAlreadyRunning()` check before any config-mutating operations
so that running `ao start` on an already-running project no longer writes
a phantom duplicate entry to agent-orchestrator.yaml. The "new orchestrator"
choice is deferred via a flag until after config is loaded.
Also add path-based deduplication in `addProjectToConfig()` so that a
project whose resolved path already exists in config is returned as-is
instead of being appended with a numeric suffix.
Closes#1150
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>