Orchestrata Ultimate › Plans › codemod-observability-wiring-design

SPEC-110 §15 observability — wiring design

Source: plans/codemod-observability-wiring-design.md at 8a2cb42ad · last changed 2026-10-06 · first committed 2026-08-07 · 233 lines.

Prose pending.

Plan text

SPEC-110 §15 observability — wiring design

Status: BUILT (S1, b0fd759a0). Satisfies RUL-022 (design before build) for the follow-on implementation brief.

1. The defect

src/orchestrata_ultimate/refactor/codemod/observability/ ships five observers — TracingObserver, MetricsObserver, DebugPanelObserver, ErrorDetector, GraphWriterObserver — and an eight-member CodemodEventType enum. Measured:

$ grep -rn "CodemodEvent(" src/ scripts/ --include=*.py | grep -v observability/
CONFIRMED: zero CodemodEvent(...) constructions outside observability/

The three non-observability files that mention the name (codemod/__init__.py, codemod/report.py, scripts/split_skeleton.py) are re-exports and string literals in a splitter script. Nothing ever constructs an event.

The root cause is not a missing producer. It is a missing subject: there is no registry, no subscribe, no emit. Five observers implement on_event and there is nowhere to attach them. This is the RUL-023 class exactly — built, not activated.

2. The load-bearing finding: no new mechanism is needed

core/patterns/observer.py already defines:

@runtime_checkable
class Observer(Protocol[T]):
    def on_event(self, event: T) -> None: ...

class SyncObservable(Observable[T]):
    def subscribe(self, observer: Observer[T]) -> Callable[[], None]
    def unsubscribe(self, observer: Observer[T]) -> None
    def emit(self, event: T) -> None

The method name is on_event in both places. The five codemod observers therefore already satisfy core.patterns.observer.Observer[CodemodEvent] structurally, today, with no edit and no adapter.

Two consequences:

1. The subject is SyncObservable[CodemodEvent]. Writing a second bus inside observability/ would duplicate a tested core pattern.
2. Observer in observability/events.py is a redundant redeclaration of core.patterns.observer.Observer[CodemodEvent]. It is dead surface that invites the two protocols to drift apart. Delete it; re-export the core one under the same name so existing imports keep working.

SyncObservable.emit also already carries the policy this needs:

a raising observer's exception is logged via logger.exception and swallowed — it is never re-raised, so one misbehaving observer can never block delivery

That is the correct semantics for observability: instrumentation must never be able to fail a codemod. No new error handling to design.

3. Emission points

One CodemodEventType member per real lifecycle boundary. Each pair brackets an operation so MetricsObserver can time it as the difference of the two timestamps.

ERROR is emitted in addition to raising, never instead of. Observability does not change control flow.

4. Correlation

CodemodEvent.correlation_id already exists and is required by the constructor. One id per top-level operation (one daemon request, or one CLI invocation), generated at the entry point and threaded down. Without this the paired STARTED/COMPLETED events cannot be joined under concurrency, and the daemon is explicitly concurrent (tests/refactor/codemod/test_concurrency.py).

5. Ownership and attachment

The subject belongs to the engine, not to a global. CodemodWorkspace (or the daemon that owns it) holds one SyncObservable[CodemodEvent]; the JSON-RPC server and the CLI each subscribe whichever observers their deployment wants. Default: none subscribed — zero cost when nobody is watching, and no implicit Prometheus/graph writes from a library import.

6. Verification

Real observers, real events, no doubles (RUL-002):

1. A recording observer (a small real class implementing on_event, appending to a list — a legitimate test collaborator, not a mock of anything) subscribed to a real workspace load asserts WORKSPACE_LOADED arrives with a truthful module count.
2. Each STARTED/COMPLETED pair asserted to arrive in order and share a correlation id under a concurrent two-request daemon exercise.
3. An observer that raises is subscribed alongside a recording one; the codemod still completes and the recording observer still receives every event — proving the isolation policy end to end rather than trusting the docstring.
4. ERROR asserted to accompany, not replace, a raised exception.

7. Out of scope

Wiring MetricsObserver to the live Prometheus at 10.92.53.40 is a separate step and a separate decision — it needs a registry owner and a scrape target, and this design deliberately leaves observers unsubscribed by default.

8. Observability wiring (as built)

CodemodWorkspace owns one SyncObservable[CodemodEvent] (ws.observable, ws.subscribe(observer)). CodemodEventType carries the 13 kinds REQ-110-025 names: WORKSPACE_LOADED, INDEX_BUILT, PASS_STARTED, PASS_COMPLETED, FILE_MUTATED, REVIEW_STARTED, REVIEW_COMPLETED, FINDING_EMITTED, COMMIT_STARTED, COMMIT_COMPLETED, COMMIT_ABORTED, ROLLBACK_TRIGGERED, ERROR_DETECTED (ERROR stays as an alias of ERROR_DETECTED).

load(), run() and commit() accept observers: Sequence[Observer[CodemodEvent]] | None. The default set is empty, so an unwatched workspace emits to nobody. A passed sequence is subscribed for the duration of that call only (on a private bus, so a concurrent operation on the same workspace never delivers into it). Every operation (one load(), run() or commit() call) generates one correlation_id and stamps it on every event it emits; two concurrent operations carry distinct ids. ERROR_DETECTED is emitted in addition to the exception, never instead of it; a failed write inside commit() emits ROLLBACK_TRIGGERED (from txn.commit_changes), ERROR_DETECTED, then COMMIT_ABORTED, then re-raises. commit() now writes through txn.commit_changes, so the atomic-write, concurrent-edit and rollback guarantees of REQ-110-014..016 apply to it.

