feat(discovery): add snapshot-backed OpenClaw discovery adapter - #201
prasith-jobleap wants to merge 13 commits into
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (result.spawnError?.code === "ENOENT") { | ||
| fs.rmSync(snapshotDir, { recursive: true, force: true }); | ||
| snapshotDir = null; | ||
| return null; |
There was a problem hiding this comment.
openclaw is not installed, this branch returns no sessions without a warning. Because OpenClaw is enabled by default, a run can appear to have found no history when it could not collect that history. The repository requires missing capabilities to be named along with how to fix them, rather than silently degraded.
Context Used: If there is a VISION.md file at the root of the repo, the PR must not conflict / diverge / drift from it. If the PR description has an "Intent" section, respect that as the accepted user intent. - Do make comments if anything in the implementation ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
On the latest Greptile note ("Missing binary is silent"): this is deliberate. A missing |
|
Speaking as Kun's firstmate. Diff reviewed on Contract-class: new-default. Unconfigured discovery gains VISION per-rule (live
Not waiting on the author for CI/no-mistakes — those are green. Waiting on a captain decision before merge:
I will not squash-merge until that call. No competing PR from me. |
…, drop dead flags
…apter PR. ci-1: every generation (live and archived) now gets `${session_id}:${sha256(startedAt\nJSON.stringify(events[0]))}`; the "first-seen keeps the bare session_id" branch in discover() was removed and the generation-key collision fallback kept; adapter header comment and README identity sentence updated. ci-2: the <active_memory_plugin> strip pattern now requires its closing tag, so an unclosed block leaves following human words intact (sibling wrapper patterns already required terminators). ci-3: discoverTranscripts computes the remote-capable harness list once and skips collectHosts entirely when it is empty (single call site). Tests added/updated: live id stable beside a later distinct archive and across its own archival; id independent of archive processing order (created_at swap); unclosed memory block preserved; configured hosts + harnesses ["openclaw"] produces no ssh calls and empty perHost/remoteMasters; bare-id assertions switched to the digest form. Verified the six new/updated tests fail on the pre-fix source and pass after. TMPDIR=/private/tmp pnpm run check: lint, format:check, typecheck OK; 885 tests pass, 0 fail. Changes left uncommitted in the worktree (README.md, src/discovery/adapters/openclaw.js, src/discovery/index.js, test/openclaw-adapter.test.js, test/remote-discovery.test.js)
… PR per the user's instructions. ci-1 (archive order changes identity). Invariant: a generation's id depends only on its own anchor, class (live/archived), generation key, and whether its anchor group holds other distinct content, never on processing order. discover() now runs two passes: the per-row loop reads every generation, dedupes by content digest per session_id (live wins over content-equal archives, duplicate archives keep the first read) and stores anchor + record; then `identify(sessionId, generations)` groups by anchor and assigns `${session_id}:${sha256(anchor)}` to every member of a single-digest group, while in a multi-digest group the live member keeps the anchor id and every archive gets `${session_id}:${sha256(anchor + "\n" + String(generation))}`. The order-dependent `generationId`/`seen` fallback was removed. The --since cutoff is still applied after identity. Header comment updated; it documents the remaining identity moves honestly: a live member of a collision group being archived, and also a lone archive gaining its first colliding sibling (that second move is inherent to the specified design, since a lone archive holds the plain anchor id until a sibling appears; noted here because the instruction asked to document only the first). README identity sentence updated accordingly. ci-2 (unclosed block bypasses self exclusion). Invariant: when any injected wrapper, closed or unclosed, precedes Backpass's sentinel, the first user message after stripping starts with the sentinel. stripScaffolding now, after removing complete wrappers, detects a leading unterminated opener (internal context, conversation info, or active_memory_plugin); if the self sentinel appears anywhere it drops everything before it, otherwise it drops only the opening `<active_memory_plugin>` tag and keeps the rest. The discover() self check (`startsWith(SELF_SESSION_SENTINEL)` after stripping) therefore sees the sentinel for all three openers; read() shares the same function so distill gets the same text. Tests (test/openclaw-adapter.test.js): twin collision group ids unchanged when archive processing order is reversed via created_at; a live member keeps its anchor id when colliding archives appear, and adding an earlier colliding archive leaves every existing id unchanged; unclosed memory block drops only its opening tag; unclosed wrapper of each kind before the sentinel yields text starting with the sentinel (and text before the sentinel is kept when no wrapper is unterminated); discover() excludes a session whose first user message is a closed or unclosed wrapper followed by the sentinel. Five tests fail on the pre-fix adapter (verified by swapping in HEAD's file) and pass after. Verification: TMPDIR=/private/tmp pnpm run check: eslint clean, prettier clean, tsc clean, 887 tests pass / 0 fail. Changes left uncommitted in the worktree: README.md, src/discovery/adapters/openclaw.js, test/openclaw-adapter.test.js
…sions) on the OpenClaw adapter per the user's instructions; ci-1 untouched as directed. Invariant: stripping never discards human text, and the self sentinel excludes a session only when it is the first non-wrapper content of the first user message (at the very start or right after a closed leading wrapper). `stripScaffolding` is the single boundary shared by discover()'s self check and read()/distill, so fixing it there covers both consumers. src/discovery/adapters/openclaw.js: removed the `UNTERMINATED_WRAPPER` regex and the branch that sliced text up to a quoted `SELF_SESSION_SENTINEL`. For an unterminated `<active_memory_plugin>` block only the opening tag is dropped and every remaining character is kept (unterminated internal-context and conversation-info openers are left verbatim as before). The discover() self check is unchanged (`startsWith(SELF_SESSION_SENTINEL)` after stripping) and so now fires only when the sentinel leads. The header comment notes the tradeoff: a Backpass prompt behind a truncated wrapper is not recognised by the sentinel and relies on the cwd/state-dir check in src/discovery/self.js, which Backpass-spawned sessions (cwd inside `<repo>/.backpass/`) always hit. test/openclaw-adapter.test.js: (1) the adapter-level exclusion test now asserts the bare sentinel and a closed wrapper + sentinel are excluded, while a human quote, an unclosed memory block, and an unclosed internal-context opener followed by the sentinel are all kept by discover(); (2) replaced the old "unclosed wrapper before the sentinel is dropped" test with one asserting every preceding character survives for each unclosed opener (memory block loses only its tag), and only a closed leading wrapper leaves the sentinel first; (3) new end-to-end test through discoverTranscripts with config.state.root set: a Backpass session whose first user message is an unclosed memory block followed by the sentinel, with workspace cwd under `.backpass/synthesis`, is excluded (perHarness.openclaw.self === 1), while a human session in the repo quoting the sentinel after the same unclosed wrapper is kept. All three tests fail on the pre-fix adapter (verified by swapping in HEAD's file) and pass after. Verification: TMPDIR=/private/tmp pnpm run check: eslint clean, prettier clean, tsc clean, 888 tests pass / 0 fail. README needed no change (it does not describe sentinel handling). Changes left uncommitted in the worktree: src/discovery/adapters/openclaw.js, test/openclaw-adapter.test.js
Sessions with no recorded cwd fall back to the configured OpenClaw workspace. That path is a guess, so it no longer reaches the deterministic path tiers: discovery caps it at tier 3 (labelled best-effort, excluded by --strict), per VISION's deterministic-association rule. Recorded cwds (session metadata or event headers) keep their normal tiers. Also drops internal ticket ids from test names.
01d7a19 to
03be937
Compare
prasith-jobleap
changed the title
| ? Math.max(...times) | ||
| : Math.max(row.updated_at || row.created_at, ...times); | ||
| const { events, model } = normalized(entries); | ||
| const anchor = [...(times.length ? [String(times[0])] : []), String(JSON.stringify(events[0]))].join("\n"); |
There was a problem hiding this comment.
Knowledge Base Used: Transcript distillation and sampling
|
Speaking as Kun's firstmate. Re-triage after substantive author pushes since the prior waiting-on-captain stamp on Author addressed captain item #2 (cwd-less association): Contract-class: still new-default. Unconfigured discovery still gains VISION per-rule (live
Not waiting on the author for CI/no-mistakes — those are green. Still waiting on a captain decision before merge (only remaining open item):
Item #2 from the prior stamp is cleared by the author's tier-3 cap. I will not squash-merge until the default-harness call. No competing PR from me. |
2 participants
What Changed
src/discovery/adapters/openclaw.js, a new local-only, SQLite-backed harness adapter that reads an online backup snapshot (openclaw backup sqlite create, orBACKPASS_OPENCLAW_DB) rather than the live database, covers live branches plus deleted/reset archives (zstd-aware), assigns order-independent generation identity from each session's anchor, strips injected context/system scaffolding, excludes probe/eval/test namespaces and self-sentinel sessions, and removes its private temp snapshot via a newcleanuphook invoked fromsrc/cli.js.openclawinALL_HARNESSES, theADAPTERSmap, the--harnesshelp text, andtranscriptSource(provenance keyed by agent, not the moving snapshot path); typed OpenClawrun/cron/subagent/heartbeat/acp/hooksessions classify as non-interactive insrc/interaction.js.cwdInferred) at tier 3 withconfidence: "inferred"(capInferredinsrc/discovery/index.js), user-scope project normalization skips those rows, andcollectHostsis skipped entirely when no selected harness can be collected remotely; README and tests (test/openclaw-adapter.test.js,test/fixtures/openclaw/build.js,test/remote-discovery.test.js,test/config.test.js) cover the new harness and these rules.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The final fix round is a one-condition change to capInferred that matches the recorded user decision exactly; I traced the tier-1, tier-3-on-its-own, strict, project-scope, and user-scope cache paths and the inferred label now survives normalizeProjects in every case, with a regression test that fails on the prior code, and no other sibling relabel site exists.
Testing
Built an isolated sandbox (throwaway git repo, isolated HOME and XDG config, synthetic schema-23 OpenClaw snapshots built with the repo's fixture builder, and fake openclaw/ssh/acpx binaries), then drove
backpass scanandbackpass analyzeend-to-end: project-scope discovery with correct namespace exclusion and interactive/non-interactive kinds, inferred-cwd sessions labelled tier 3 and dropped by --strict in project and user scope (including a deleted-subdirectory workspace), session identity unchanged across archival and across colliding-archive processing order, --since using archive activity time, self-session exclusion for bare and closed-wrapper sentinels with a human quote kept, injected scaffolding absent from the trace the analysis agent actually receives while human words after an unclosed wrapper survive, silent empty harness when the openclaw binary is absent, the backup-snapshot path through a fake openclaw with cleanup of the private temp directory and refusal of an artifact outside it, and no ssh contact for configured hosts under --harness openclaw with a control run proving the host is contacted otherwise. All passed. One early run accidentally used a PATH containing the operator's real openclaw binary; its backup command failed (exit 1) under the sandbox HOME, wrote nothing to real data, and the scenario was re-driven with the binary absent.openclaw backup sqlite create --agent main --repository <private tmp> --json, reads the returned snapshot, and removes the private temp directory on exitEvidence: Project-scope scan table (snapshot A)
Evidence: Project-scope scan JSON (snapshot A)
Evidence: Strict scan drops inferred-cwd sessions
Evidence: Identity across archival and archive order
A livex gen=live id=livex:acd93426bb9fda… B livex gen=gen-7 id=livex:acd93426bb9fda… C livex gen=live id=livex:acd93426bb9fda… / gen-3 id=livex:bbbc2145991280… D livex gen=live id=livex:acd93426bb9fda… / gen-3 id=livex:bbbc2145991280…Evidence: Self-session exclusion
Evidence: Trace delivered to the analysis agent (scaffolding stripped)
Evidence: Missing openclaw binary
Evidence: Backup snapshot path and temp cleanup
Evidence: Backup artifact escape refused
Evidence: Backup failure warning
Evidence: Hosts skipped for openclaw-only run
Evidence: User-scope inferred label preserved
Evidence: --since uses archive activity time
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (4) ✅
src/discovery/adapters/openclaw.js:384- For a transcript whose entries carry no timestamps (a case the adapter explicitly supports:startedAt = times[0] ?? row.created_atat line 370 and the P1-3 test's undated archive), the anchor's time component is the row's owncreated_at. That value differs between the livesession_windowsrow (window creation) and thesession_transcript_archivesrow (archive creation), so an undated session's id changes when it is archived even though its content is unchanged. This contradicts the header comment (lines 18-21) and the README sentence 'identity survives archival', and the gap ledger would count the same session as two sightings across runs. Sibling site: the mtime fallback at line 374 uses the same undated path but is harmless there. Smallest remedy: whentimesis empty, build the anchor from the first event alone (no time component) or note the exception in the header and README; the anchor formula was specified by the user in an earlier decision, so the change needs sign-off rather than a silent fix.🔧 Fix applied.
1 warning still open:
src/discovery/index.js:295-capInferred(added in fix commit 373dbde, the starting head) returns a fresh{ tier, confidence, reason }object and so discards theprojectandprojectRootfields that every scope's associate function puts on the association: project scope sets both torepo.root(src/scope.js:205-213) and user scope sets them from the git toplevel, remote, or cwd key (src/scope.js:69-104). Every CLI path reaches this throughscan.js:12, which always passesscope. Concrete sequence: the repo under analysis is the configured OpenClaw workspace (the usual place for an OpenClaw AGENTS.md), an OpenClaw session has no recorded cwd (the fixture'sfallbackshape),associatereturns tier 1 withproject = repo.root,capInferredrewrites it to tier 3 withprojectundefined, andtoTranscript(index.js:421) storesproject: null. Downstream that session is silently omitted fromanalyzedByProject,sourceProjects, andsessionsByProjectin src/fold.js:73,123,133, from the gap ledger'sprojectfield (src/gap-ledger.js:243), and from the user-scopeminGapProjectsgate (src/proposal.js:612-624), so its evidence can never satisfy the project floor and the relevance-by-project table undercounts it. Nothing errors. The intended cap is only on tier/confidence/reason; the smallest remedy is to spread the original association ({ ...association, tier: 3, confidence: "inferred", reason }) soproject/projectRootsurvive, and to extend the unit test at test/openclaw-adapter.test.js:534-544 with an association carryingproject/projectRootso the test cannot pass while the keys are dropped. No sibling sites:capInferredis called only at index.js:308, and the remote path never sees OpenClaw rows because the adapter islocalOnly.🔧 Fix applied.
2 warnings still open:
src/discovery/index.js:295-capInferred(added in fix commit 373dbde, the starting head) returns a fresh{ tier, confidence, reason }object and so discards theprojectandprojectRootfields that every scope's associate function puts on the association: project scope sets both torepo.root(src/scope.js:205-213) and user scope sets them from the git toplevel, remote, or cwd key (src/scope.js:69-104). Every CLI path reaches this throughscan.js:12, which always passesscope. Concrete sequence: the repo under analysis is the configured OpenClaw workspace (the usual place for an OpenClaw AGENTS.md), an OpenClaw session has no recorded cwd (the fixture'sfallbackshape),associatereturns tier 1 withproject = repo.root,capInferredrewrites it to tier 3 withprojectundefined, andtoTranscript(index.js:421) storesproject: null. Downstream that session is silently omitted fromanalyzedByProject,sourceProjects, andsessionsByProjectin src/fold.js:73,123,133, from the gap ledger'sprojectfield (src/gap-ledger.js:243), and from the user-scopeminGapProjectsgate (src/proposal.js:612-624), so its evidence can never satisfy the project floor and the relevance-by-project table undercounts it. Nothing errors. The intended cap is only on tier/confidence/reason; the smallest remedy is to spread the original association ({ ...association, tier: 3, confidence: "inferred", reason }) soproject/projectRootsurvive, and to extend the unit test at test/openclaw-adapter.test.js:534-544 with an association carryingproject/projectRootso the test cannot pass while the keys are dropped. No sibling sites:capInferredis called only at index.js:308, and the remote path never sees OpenClaw rows because the adapter islocalOnly.src/discovery/index.js:295- The "inferred" label that fix round 373dbde introduced (and round 47498ce preserved alongside project fields) is overwritten in user scope by the pre-existingnormalizeProjectsin src/scope.js:257-268, which treats every tier-3 association with a local cwd as a bare-cwd association. Concrete sequence:backpass scan --scope user(openclaw is enabled by default), an OpenClaw session with no recorded cwd falls back to the configured workspace, that workspace is a git toplevel, soassociateUserreturns tier 1confidence: "git"and indexes its worktrees intoknownWorktrees(the cached association is the uncapped one, src/scope.js:245-251);capInferredreturns a fresh{ tier: 3, confidence: "inferred", reason: "inferred cwd (no recorded cwd): ..." }; thenscope.normalizeProjects(transcripts)(src/discovery/index.js:171) matches the cwd against the indexed root (listWorktreesalways includes the root, src/repo.js:54-66) and setsassociation.confidence = "git"andassociation.reason = "cwd is in registered worktree ..."(src/scope.js:267-268). Tier stays 3 and--strictstill drops the session, but the distill header (association: tier 3 (git), src/distill.js:136) now presents a guessed cwd as a recorded git location, which is the exact mislabel the cap was meant to prevent. Nothing errors. No sibling sites: project scope has no normalizer and the remote path never sees OpenClaw rows. Smallest remedy: innormalizeProjectsskip associations already labelled inferred (if (transcript.association?.confidence === "inferred") continue;), with a user-scope test asserting the capped association keepsconfidence: "inferred"afterdiscoverTranscripts.🔧 Fix applied.
3 warnings still open:
src/discovery/index.js:295-capInferred(added in fix commit 373dbde, the starting head) returns a fresh{ tier, confidence, reason }object and so discards theprojectandprojectRootfields that every scope's associate function puts on the association: project scope sets both torepo.root(src/scope.js:205-213) and user scope sets them from the git toplevel, remote, or cwd key (src/scope.js:69-104). Every CLI path reaches this throughscan.js:12, which always passesscope. Concrete sequence: the repo under analysis is the configured OpenClaw workspace (the usual place for an OpenClaw AGENTS.md), an OpenClaw session has no recorded cwd (the fixture'sfallbackshape),associatereturns tier 1 withproject = repo.root,capInferredrewrites it to tier 3 withprojectundefined, andtoTranscript(index.js:421) storesproject: null. Downstream that session is silently omitted fromanalyzedByProject,sourceProjects, andsessionsByProjectin src/fold.js:73,123,133, from the gap ledger'sprojectfield (src/gap-ledger.js:243), and from the user-scopeminGapProjectsgate (src/proposal.js:612-624), so its evidence can never satisfy the project floor and the relevance-by-project table undercounts it. Nothing errors. The intended cap is only on tier/confidence/reason; the smallest remedy is to spread the original association ({ ...association, tier: 3, confidence: "inferred", reason }) soproject/projectRootsurvive, and to extend the unit test at test/openclaw-adapter.test.js:534-544 with an association carryingproject/projectRootso the test cannot pass while the keys are dropped. No sibling sites:capInferredis called only at index.js:308, and the remote path never sees OpenClaw rows because the adapter islocalOnly.src/discovery/index.js:295- The "inferred" label that fix round 373dbde introduced (and round 47498ce preserved alongside project fields) is overwritten in user scope by the pre-existingnormalizeProjectsin src/scope.js:257-268, which treats every tier-3 association with a local cwd as a bare-cwd association. Concrete sequence:backpass scan --scope user(openclaw is enabled by default), an OpenClaw session with no recorded cwd falls back to the configured workspace, that workspace is a git toplevel, soassociateUserreturns tier 1confidence: "git"and indexes its worktrees intoknownWorktrees(the cached association is the uncapped one, src/scope.js:245-251);capInferredreturns a fresh{ tier: 3, confidence: "inferred", reason: "inferred cwd (no recorded cwd): ..." }; thenscope.normalizeProjects(transcripts)(src/discovery/index.js:171) matches the cwd against the indexed root (listWorktreesalways includes the root, src/repo.js:54-66) and setsassociation.confidence = "git"andassociation.reason = "cwd is in registered worktree ..."(src/scope.js:267-268). Tier stays 3 and--strictstill drops the session, but the distill header (association: tier 3 (git), src/distill.js:136) now presents a guessed cwd as a recorded git location, which is the exact mislabel the cap was meant to prevent. Nothing errors. No sibling sites: project scope has no normalizer and the remote path never sees OpenClaw rows. Smallest remedy: innormalizeProjectsskip associations already labelled inferred (if (transcript.association?.confidence === "inferred") continue;), with a user-scope test asserting the capped association keepsconfidence: "inferred"afterdiscoverTranscripts.src/discovery/index.js:294- Sibling site of the invariant fix rounds 373dbde and 2d9bc53 addressed ("an inferred cwd is labelled inferred and never relabelled"), left behind rather than introduced by 2d9bc53.capInferredreturns the association untouched whenassociation.tier >= 3, so an inferred cwd that only reaches tier 3 on its own never receivesconfidence: "inferred", and the new guard innormalizeProjects(src/scope.js:261) does not recognise it. Concrete user-scope sequence:backpass scan --scope user(openclaw on by default); an OpenClaw session has no recorded cwd and falls back to the configured workspace<repo>/agent-workspace, a subdirectory that has since been deleted while<repo>is a live git repo.gitToplevelreturns null for a missing path (src/repo.js:46), soassociateUseryields tier 3confidence: "cwd"(src/scope.js:96-102);capInferredreturns it unchanged because tier is already 3; any other session recorded in<repo>indexes its worktrees intoknownWorktrees;normalizeProjectsthen matches<repo>/agent-workspaceunder<repo>and setsconfidence = "git",reason = "cwd is in registered worktree <repo>"(src/scope.js:267-268). The distill header (src/distill.js:136) andscanreason column (src/commands/scan.js:149) now present a guessed, non-existent path as a registered git location, the same mislabel r3-1 fixed for the tier-1 case. Nothing errors; tier stays 3 and--strictstill drops it, so this is a labelling defect only. Project-scope sibling: the same early return leaves a dead-path tier 3 (confidence: "path"/"glob", src/discovery/association.js:114-118) without the inferred label, so the row'sextra.cwdSource: "configured-workspace"is the only hint the cwd was guessed. Smallest remedy: drop theassociation.tier >= 3early return so everycwdInferredrow is stamped{ ...association, tier: 3, confidence: "inferred", reason: "inferred cwd (no recorded cwd): ..." }, which also makes thenormalizeProjectsguard cover it. This flips the asserted outcome of the pipeline-authored unit test at test/openclaw-adapter.test.js:586-587 (capInferred(tier3, { cwdInferred: true })currently expected to return the input unchanged) and changes the cap's designed contract, so the remedy, not the defect, needs sign-off.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
openclaw backup sqlite create --agent main --repository <private tmp> --json, reads the returned snapshot, and removes the private temp directory on exitbackpass scan --harness openclaw --since all(plain and--json) in project scope against snapshot A with BACKPASS_OPENCLAW_DB setbackpass scan --harness openclaw --since all --strict(tier-3 inferred sessions dropped)backpass scan --harness openclaw --since 30d --json(old-activity archive created today excluded)backpass scan --jsonacross snapshots A/B/C/D comparing nativeId for a live session, its unchanged archive, an undated session, and a colliding earlier/later archivebackpass scan --harness openclawwith openclaw absent from PATH and no BACKPASS_OPENCLAW_DBbackpass scan --harness openclawwith a fakeopenclaw backup sqlite createbinary on PATH and TMPDIR isolated; checked temp dir removed after exitAdversarial fake openclaw returning an artifact outside its repository, and one exiting 1backpass scan --harness openclawwith discovery.hosts configured and BACKPASS_SSH_BIN pointing at a logging fake ssh; control run with--harness claude,openclawbackpass scan --scope user --harness openclaw --since all --json(and--strict) with the configured workspace as a git toplevel, and with OPENCLAW_STATE_DIR pointing at a config whose workspace is a deleted subdirectorybackpass analyze --harness openclaw --analysis-agent pi --jobs 1 --limit 1 --jsonwith BACKPASS_ACPX_BIN set to a recording fake acpx; inspected the raw events file and distilled prompt it received✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.