feat(openclaw): source adapter and filesystem destination (#37) - #108
Conversation
Add openclaw-agent-sqlite-v1 reader for per-agent SQLite stores, composite session ids, default internal/cron filtering, and destination install to workspace skills/ plus ~/.openclaw/skills. Matrix expands to 10×10=100.
Keep the OpenClaw product change free of session screenshots and MCP dumps.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3dcd8ef5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| event = _decode_event(event_json) | ||
| kind = event["type"] | ||
| if kind in {"compaction", "branch_summary", "custom", "session"}: | ||
| continue | ||
| if kind in {"message", "custom_message"}: |
There was a problem hiding this comment.
When an OpenClaw transcript contains a fork or rewind, transcript_events is an append-only tree, but this reducer processes every message in sequence order without following id/parentId from the active leaf. Messages from abandoned sibling branches therefore become handoff turns, making the recovered request/action incorrect and potentially exposing abandoned content; select the active path before sanitizing and emitting messages.
Useful? React with 👍 / 👎.
| if kind in {"compaction", "branch_summary", "custom", "session"}: | ||
| continue |
There was a problem hiding this comment.
When OpenClaw has compacted or pruned older transcript entries, the compaction summary is the remaining representation of that context. Unconditionally skipping these events leaves only the retained tail and produces an incomplete handoff; emit the summary as inert context when its compaction node belongs to the active branch.
Useful? React with 👍 / 👎.
| if session_filter is not None: | ||
| for item in items: | ||
| native = item.session_id.split(":", 1)[-1] | ||
| if item.session_id == _composite_id(agent_id, session_filter) or native == session_filter: | ||
| values.append(item) |
There was a problem hiding this comment.
When the requested current session is older than the first 50 eligible nodes, or names a reset window present only in session_windows, items has already been capped to current session_nodes before this exact-ID filter runs. The reader consequently returns E_NO_MATCH and never reaches _show_session's historical-window fallback; query exact composite/native references directly across nodes and windows before applying the normal listing cap.
Useful? React with 👍 / 👎.
| LIMIT ? | ||
| """, | ||
| (session_id, limit), | ||
| ).fetchall() |
There was a problem hiding this comment.
For databases larger than the snapshot ceiling, the adapter uses a live query-only connection where event_json rows can be arbitrarily large, yet fetchall() materializes up to 50,001 rows before the per-record and aggregate byte checks below run. A corrupt or unusually large local store can therefore exhaust process memory instead of returning E_LIMIT_EXCEEDED; iterate and validate each row as it is fetched, or enforce byte bounds in the query.
Useful? React with 👍 / 👎.
| ## Product | ||
|
|
||
| Offline, local-only **context migration** across enabled source adapters × enabled destination hosts (currently **9×9=81** cells, **derived from registries** — not a fixed product constant). | ||
| Offline, local-only **context migration** across enabled source adapters × enabled destination hosts (currently **10×10=100** cells, **derived from registries** — not a fixed product constant). |
There was a problem hiding this comment.
This change declares the current registry-derived matrix as 100 cells and adds OpenClaw to the installed smoke, but the repository's installed-runner honesty gate still states that the current count is 81. That leaves contradictory verification guidance for the same commit; update the gate to the derived 100-cell result or remove its fixed count.
AGENTS.md reference: AGENTS.md:L23-L25
Useful? React with 👍 / 👎.
Address Codex P1s: walk id/parentId when present, keep compaction summaries on the active path, and resolve exact session ids before the list cap.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ce97c8c71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| role = event.get("role") | ||
| if role not in {"user", "assistant", "tool"}: | ||
| continue | ||
| text = _message_text(event) |
There was a problem hiding this comment.
When an OpenClaw transcript row uses the tree-event message shape (type/id/parentId at the top level and role/content inside message), this reads both fields from the outer event. Every user and assistant message is therefore skipped and show succeeds with an empty turns tuple; the synthetic fixtures hide this by flattening those fields. Unwrap the nested message and extract only its textual content before sanitizing.
Useful? React with 👍 / 👎.
| break | ||
| seen.add(identifier) | ||
| path.append(current) | ||
| parent = current.get("parentId") |
There was a problem hiding this comment.
When an ID-bearing compaction identifies retained history via firstKeptEntryId or firstKeptSeq, always following parentId walks through the entries that the summary replaced, so the handoff duplicates compacted text and can restore content removed from active context. Fresh evidence on the current HEAD is this newly added ancestry loop, which never consults either retention marker; follow the retention pointer at compaction nodes while still emitting their summary.
Useful? React with 👍 / 👎.
| candidates = [ | ||
| event | ||
| for event in decoded | ||
| if event.get("type") in {"message", "custom_message", "compaction"} | ||
| ] |
There was a problem hiding this comment.
When the active leaf descends from a branch_summary node, filtering that non-emitting node out before building by_id leaves its child with a missing parent, so the ancestry walk stops there and drops the entire earlier active conversation. Fresh evidence on the current HEAD is that the new branch reducer indexes only these three emitting types; retain recognized graph-only nodes in the ancestry index and omit them only during turn emission.
Useful? React with 👍 / 👎.
Honor nested message.role/content, keep branch_summary in ancestry, and follow compaction firstKept* pointers so abandoned/replaced nodes stay out of the active path.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63d19d5ce5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for event in reversed(candidates): | ||
| if event.get("type") not in emit_types: | ||
| continue |
There was a problem hiding this comment.
When the newest active entry is a branch_summary with no subsequent message, this loop skips it and selects the preceding emitting event, which may be an abandoned sibling rather than the summary's parent; the resulting handoff can therefore expose the wrong branch. Fresh evidence beyond the earlier ancestry-index issue is that the leaf-selection loop still explicitly excludes graph-only summary nodes even though they are now retained in by_id.
Useful? React with 👍 / 👎.
| if session_filter is None and len(values) >= DEFAULT_BOUNDS.listed_sessions: | ||
| break |
There was a problem hiding this comment.
In a multi-agent home where the first alphabetically discovered agent contributes 50 eligible sessions, this break prevents all later agent databases from being examined. Because the global timestamp sort happens only afterward, latest can select an older session while a newer session exists under another agent; collect each agent's bounded prefix and apply the 50-session cap only after the global merge.
Useful? React with 👍 / 👎.
| role = _event_role(event) | ||
| if role not in {"user", "assistant", "tool"}: | ||
| continue |
There was a problem hiding this comment.
For tree-shaped tool events whose nested message role is toolResult (and similarly bashExecution), this allowlist drops the event because it accepts only the normalized role tool. Tool-heavy sessions consequently lose their persisted results even though the content decoder can read their text, leaving the recovered action/context incomplete; map supported public tool-result roles to tool before sanitization.
Useful? React with 👍 / 👎.
| for index, event in enumerate(candidates): | ||
| identifier = event.get("id") | ||
| if isinstance(identifier, str) and identifier: | ||
| by_id[identifier] = event |
There was a problem hiding this comment.
When a corrupt or concurrently inconsistent transcript repeats an event id, this assignment silently replaces the earlier node. Parent lookups can then splice ancestry through whichever duplicate appeared last, producing a plausible but incorrect branch instead of failing closed; detect an existing identifier and return E_CORRUPT_RECORD as is already done for duplicate sequence numbers.
Useful? React with 👍 / 👎.
Scan every agent before the listed_sessions cap, allow branch_summary as the active leaf, map toolResult roles, and fail closed on duplicate event ids.
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b53060af83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(seq, int): | ||
| by_seq[seq] = event | ||
| else: | ||
| by_seq[index + 1] = event |
There was a problem hiding this comment.
When event_json omits its own seq and a filtered custom or session row precedes the retained entry, this fallback renumbers the filtered candidate list rather than using transcript_events.seq. A firstKeptSeq can consequently resolve to the wrong event—or the compaction node itself—dropping retained context from the handoff. Fresh evidence beyond the earlier retention-pointer finding is that the current implementation reads the SQL sequence at line 659 but never attaches it to the decoded event before constructing by_seq.
Useful? React with 👍 / 👎.
| try: | ||
| with _open_connection(database, root, budget) as connection: | ||
| _require_schema(connection, expected_agent=agent_id) | ||
| if session_filter is not None: |
There was a problem hiding this comment.
When show receives a supported text reference such as the fixture title Synthetic basic, _parse_ref assigns the entire value to session_filter, so this branch performs only exact session-ID queries and returns no summaries before select_session can match the title. The OpenClaw source profile inherits exact_ref_kinds=("id", "path", "text", "latest"), but text and path references therefore return E_NO_MATCH; after an exact-ID miss, fall back to the normal bounded listing and generic selection path.
Useful? React with 👍 / 👎.
| if role in {"toolResult", "bashExecution", "tool_result", "function"}: | ||
| role = "tool" |
There was a problem hiding this comment.
For a nested bashExecution message, the persisted payload uses command and output rather than text or content; this new normalization reaches the tool branch, but _message_text still returns None and line 696 discards the event. Tool-heavy sessions therefore continue losing bash commands and results. Fresh evidence beyond the earlier role-normalization finding is that the current HEAD maps bashExecution without adding the corresponding payload decoder.
Useful? React with 👍 / 👎.
| role = _event_role(event) | ||
| if role in {"toolResult", "bashExecution", "tool_result", "function"}: | ||
| role = "tool" | ||
| if role not in {"user", "assistant", "tool"}: |
There was a problem hiding this comment.
For a standard visible custom_message containing top-level content and display: true, _event_role returns None because these entries do not carry a user/assistant role, so the allowlist skips content that was explicitly persisted for display. Handle custom_message separately, require its visibility flag, and normalize its public content to an appropriate inert turn.
Useful? React with 👍 / 👎.
| if not isinstance(identifier, str) or not identifier or identifier in seen: | ||
| break |
There was a problem hiding this comment.
When a corrupt or concurrently inconsistent transcript contains a parent cycle, encountering an already-seen ID merely breaks the walk and returns the accumulated nodes as a plausible active conversation. This hides the structural corruption and can produce an invented handoff; raise E_CORRUPT_RECORD when identifier in seen, as the reducer already does for duplicate IDs.
Useful? React with 👍 / 👎.
| values.sort( | ||
| key=lambda item: (item.updated_at is None, item.updated_at or "", item.session_id), | ||
| reverse=True, | ||
| ) |
There was a problem hiding this comment.
When more than 50 sessions across agents share the same millisecond timestamp, reverse=True reverses the session-ID tie-break as well as the timestamp, so lexicographically larger IDs survive the cap. The reader's canonical summary_sort_key uses ascending IDs for timestamp ties; because the cap runs first here, latest can select a different session than it would from the complete merged set. Sort timestamp descending while keeping the ID tie-break ascending.
Useful? React with 👍 / 👎.
Attach transcript_events.seq for firstKeptSeq, treat free-text refs as selector input, decode bashExecution tool bodies, and fail closed on cycles.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29453dc4e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not values and not any_supported and _agent_db_paths(root): | ||
| raise DiagnosticError("E_UNSUPPORTED_FORMAT", source=self.key, provider=FORMAT_ID) |
There was a problem hiding this comment.
When a healthy store is queried with a composite reference whose agent does not exist, such as missing:sess-1, the agent filter skips every database, leaving any_supported false while the second _agent_db_paths(root) call remains nonempty. This raises E_UNSUPPORTED_FORMAT for an ordinary stale or mistyped reference instead of returning an empty list so the reader can emit E_NO_MATCH.
Useful? React with 👍 / 👎.
| ).fetchall() | ||
| if len(rows) > DEFAULT_BOUNDS.scanned_records: |
There was a problem hiding this comment.
When one agent database contains more than the scan ceiling of session nodes, fetchall() materializes every selected row—including potentially large entry_json values—before the length check runs. A corrupt or unusually large local store can therefore exhaust memory before returning E_LIMIT_EXCEEDED; add a SQL limit of the ceiling plus one or iterate rows while enforcing the bound.
Useful? React with 👍 / 👎.
| # Bound per agent, but never stop scanning other agents before the | ||
| # global timestamp sort (Codex P1 multi-agent latest). | ||
| values.extend(items[: DEFAULT_BOUNDS.listed_sessions]) |
There was a problem hiding this comment.
In a multi-agent home, _list_nodes applies scanned_records independently to each database and this extension can retain 50 summaries per agent without consuming the shared ReadBudget. With many otherwise valid agents, one list operation can read millions of rows and build up to 100,000 summaries before the reader's final aggregate check rejects them; enforce the scan and result bounds cumulatively across the loop.
Useful? React with 👍 / 👎.
Merge disposition (exact HEAD
|
1 participant
Summary
Closes #37 (filesystem/product path).
openclawadapter foragents/<id>/agent/openclaw-agent.sqlite(openclaw-agent-sqlite-v1, schema user_version 11)agentId:sessionId; default list filters internal/cron/spawn/run/pluginskills/+~/.openclaw/skillsdirect Skill rootsadapters/openclaw.py; matrix 10×10=100openclaw skills install/ picker activation remains not-runTest plan