Address Copilot review:
- ResolveOpenPRDecision now sets a stable, timestamp-free Evidence summary
"<condition> #<num> <url>" for every ladder outcome, consuming the
previously-unused OpenPRInput.Number/URL identity inputs and making PR
decisions traceable in logs. Covered by TestResolveOpenPRDecisionEvidence.
- Document LifecycleDecision's zero-value contract: an empty PRState/PRReason
means "this decider does not address PR — leave unchanged", not PRNone. The
LCM must map empty PR fields to a nil LifecyclePatch.PR; writing PRNone on a
probe tick would clobber a live PR. (Pointers were considered but the empty
sentinel is distinguishable from every valid state and consistent with the
codebase's value-enum style; LifecyclePatch already owns nil-means-unchanged.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Clarify the timestamp-stripping block flagged in review: spell out what
each of the three regexes matches (full ISO/RFC3339 datetime, bare
time-of-day, bare unix epoch) and why order matters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address PR review (aa-1):
- Move the decider input/output type definitions (LifecycleDecision,
ProbeInput, ProcessLiveness, OpenPRInput, DetectingInput) out of the
execution file into a dedicated types.go, leaving decide.go for behaviour.
- Expand the detecting() helper doc to spell out that it adapts a probe
verdict into the shared CreateDetectingDecision anti-flap path so each
probe branch doesn't re-implement the quarantine counter.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address PR review (aa-1):
- ResolveOpenPRDecision now keys MERGEABLE on Mergeable alone (checked
before Approved). Mergeability is the authoritative merge gate, so a PR
on a no-required-review repo no longer falls through to PR_OPEN. The
approved+mergeable and approved-only cases are unchanged.
- Broaden the derive-consistency test to cover the probe and terminal
deciders too, not just the open-PR ladder.
- Document the HashEvidence epoch-stripping regex's breadth.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the stubbed deciders in domain/decide with real, total,
side-effect-free implementations:
- ResolveProbeDecision: kill-intent short-circuits to terminal; failed
probes and probe disagreement route to detecting; only runtime-dead +
process-dead + no-recent-activity concludes killed.
- ResolveOpenPRDecision: the PR pipeline ladder
(ci_failing > changes_requested > approved+mergeable > approved >
review_pending > idle-beyond > open).
- ResolveTerminalPRStateDecision: merged -> idle/merged_waiting_decision,
closed -> idle.
- CreateDetectingDecision: anti-flap quarantine — unchanged-evidence
counter with StartedAt preserved across the episode so the duration cap
is a real wall-clock safety net; escalates to stuck at 3 ticks or 5m.
- HashEvidence: strips timestamps/epochs and collapses whitespace before
hashing so restamped-but-unchanged signals compare equal.
Table tests cover every branch (100% statement coverage), including a
consistency check that the open-PR ladder's display Status matches
DeriveLegacyStatus over the canonical state it emits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review feedback on PR #2:
- Add CanonicalSessionLifecycle.Revision (monotonic write counter) distinct
from the schema Version; LifecyclePatch.ExpectedVersion -> ExpectedRevision
now compares it, so optimistic locking actually works.
- LifecycleStore.List returns []domain.SessionRecord (persistence shape, no
derived status); add SessionRecord and make Session embed it. Keeps the
Session Manager the single producer of the derived display status.
- SpawnOutcome.RuntimeHandle is now the structured ports.RuntimeHandle, not a
string, so Destroy/SendMessage get the handle without ad-hoc encoding.
- Agent.IsProcessRunning -> ProbeProcess returning ProcessProbe (liveness),
not domain.ActivityState; the name no longer implies a boolean.
- Document LifecyclePatch Detecting vs ClearDetecting precedence (clear wins).
- Correct the DeriveLegacyStatus doc: hard session states outrank PR facts;
"PR dominates" applies only to the soft idle/working states. Implementation
was already correct (matches canonical AO); only the comment overstated it.
- Replace personal-name attributions in package/interface comments with
role-based terms (SCM poller / persistence adapter / API layer).
go build / go vet / go test all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contract-first boundary for the Lifecycle Manager + Session Manager lane.
Pure shapes only — types, interfaces, and the display-status derivation —
so adil (SCM poller), Tom (persistence), and aditi (API) can review and
build against a stable boundary before any behaviour lands.
domain/
- CanonicalSessionLifecycle: the only persisted state (session/pr/runtime
sub-states), with Activity + Detecting sub-states added as decider inputs
that must survive between observations.
- DeriveLegacyStatus: the sole producer of the derived display status
(never persisted), with 11 table tests.
ports/
- inbound: LifecycleManager (Apply* pipeline, per-session serialised) and
SessionManager.
- outbound: LifecycleStore (Tom), Notifier, AgentMessenger, and the
Runtime/Agent/Workspace plugin ports (co-owned with the agents lane).
- facts: SCMFacts / RuntimeFacts / ActivitySignal DTOs.
decide/ pure-core signatures + I/O types; bodies stubbed for the next PR.
Folds in four design-review fixes (documented in-code, pending team confirm):
1. Activity + Detecting persisted so the pure decider has memory across calls.
2. Per-session serialisation documented; LifecyclePatch.ExpectedVersion
offers optimistic-locking as an alternative.
3. LifecyclePatch is a sparse pointer-field merge-patch (+ ClearDetecting).
4. SCMFacts gains Fetched (failed fetch != "PR closed") and per-comment
IsBot (bot vs human route to different reactions).
go build / go vet / go test all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Initial buildable skeleton for the agent-orchestrator rewrite, splitting
the repo into a Go backend daemon and an Electron + TypeScript frontend.
- backend/: go.mod (Go 1.22) + main.go that compiles and prints a startup line
- frontend/: package.json, strict tsconfig.json, Electron main-process stub
- .gitignore for Node/Electron/Go/OS/editor/env artifacts
- README note describing the new two-folder structure
No app logic or architecture layering yet (routes/controllers/services/etc.
come in a later task). go build and tsc --noEmit both pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
harshitsinghbhandari