From 3a69c9703fa4ab2b845c10ac84a65745727db6e8 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Wed, 14 Oct 2026 12:42:17 +0000 Subject: [PATCH 1/3] Handoff note: issue #241 --- .agent/handoff/241.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .agent/handoff/241.md diff --git a/.agent/handoff/241.md b/.agent/handoff/241.md new file mode 100644 index 0000000..b89d629 --- /dev/null +++ b/.agent/handoff/241.md @@ -0,0 +1,25 @@ +# Handoff note: issue #241 + +- issue: 241 +- started_at: 2026-10-14T12:31:26.475895Z +- reason: token_soft_threshold +- last_work_commit: 11f6b9bf07d96aeea85e3e1c2207a4eaab2d8f29 + +## Summary + +Implemented lifecycle memory for telemetry publication progress and workspace-dirty state in `simple_coding_agent/lifecycle.py` (uncommitted). Added `_observed_publication` / `_observed_workspace_dirty` dicts, `_remember_publication_result` / `_remember_workspace_dirty` / `_observe_and_remember_workspace_dirty` helpers, and hooked `_publish_bounded`, `_publish_terminal`, recovery-retry publish, `DirtyWorkspaceError` prepare paths, `_hold_dirty_workspace`/`_emit_workspace_hold`, and successful cleanup paths. Still pending: status/telemetry recovery `workspace_dirty` fields and telemetry remembered-value serving. + +## Remaining work + +1. Finish `_recovery_snapshot_locked` (status): add live `workspace_dirty` field to all three return dicts (startup-hold, dirty-tree hold, checkpoint-present) without changing live GitHub lookups. +2. Finish `_recovery_snapshot_network_free` (telemetry): serve remembered `publication.comment_confirmed`/`pull_request` plus remembered `workspace_dirty` (None when nothing observed); add `workspace_dirty` field; make no tracker/publisher/workspace method calls (only `working_directory` attr reads + remembered dicts). +3. Patch remaining `DirtyWorkspaceError` prepare paths if missed (release) and retry/release cleanup success paths to observe dirty. +4. Add regression test in `tests/test_telemetry.py`: push + failed publication (comment_posted=False, PR known) leading to recovery hold; assert telemetry shows known PR/comment while exploding tracker fake fails on any telemetry-path call; assert `agentctl status` still does live lookups. +5. Update `docs/agents/telemetry-snapshot.md` null-means-unknown bullet to remembered semantics if needed. +6. Run gate: ruff check, pytest py3.11, pytest py3.13, then `code-review` skill, then commit. + +## Evidence + +- Modified file: `simple_coding_agent/lifecycle.py` (git diff --stat: 1 file, ~133 insertions). +- Relevant code: `_remember_publication_result`, `_observe_and_remember_workspace_dirty`, `_publish_bounded`, `_publish_terminal`, `_retry_retained_attempt`, `_hold_dirty_workspace`, `_emit_workspace_hold`. +- Tests to check: `tests/test_telemetry.py` (esp. `test_telemetry_reports_a_retained_hold_without_lookups`), `tests/test_recovery.py`, `tests/test_lifecycle_control.py`. From 9a006ddf829fb879d5990240084560228e62f636 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Wed, 14 Oct 2026 12:42:25 +0000 Subject: [PATCH 2/3] Seal attempt work for #241 (handoff_requested) Attempt-Id: 2026-10-14T12:31:26.475895Z --- simple_coding_agent/lifecycle.py | 139 +++++++++++++++++++++++++++++-- 1 file changed, 133 insertions(+), 6 deletions(-) diff --git a/simple_coding_agent/lifecycle.py b/simple_coding_agent/lifecycle.py index a1ae77a..5f990a0 100644 --- a/simple_coding_agent/lifecycle.py +++ b/simple_coding_agent/lifecycle.py @@ -1220,6 +1220,16 @@ def __init__( # ``control_observed_at``. Served again while the control lock is # busy; ``None`` until the first successful build. self._telemetry_control: dict | None = None + # Last publication progress and workspace-dirty state the lifecycle + # itself observed, keyed by stable attempt ID. Publication flows + # (live attempts, startup reconciliation, recovery retry) record the + # publisher's verified result here; dirty handlers and cleanup + # outcomes record the workspace state. Telemetry serves these + # remembered values without network or git I/O; they stay absent + # (``None``) until something is observed in this process. ``status`` + # keeps its live lookups and never reads these. + self._observed_publication: dict[str, dict] = {} + self._observed_workspace_dirty: dict[str, bool] = {} @property def active_issue_number(self) -> int | None: @@ -1687,6 +1697,84 @@ def _workspace_is_dirty(self) -> bool: except Exception: return True + def _remember_publication_result( + self, attempt_id: str, published: object, phase: str | None = None + ) -> None: + """Remember the publisher's verified result for telemetry. + + Records ``comment_confirmed`` from ``comment_posted`` and the pull + request number/url when the publisher returns one. Called in the + publication flows (live attempts, startup reconciliation, recovery + retry) where the lifecycle already holds the verified result, so + telemetry can later serve it without network I/O. Never raises. + """ + + if not isinstance(attempt_id, str) or not attempt_id: + return + try: + if hasattr(published, "comment_posted"): + comment_confirmed: bool | None = bool( + getattr(published, "comment_posted") + ) + else: + comment_confirmed = None + raw_pr = getattr(published, "pull_request", None) + pull_request: dict | None = None + if raw_pr is not None: + if isinstance(raw_pr, dict): + number = raw_pr.get("number") + url = raw_pr.get("url") + else: + number = getattr(raw_pr, "number", None) + url = getattr(raw_pr, "url", None) + # A publisher that returns a PR object without a number still + # counts as an observation (known, but unnumbered). + pull_request = {"number": number, "url": url} + resolved_phase = phase + if resolved_phase is None: + try: + checkpoint = self._attempt_state.read() + if ( + checkpoint is not None + and checkpoint.started_at == attempt_id + ): + resolved_phase = checkpoint.phase.value + except Exception: + resolved_phase = None + self._observed_publication[attempt_id] = { + "phase": resolved_phase, + "comment_confirmed": comment_confirmed, + "pull_request": copy.deepcopy(pull_request) + if pull_request is not None + else None, + } + except Exception: + return + + def _remember_workspace_dirty(self, attempt_id: str, dirty: bool) -> None: + """Remember the workspace-dirty state for telemetry (never raises).""" + + if not isinstance(attempt_id, str) or not attempt_id: + return + try: + self._observed_workspace_dirty[attempt_id] = bool(dirty) + except Exception: + return + + def _observe_and_remember_workspace_dirty(self, attempt_id: str) -> None: + """Best-effort dirty check after publication; telemetry remembers it. + + Runs only in publication flows (which already perform git I/O), never + on the telemetry path. An unreadable tree records nothing, leaving + the remembered value ``None`` (not known yet). + """ + + try: + dirty = not self._workspace.is_clean() + except Exception: + return + self._remember_workspace_dirty(attempt_id, dirty) + def _dirty_tree_hold_locked(self) -> bool: """Block the claim while the tree holds unexplained work (no exit). @@ -1895,6 +1983,7 @@ def _retry_retained_attempt( base_branch=profile.base_branch, issue_number=claim.issue.number ) except DirtyWorkspaceError as error: + self._remember_workspace_dirty(attempt_id, True) return False, ( f"recovery retry for attempt {attempt_id} not fulfilled:" f" {_exception_detail(error)} The branch, checkpoint, and" @@ -1951,6 +2040,8 @@ def _retry_retained_attempt( " the hold remains." f" {hold_suffix}" ) + self._remember_publication_result(attempt_id, published) + self._observe_and_remember_workspace_dirty(attempt_id) outcome = published.outcome comment_posted = getattr(published, "comment_posted", True) if not comment_posted: @@ -1987,6 +2078,7 @@ def _retry_retained_attempt( # above), so only genuinely unpublished work retains. retain_branch=bool(retain_branch), ) + self._observe_and_remember_workspace_dirty(attempt_id) except DirtyWorkspaceError as error: # Cleanup refused to leave the attempt branch while the tree is # dirty: the hold remains with everything preserved for @@ -2083,6 +2175,7 @@ def _release_retained_attempt( base_branch=profile.base_branch, issue_number=claim.issue.number ) except DirtyWorkspaceError as error: + self._remember_workspace_dirty(attempt_id, True) return False, ( f"recovery release for attempt {attempt_id} not fulfilled:" f" {_exception_detail(error)} The branch, checkpoint, and" @@ -3176,6 +3269,7 @@ def run_once(self) -> LifecycleResult: # the finally boundary below logs ``attempt_finalization_held`` # and retains the checkpoint, so the next poll holds on the # retained attempt instead of claiming new work. + self._remember_workspace_dirty(checkpoint.started_at, True) self._event_log( "workspace_hold", f"{_exception_detail(error)} The branch, checkpoint, and working tree " @@ -3277,6 +3371,9 @@ def run_once(self) -> LifecycleResult: retain_branch=retain_branch or not comment_posted, ) cleanup_ok = True + self._observe_and_remember_workspace_dirty( + checkpoint.started_at + ) except DirtyWorkspaceError as error: # Cleanup refused to leave the attempt branch while the # tree is dirty: hold intake with everything preserved @@ -3631,9 +3728,18 @@ def _publish_bounded(self, request: PublicationRequest): """Publish, bounding the call by the pending handoff's remaining budget.""" budget = self._handoff_publish_budget() - if budget is None: - return self._publisher.publish(request) - return self._publisher.publish(request, timeout=budget) + try: + if budget is None: + published = self._publisher.publish(request) + else: + published = self._publisher.publish(request, timeout=budget) + except Exception: + raise + # Remember the verified result for telemetry (no extra lookup), plus + # the workspace state observed right after publication. + self._remember_publication_result(request.started_at, published) + self._observe_and_remember_workspace_dirty(request.started_at) + return published def _handoff_completion_cause( self, @@ -4004,6 +4110,11 @@ def _hold_dirty_workspace(self, issue_number: int, error: DirtyWorkspaceError) - inspection. """ + try: + attempt_id = _attempt_id(self._attempt_state.read()) + self._remember_workspace_dirty(attempt_id, True) + except Exception: + pass self._emit_workspace_hold(issue_number, error) def _emit_workspace_hold(self, issue_number: int, error: DirtyWorkspaceError) -> None: @@ -4014,6 +4125,11 @@ def _emit_workspace_hold(self, issue_number: int, error: DirtyWorkspaceError) -> best-effort from the tree. """ + try: + attempt_id = _attempt_id(self._attempt_state.read()) + self._remember_workspace_dirty(attempt_id, True) + except Exception: + pass detail = ( f"{_exception_detail(error)} The branch, checkpoint, and working tree " "are preserved for inspection; issue intake is held until the " @@ -4318,6 +4434,7 @@ def _reconcile_startup(self) -> AttemptOutcome | None: # the finally boundary below logs ``attempt_finalization_held`` # and retains the checkpoint, so the next poll holds on the # retained attempt instead of claiming new work. + self._remember_workspace_dirty(checkpoint.started_at, True) self._event_log( "workspace_hold", f"{_exception_detail(error)} The branch, checkpoint, and working tree " @@ -4411,6 +4528,9 @@ def _reconcile_startup(self) -> AttemptOutcome | None: retain_branch=retain_branch or not comment_posted, ) cleanup_ok = True + self._observe_and_remember_workspace_dirty( + checkpoint.started_at + ) except DirtyWorkspaceError as error: # Cleanup refused to leave the attempt branch while the # tree is dirty: hold intake with everything preserved @@ -4496,9 +4616,16 @@ def _publish_terminal( auto_merge_opt_in=auto_merge_opt_in, requeue_source=requeue_source, ) - if timeout is None: - return self._publisher.publish(request) - return self._publisher.publish(request, timeout=timeout) + try: + if timeout is None: + published = self._publisher.publish(request) + else: + published = self._publisher.publish(request, timeout=timeout) + except Exception: + raise + self._remember_publication_result(started_at, published) + self._observe_and_remember_workspace_dirty(started_at) + return published def _publish_rebase_conflict( self, From 68d671460bda8af3a01b5853cdfa2dac25ed7e4f Mon Sep 17 00:00:00 2001 From: Claude Code Date: Wed, 14 Oct 2026 12:59:04 +0000 Subject: [PATCH 3/3] Serve lifecycle-remembered publication and workspace state in telemetry recovery (#241) While an attempt is publishing or held in recovery, telemetry's control.recovery carries the publication progress (comment confirmed, pull request) and workspace-dirty state the lifecycle itself last observed, instead of 'not known yet'. The telemetry path still makes no tracker, publisher, or workspace calls: the values are remembered during publication, recovery, and startup reconciliation, and stay null until something is observed in this process. agentctl status keeps its live GitHub lookups and gains the same workspace_dirty field computed live, so the two sections cannot drift. Co-Authored-By: Claude Code --- docs/agents/telemetry-snapshot.md | 6 +- simple_coding_agent/lifecycle.py | 76 +++++++++++-------- tests/test_telemetry.py | 121 ++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 32 deletions(-) diff --git a/docs/agents/telemetry-snapshot.md b/docs/agents/telemetry-snapshot.md index 978a22e..93e0249 100644 --- a/docs/agents/telemetry-snapshot.md +++ b/docs/agents/telemetry-snapshot.md @@ -93,7 +93,11 @@ fields (ids, timestamps, enums, booleans) are bare values, not figures. `last_finished: null` is no finished attempt known. Likewise a null `pull_request` inside `last_finished` means no PR, and null `comment_confirmed`/`pull_request` inside `control.recovery.publication` - mean "not known yet" (the snapshot performs no network I/O). + or null `workspace_dirty` inside `control.recovery` mean "not observed + yet in this process": while an attempt is publishing or held in recovery, + telemetry carries the publication progress and workspace-dirty state the + lifecycle itself last observed (no network or git I/O on the telemetry + path), and `agentctl status` keeps its live lookups for the same fields. ## Versioning rules diff --git a/simple_coding_agent/lifecycle.py b/simple_coding_agent/lifecycle.py index 5f990a0..be562da 100644 --- a/simple_coding_agent/lifecycle.py +++ b/simple_coding_agent/lifecycle.py @@ -1712,11 +1712,9 @@ def _remember_publication_result( if not isinstance(attempt_id, str) or not attempt_id: return try: - if hasattr(published, "comment_posted"): - comment_confirmed: bool | None = bool( - getattr(published, "comment_posted") - ) - else: + try: + comment_confirmed: bool | None = bool(published.comment_posted) + except AttributeError: comment_confirmed = None raw_pr = getattr(published, "pull_request", None) pull_request: dict | None = None @@ -2550,7 +2548,8 @@ def _recovery_snapshot_locked( # the startup hold (read-only; never logs from status) with # the dirty paths when the tree is dirty too. reason = self._startup_hold_detail - if self._workspace_is_dirty(): + workspace_dirty = self._workspace_is_dirty() + if workspace_dirty: suffix = self._dirty_paths_suffix() reason += " The working tree is also dirty" + ( suffix if suffix else "." @@ -2562,6 +2561,7 @@ def _recovery_snapshot_locked( "outcome": None, "branch": None, "workspace": str(self._workspace.working_directory), + "workspace_dirty": workspace_dirty, "checkpoint": False, "publication": { "phase": None, @@ -2585,6 +2585,7 @@ def _recovery_snapshot_locked( "outcome": None, "branch": None, "workspace": str(self._workspace.working_directory), + "workspace_dirty": True, "checkpoint": False, "publication": { "phase": None, @@ -2630,6 +2631,7 @@ def _recovery_snapshot_locked( "outcome": outcome, "branch": checkpoint.branch, "workspace": str(self._workspace.working_directory), + "workspace_dirty": self._workspace_is_dirty(), "checkpoint": True, "publication": publication, "hold_reason": describe_hold( @@ -2916,9 +2918,10 @@ def telemetry_control_snapshot(self) -> dict: The same payload :meth:`control_status` builds, but without workspace or GitHub I/O: no dirty-tree check runs, no tracker or publisher call - is made, and publication fields that would need a lookup stay ``None`` - ("not known yet"). The section carries ``control_observed_at`` saying - when it was built. + is made, and the publication progress and workspace-dirty state are + served from what the lifecycle itself last observed (``None``/"not + known yet" until something is observed in this process). The section + carries ``control_observed_at`` saying when it was built. Waits at most ``CONTROL_LOCK_TIMEOUT_SECONDS`` for the control lock so telemetry never holds up other control commands. While the lock is @@ -3049,9 +3052,11 @@ def _recovery_snapshot_network_free( Mirrors :meth:`_recovery_snapshot_locked`: the workspace dirty check is skipped (a dirty tree alone never produces a section here) and the - publication progress stays ``None`` ("not known yet") because the - GitHub lookups never run. Never touches the tracker, publisher, or - workspace methods. Caller holds the control lock. + publication progress and workspace-dirty state are served from what + the lifecycle itself last observed (``None``/"not known yet" until + something is observed in this process) because the GitHub lookups + never run. Never touches the tracker, publisher, or workspace + methods. Caller holds the control lock. """ from simple_coding_agent.recovery import ( @@ -3070,6 +3075,7 @@ def _recovery_snapshot_network_free( "outcome": None, "branch": None, "workspace": str(self._workspace.working_directory), + "workspace_dirty": None, "checkpoint": False, "publication": { "phase": None, @@ -3096,11 +3102,24 @@ def _recovery_snapshot_network_free( outcome = terminal.outcome.value except Exception: outcome = None - publication = { - "phase": checkpoint.phase.value, - "comment_confirmed": None, - "pull_request": None, - } + remembered = self._observed_publication.get(checkpoint.started_at) + if remembered is not None: + publication = { + "phase": remembered.get("phase") + if remembered.get("phase") is not None + else checkpoint.phase.value, + "comment_confirmed": remembered.get("comment_confirmed"), + "pull_request": copy.deepcopy(remembered.get("pull_request")) + if remembered.get("pull_request") is not None + else None, + } + else: + publication = { + "phase": checkpoint.phase.value, + "comment_confirmed": None, + "pull_request": None, + } + workspace_dirty = self._observed_workspace_dirty.get(checkpoint.started_at) if self._recovering: next_action = ( "Wait for startup reconciliation to finish; do not run recovery" @@ -3120,6 +3139,7 @@ def _recovery_snapshot_network_free( "outcome": outcome, "branch": checkpoint.branch, "workspace": str(self._workspace.working_directory), + "workspace_dirty": workspace_dirty, "checkpoint": True, "publication": publication, "hold_reason": describe_hold( @@ -3728,13 +3748,10 @@ def _publish_bounded(self, request: PublicationRequest): """Publish, bounding the call by the pending handoff's remaining budget.""" budget = self._handoff_publish_budget() - try: - if budget is None: - published = self._publisher.publish(request) - else: - published = self._publisher.publish(request, timeout=budget) - except Exception: - raise + if budget is None: + published = self._publisher.publish(request) + else: + published = self._publisher.publish(request, timeout=budget) # Remember the verified result for telemetry (no extra lookup), plus # the workspace state observed right after publication. self._remember_publication_result(request.started_at, published) @@ -4616,13 +4633,10 @@ def _publish_terminal( auto_merge_opt_in=auto_merge_opt_in, requeue_source=requeue_source, ) - try: - if timeout is None: - published = self._publisher.publish(request) - else: - published = self._publisher.publish(request, timeout=timeout) - except Exception: - raise + if timeout is None: + published = self._publisher.publish(request) + else: + published = self._publisher.publish(request, timeout=timeout) self._remember_publication_result(started_at, published) self._observe_and_remember_workspace_dirty(started_at) return published diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index ebaefb3..dabae50 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -598,6 +598,127 @@ def black_hole(connection: socket.socket) -> None: assert elapsed < 8 +def test_telemetry_serves_remembered_publication_while_status_stays_live( + tmp_path: Path, +) -> None: + """Telemetry shows what publication itself observed; status stays live. + + After a push and a failed (unconfirmed) publication the attempt is held + for recovery with its checkpoint retained. Telemetry must report the + pull request and comment state the lifecycle itself observed — without + any tracker, publisher, or workspace call — while ``agentctl status`` + keeps its live GitHub lookups and live dirty-tree check. + """ + + from types import SimpleNamespace + + from simple_coding_agent.completion import AttemptOutcome + from simple_coding_agent.publication import PullRequest + + lifecycle, _, _ = make_lifecycle(tmp_path) + state = AttemptStateStore(tmp_path) + checkpoint = state.start(issue_number=24, branch="agent/issue-24") + assert checkpoint is not None + attempt_id = checkpoint.started_at + + workspace = lifecycle._workspace # noqa: SLF001 + workspace.make_dirty("work.txt", "uncommitted work") + + live_calls: list[str] = [] + + class LiveTransport: + """Live GitHub answers that deliberately differ from memory.""" + + def find_attempt_comment( + self, repository: str, issue_number: int, marker: str + ) -> bool: + live_calls.append("find_attempt_comment") + assert attempt_id in marker + return True + + def find_pull_request(self, repository: str, head: str): + live_calls.append("find_pull_request") + return PullRequest(number=9, url="https://example.test/9") + + remembered_pr = PullRequest(number=7, url="https://example.test/7") + + class PushThenUnconfirmedPublisher: + """The push succeeded (known PR) but the result comment is unconfirmed.""" + + _repository = "owner/repo" + + def __init__(self) -> None: + self._github = LiveTransport() + + def publish( + self, request: object, *, timeout: float | None = None + ): + return SimpleNamespace( + outcome=AttemptOutcome.COMPLETE, + branch_url="https://example.test/tree/agent/issue-24", + pull_request=remembered_pr, + comment_posted=False, + ) + + lifecycle._publisher = PushThenUnconfirmedPublisher() # noqa: SLF001 + claim = SimpleNamespace( + issue=SimpleNamespace(number=24, title="Implement recovery") + ) + # Drive the real publication seam so the remember hook is exercised, not + # the remember helper directly. + lifecycle._publish_terminal( # noqa: SLF001 + claim, attempt_id, None, None, AttemptOutcome.COMPLETE + ) + # The tree is clean again by the time anyone looks: the live status check + # must see clean while telemetry still reports what publication itself + # observed (dirty). + workspace.dirty.clear() + + # From here the telemetry path must make no calls: the tracker explodes + # on any method, the workspace refuses any inspection, and the live + # transport counts every GitHub lookup. + lifecycle._tracker = ExplodingTracker() # noqa: SLF001 + original_is_clean = workspace.is_clean + original_dirty_paths = workspace.dirty_paths + workspace.is_clean = lambda: (_ for _ in ()).throw( # noqa: SLF001 + AssertionError("is_clean must not run on the telemetry path") + ) + workspace.dirty_paths = lambda: (_ for _ in ()).throw( # noqa: SLF001 + AssertionError("dirty_paths must not run on the telemetry path") + ) + try: + snapshot = lifecycle.telemetry_snapshot() + finally: + workspace.is_clean = original_is_clean # noqa: SLF001 + workspace.dirty_paths = original_dirty_paths # noqa: SLF001 + + recovery = snapshot["control"]["recovery"] + assert recovery is not None + assert recovery["attempt_id"] == attempt_id + assert recovery["publication"] == { + "phase": checkpoint.phase.value, + "comment_confirmed": False, + "pull_request": {"number": 7, "url": "https://example.test/7"}, + } + assert recovery["workspace_dirty"] is True + assert live_calls == [] + + # ``agentctl status`` keeps its live lookups: it reports what GitHub and + # the tree say now, not what the lifecycle remembered. + status = lifecycle.control_status("owner/repo") + + live = status["recovery"] + assert live is not None + assert live["publication"]["comment_confirmed"] is True + assert live["publication"]["pull_request"] == { + "number": 9, + "url": "https://example.test/9", + } + assert live["workspace_dirty"] is False + assert "find_attempt_comment" in live_calls + assert "find_pull_request" in live_calls + + def test_telemetry_uses_a_short_timeout_while_other_commands_keep_theirs() -> None: import simple_coding_agent.telemetry as telemetry_module