Multi-pass write-back (round-1 fix). run() feeds pass N the forest that pass N-1 produced, so each pass's ChangeEntry.original_tree is the previous pass's OUTPUT, while the transaction's concurrent-edit guard compares the disk bytes against original_tree. ChangeSet.merge therefore keeps the EARLIEST original_tree per path (the load-time tree whose bytes are on disk) and the LATEST entry's mutated_tree/metadata; two passes editing one file now commit.

Observer semantics (REQ-110-027). The per-call observers= list overrides the DEFAULT set, which is empty (§5/§7). Workspace-level subscribe() registrations are explicit and additive, not defaults. An observer object reachable through both the workspace bus and the per-call list receives each event exactly once: the private per-operation bus omits observers already on the workspace bus (identity comparison).

Known limitation: equality vs identity dedupe (S1 review round 2, M4). The identity dedupe lives in _Operation.__init__ only. The core _SubscriptionRegistry.subscribe (core/patterns/observer.py) dedupes by equality (not in), so an observer that defines __eq__ (for example a dataclass) and compares equal to one already subscribed is silently dropped at subscribe() time, and two distinct but equal observers behave as one. All five built-in observers use identity equality and are unaffected. The core module is out of S1 scope and was not changed; the fix (an identity check in the registry) is owed to the owner of core/patterns/observer.py.

Production constructor (D10, --observe). default_observers() (public, formerly _default_observers) builds the five REQ-110-026 observers. scripts/codemod.py run --observe passes that set as observers= to run() and commit() for one invocation, so the built-ins have a production caller and the REQ-110-027 override has a referent. The default stays empty without the flag. The debug panel writes NDJSON traces under pipeline_output/traces/ relative to the working directory.

File mode (round-2 fix B1). txn._write_atomic creates the temp file with mkstemp (mode 0600) and renames it over the target. It stats the target first and fchmods the temp file to the target's permission bits before the rename, so a 0755 script stays 0755 and a 0644 file stays 0644. Round 3: the fchmod runs AFTER os.write (the kernel clears setuid/setgid on write); measured as uid 1001, 0o4755 and 0o2755 both round-trip. Round 4 (M1): the rename swaps in an inode owned by the writer, so the uid is not preserved (impossible without root; the file ends up owned by the committer). The group is preserved with fchown(fd, -1, st_gid) before fchmod; round 4 swallowed a PermissionError only when the writer was not a member of that group (measured as uid 1001: a member group is kept; a foreign group 2000 gives EPERM and the file keeps the writer's group 1007) — that membership rule was replaced in round 5 by the errno rule below. commit() does not create a file (_check_concurrent_edits refuses a missing target); a target deleted between that check and the write is refused with ConcurrentEditError by _write_atomic (there is no "new file gets 0600" case). The stat to rename window inside _write_atomic stays open without renameat2: a target deleted in that window is recreated by the rename. _rollback (round 4, R4-B1) restores only files_written, since every other entry is unchanged on disk, through the same tempfile + rename path with the snapshot's recorded mode and group (R3-B1: a 0444 target is restored byte-exact and stays 0444, the original exception is re-raised). Each per-file restore is wrapped: a failure is logged, attached to the original exception with add_note and listed in the ROLLBACK_TRIGGERED payload (rollback_failures, elements are "path: ExcType: message" strings); it never masks the original and the event is always emitted. Round 5 (R5-B1): fchown of the group treats OSError with errno EPERM or EINVAL as "group cannot be preserved" (warning with path, wanted gid and resulting gid, commit continues); the os.getgroups() membership test is gone because it is wrong in user namespaces, where every unmapped host group reads as 65534 and fchown to it is EINVAL. A raising on_event sink is logged and swallowed so the original exception still propagates (round-5 note). Round 5 (M-c): rollback restores the pre-commit bytes of every WRITTEN file even if it was deleted or edited after its write: a deleted written file is recreated and a later edit is overwritten, because the restore targets the state before the commit. A failed write unlinks its temp file (N-e); _replace_file loops on os.write so a short write surfaces its error instead of renaming a truncated file. The ConcurrentEditError message states what was compared: the bytes on disk against the tree the change was computed against (round-2 N2; this also covers run(p1), a dry-run commit, run(p2), then a real commit, where p2's original_tree is p1's in-memory output and the refusal is correct but not caused by an outside edit).

Deviation from §3 (review events). §3 places REVIEW_STARTED/ REVIEW_COMPLETED per review pass. As built, commit() emits ONE aggregate REVIEW_STARTED (payload review_count) and ONE REVIEW_COMPLETED (finding, error and warning totals) around the whole review battery, with one FINDING_EMITTED per finding. Reason: the aggregate pair is what the commit gate consumes (the gate refuses on the combined error count), and a per-pass pair would need a stable per-review identity that ReviewPass (a Protocol with only review()) does not require. The code was not changed to per-pass; per-pass pairs remain available as an additive follow-up if a consumer needs per-review timing.

Severity mismatch (ERR-110-011). The spec names WARNING for an observer that raises during dispatch; core.patterns.observer.SyncObservable.emit logs with logger.exception (ERROR severity). core/patterns/observer.py is out of S1 scope and was not changed; the log-and-continue behaviour (the substance of ERR-110-011) is proven, the severity differs. Recorded for the owner of that module.

Proof: tests/refactor/codemod/test_observability_wiring.py, tests/refactor/codemod/test_transactional_writeback.py (file mode, concurrent-edit message), tests/scripts/test_codemod_cli.py (--observe).

Related

• Plans
• Orchestrata Ultimate — project index
• Workflow
• Tasks