Skip to content

feat(senpi): OmO-native (senpi) attach/observe adapter - #4

Merged
darko-mijic merged 103 commits into
mainfrom
senpi-adapter
Aug 28, 2026
Merged

feat(senpi): OmO-native (senpi) attach/observe adapter#4
darko-mijic merged 103 commits into
mainfrom
senpi-adapter

Conversation

@darko-mijic

@darko-mijic darko-mijic commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

What this is

An attach-only observe adapter for OmO native (senpi) coding sessions, plus everything the first public release of this package needed to be safe to consume: the 0.3.0 release contract, shared processing mechanics, and a security hardening pass from the 2026-08-28 review. 77 commits over main, at ca3e4de.

This PR pairs with Cockpit PR #9. Cockpit consumes this branch through a git pin and its OmO adapter is built directly on the exports here, so the two diffs should be read together.

The senpi adapter

The adapter attaches to existing sessions; it never spawns, resumes, or drives an agent. Surfaces:

  • ./senpi and ./senpi/processing as exact barrel subpaths: session v3 Zod contracts, agent-home resolution, discovery with header-cwd verification, tree-linear projection with compaction handling (retainedTail plus legacy firstKeptEntryId), a bounded JSONL cursor with revisioned checkpoints, tail with splice mutations, watch with stable-cursor quiescence, and native block reduce/fold.
  • A hooks library: wire schemas (camelCase primaries with snake_case aliases), a validator with pinned diagnostic codes, a runner with vendored exit semantics, an output builder, read-only trust inspection with hash parity, a consent-gated trust writer, and an observe-only forwarder asset with a registration helper.
  • Vendored upstream contract artifacts under docs/upstream/senpi/ with two-directional drift tests in CI, plus the adapter reference doc and the live-store smoke probe (pnpm senpi:smoke).

Release preparation

  • Version 0.3.0. Full export map audited, changelog written, and the packed tarball installed into clean Node 22 and Node 24 projects that import every documented subpath. The same matrix runs in CI. Publication was prepared and then skipped by owner decision; the git pin is the transport for now.
  • Typed checkpoint conflicts. StaleCheckpointConflict carries structured revision data instead of callers matching on error prose. A contract test pins its shape (name, constructor fields, isStaleCheckpointConflict) as a cross-repo contract, because Cockpit matches on it.
  • Incremental tailing with shared mechanics. Byte cursors, checkpoint revisions, unchanged-input detection, watch scheduling, and discovery primitives moved to an unbarreled src/internal/ after fixtures locked both adapters' semantic output. Tails now read from the checkpoint rather than byte zero, and unchanged polls return without reprojection. The senpi and grok semantic parsers stay separate.
  • Hook, trust, and forwarder contracts frozen. Every mutating primitive is explicit and opt-in, documented with the caller's consent duties.
  • Non-mutating compatibility probe. A fixture-first probe applies a future-harness checklist to discovery, identity, tail, reset/fork, and provenance availability with manual checkpoint mode forced. It records findings only; it writes nothing and is not an integration approval.

Security hardening from the review

  • Marker-root containment resolves symlinks before deciding a marker directory is inside the allowed roots, and re-verifies at the write choke point, so a symlinked directory can no longer escape.
  • Stale marker-lock reclamation verifies device/inode identity and a nonce before removing a lock, so a lock recreated between stat and rm survives. Exactly one concurrent reclaimer wins.
  • Senpi registration documents validate fail-closed: unknown hook event names, empty handler lists, and arbitrary objects are rejected instead of silently accepted.
  • The forwarder defaults to loopback binding, refuses redirects, and caps stdin.
  • The three wall-clock tests now use injected clocks and fake timers.

The Greptile review ran on this branch with no untriaged security or P0/P1 findings.

Pairing with Cockpit

Cockpit PR #9 pins github:libar-dev/agent-harness-kit#ca3e4de and consumes ./senpi, ./senpi/processing, and ./grok/processing through dynamic import() only (the kit is ESM-only; Cockpit's main bundle is CJS). Where Cockpit needs to match a thrown conflict, it does so through the pinned StaleCheckpointConflict name contract rather than importing the class, so neither repo's internals leak into the other. The prepack order tolerates archive contexts (a codeload tarball has no .git), which is what makes the pin installable from a clean checkout.

