agent: Report last known publication progress and workspace state in telemetry

.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`.

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
 

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,82 @@ 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:
+            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
+            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 +1981,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 +2038,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 +2076,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 +2173,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"
@@ -2457,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 "."
@@ -2469,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,
@@ -2492,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,
@@ -2537,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(
@@ -2823,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
@@ -2956,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 (
@@ -2977,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,
@@ -3003,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"
@@ -3027,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(
@@ -3176,6 +3289,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 +3391,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
@@ -3632,8 +3749,14 @@ def _publish_bounded(self, request: PublicationRequest):
 
         budget = self._handoff_publish_budget()
         if budget is None:
-            return self._publisher.publish(request)
-        return self._publisher.publish(request, timeout=budget)
+            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)
+        self._observe_and_remember_workspace_dirty(request.started_at)
+        return published
 
     def _handoff_completion_cause(
         self,
@@ -4004,6 +4127,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 +4142,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 +4451,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 +4545,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
@@ -4497,8 +4634,12 @@ def _publish_terminal(
             requeue_source=requeue_source,
         )
         if timeout is None:
-            return self._publisher.publish(request)
-        return self._publisher.publish(request, timeout=timeout)
+            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
 
     def _publish_rebase_conflict(
         self,