Verification

  • At the pushed tip: pnpm run build && pnpm run check && pnpm run test:run all exit 0, and Cockpit's clean install against the pin reproducibly builds (pnpm install && pnpm typecheck && pnpm build from wiped node_modules on the Cockpit side).
  • The joint final verification wave (compliance, code quality, real manual QA, scope fidelity) APPROVED in the closing review session; its full-suite run caught regressions that per-lane targeted runs had missed, and the fixes are on this branch.
  • The adapter's original closeout evidence still holds: drift tests both directions against the vendored artifacts (including a mutation-detection case), a golden tail proof matched byte-for-byte against an independent walker on a real 2,189-entry session, trust hash goldens reproduced against engine code, and a live-store probe listed 50 sessions and tailed a live-appended session with correct revision and mutation semantics.

Scope guards

No runtime dependency on senpi/OmO packages, no imports from grok or Claude internals, no auth.json reads, no new runtime npm dependencies, cursor and marker internals unexported, root barrel untouched.

Merge gate

Do not merge until Cockpit PR #9 has landed and been tested against this pin. Intended order: Cockpit #8, Cockpit #9, then this PR. After merge, publication of 0.3.0 to npm remains a separate owner decision.

Greptile Summary

The change expands the Senpi integration with incremental processing, hook trust management, shared lease locking, package-consumer checks, and immutable CI action references. However, removing the final trust entry can delete trust state that a replacement writer committed after reclaiming an expired lease.

Confidence Score: 4/5

Not merge-safe until trust-state deletion verifies that its lease is still held at the deletion commit point.

One accepted P1 non-security finding remains: an expired removing writer can delete a replacement writer's newly committed trust state. The score for one non-security P1 finding is 4.

Files Needing Attention: src/senpi/trust-writer.ts

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for the posted P1 finding.
  • The Delayed-removal fixture source log was captured to verify the fixture setup referenced in the P1 finding.
  • The Replacement-writer helper log was captured to verify the helper logic used in the P1 proof.
  • The Executed stale-lease replacement output log was captured to verify the execution path described in the P1 proof.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (5): Last reviewed commit: "fix(senpi): renew trust lease at the des..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@darko-mijic

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

Comment thread src/internal/marker-lock.ts Outdated
Comment thread src/senpi/trust-writer.ts Outdated
@darko-mijic

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

Comment thread src/internal/marker-lock.ts Outdated
Comment thread src/senpi/trust-writer.ts Outdated
@darko-mijic

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

Comment thread src/senpi/trust-writer.ts
Comment on lines +348 to +350
if (leftoverIds.length === 0 && leftoverRootKeys.length === 0) {
rmSync(statePath, { force: true });
return { path: statePath, id, removed: existed };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale remover deletes replacement state

The empty-state removal path calls rmSync without checking that its lease is still held. If this writer stalls after the renewal at line 337, another writer can reclaim the expired lease and save a new trust entry. When the original callback resumes, it deletes that replacement writer’s hooks-state.json. Renew or assert ownership immediately before deleting the state file so a reclaimed writer fails instead of removing newer trust data.

Artifacts

Authored reclaimed-lease deletion repro source

  • The test copies source into a temporary sandbox, pauses only the original remover before its empty-state deletion, ages and reclaims its token, commits a replacement entry, and resumes deletion, with the takeaway.

Observed reclaimed-lease repro output

  • The executed repro output confirms writer 2’s replacement state existed before resume and was absent after the original remover resumed, with the takeaway.

View artifacts

T-Rex Ran code and verified through T-Rex

@darko-mijic

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

Comment thread src/senpi/trust-writer.ts
Comment on lines +353 to +354
await lease.renew();
rmSync(statePath, { force: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Lease expiry deletes replacement state

If the removing writer is descheduled longer than staleMs after the final lease.renew(), another writer can reclaim the lease and commit replacement trust state before this callback resumes. This unconditional rmSync then deletes the replacement writer's hooks-state.json, losing newly committed trust entries. Assert the lease is still held immediately before deletion, or use a fenced deletion operation that cannot remove state written by a replacement owner.

Artifacts

Delayed-removal fixture source

  • Captured source of the deterministic fixture that pauses immediately after final lease renewal and before rmSync, takeaway: it exercises the claimed timing window.

Replacement-writer helper source

  • Captured source of the separate writer that reclaims the stale lease and writes replacement hooks-state.json state, takeaway: a second writer is exercised in the pause window.

Executed stale-lease replacement output

  • Captured successful Vitest execution reports replacement state present before delayed rmSync and deleted afterward, takeaway: the current implementation permits the claimed schedule.

View artifacts

T-Rex Ran code and verified through T-Rex

@darko-mijic
darko-mijic merged commit 061408d into main Aug 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant