feat: add polyglot agent framework adapters by qiankunli · Pull Request #1 · compforge/agent-ledger

qiankunli · 2026-08-09T04:25:04Z

No description provided.

qiankunli · 2026-08-09T04:27:05Z

+    defaults:
+      run:
+        working-directory: typescript
+    steps:


🤖 devloop code-review · deepseek-v4-pro · ready in 1m 44s

Both the new typescript and go jobs lack a timeout-minutes setting. If a test hangs (e.g., infinite loop, deadlocked async operation, or a make test that waits forever on a missing service), the job will run until GitHub's default 6-hour timeout, wasting runner minutes and delaying other queued workflows.

Add timeout-minutes: 10 (or whatever is appropriate for these test suites) to each job:

typescript: runs-on: ubuntu-latest timeout-minutes: 10 ... go: runs-on: ubuntu-latest timeout-minutes: 10 ...

_{ccr:fp=55e1a7720aef}

ccr:label=debatable — GitHub Actions 已有 6 小时硬上限，仓库现有 Python job 同样未设 timeout-minutes；为新 job 增加更短预算是运维偏好，不构成功能或可靠性缺陷 #padding

qiankunli · 2026-08-09T04:28:42Z

+				failure = fmt.Errorf("%v; record failure: %w", failure, err)
+			}
+			output <- agentgo.StreamEvent{Type: agentgo.StreamEventError, Err: failure}
+		}


🤖 devloop code-review · deepseek-v4-pro · ready in 3m 23s

In GenerateStream, the non-terminal early-close path (lines 72-78) checks ctx.Err() == nil and then does an unconditional output <- agentgo.StreamEvent{...} at line 77. There is a TOCTOU race: the context could be cancelled between the nil check at line 72 and the send at line 77. If the caller has already stopped reading from output due to the cancellation, this goroutine blocks forever on the send, leaking the goroutine (since defer close(output) cannot execute while blocked).

The fix: use the same select pattern as the main event loop (lines 66-70) to respect context cancellation on the synthetic error send:

select { case output <- agentgo.StreamEvent{Type: agentgo.StreamEventError, Err: failure}: case <-ctx.Done(): }

_{ccr:fp=794427f69418}

ccr:label=important — 属实；go/adapters/agentgo/model.go:72 先检查 ctx 后在 :77 无条件发送，取消与发送之间存在竞态，缓冲区满时 goroutine 无法响应取消，应改为 select 发送

已在 0871630 修复：early-close 的终端事件改为 select 发送，可在输出缓冲区满时响应 ctx 取消，并补了满缓冲取消回归测试。

qiankunli · 2026-08-09T04:31:25Z

+	}
+	recorder.expectedVersion = expectedVersion
+	return recorder, nil
+}


🤖 devloop code-review · seed-2.1-turbo · ready in 6m 5s

ResumeRecorder accepts RecorderOptions which includes ExpectedVersion, but the value is silently ignored: NewSessionRecorder initially respects it, then ResumeRecorder unconditionally overwrites recorder.expectedVersion with the version from the loaded stream. The TypeScript reference implementation prevents this at the type level by using Omit<RecorderOptions, "expectedVersion">. A caller passing ExpectedVersion to ResumeRecorder would have their intent silently dropped, potentially causing the first Record to use a stale expected version.

Fix: either (a) add a validation error if ExpectedVersion is non-nil in ResumeRecorder, or (b) change the parameter to exclude ExpectedVersion (e.g., accept a separate struct or document that it's ignored).

_{ccr:fp=5a108ea5c3dc}

ccr:label=minor — 属实；go/recorder.go:45 接收含 ExpectedVersion 的 RecorderOptions，但 :54 总会用扫描结果覆盖它，与 NewSessionRecorder 的同名参数语义不一致，属于 API 易误用点

已在 0871630 修复：ResumeRecorder 传入 ExpectedVersion 时现在显式报错，避免与扫描得到的 stream head 产生两套语义。

qiankunli · 2026-08-09T04:33:33Z

        self.store = store
-        self.stream = RunStream(session_id=session_id, run_id=run_id)
+        self.run_id = run_id
+        self.stream = EventStream(session_id=session_id, stream_id=run_id)


🤖 devloop code-review · seed-2.1-pro · ready in 8m 14s

The Python SessionRecorder does not support a stream_id parameter, unlike its TypeScript counterpart. In __init__ (line 48) and resume (line 64), EventStream is always constructed with stream_id=run_id:

self.stream = EventStream(session_id=session_id, stream_id=run_id)

The TypeScript RecorderOptions includes an optional streamId (line 9 of typescript/packages/core/src/recorder.ts), and the constructor uses streamId ?? runId (line 25). This is critical for use cases like LedgerPiSessionStorage, where the stream identity (framework/pi/native-session) differs from the runId.

If a Python adapter or framework needs to store events under a stream key that differs from the logical run_id, there is no way to express that — the recorder will always write to and read from stream_id=run_id, causing data misalignment when interoperating with code that uses a different stream_id.

_{ccr:fp=0b0802851dd7}

ccr:label=important — 属实；python/src/agent_ledger/recorder.py:48 和 :64 都把 stream_id 固定为 run_id，使 Python 适配器无法使用 RFC 0002 定义的跨 runtime framework state stream #cross-file

已在 0871630 修复：Python SessionRecorder 构造与 resume 均支持独立 stream_id，并覆盖 framework state stream 跨 runtime run 续写及因果父链隔离。

qiankunli · 2026-08-09T04:35:31Z

+	return func(yield func(StoredEvent, error) bool) {
+		s.mu.Lock()
+		events, err := clone(s.streams[streamKey(stream)])
+		s.mu.Unlock()


🤖 devloop code-review · deepseek-v4-pro · ready in 10m 11s

Load and ScanSession both call clone() on the entire event slice while holding s.mu. The clone function does a full JSON marshal + unmarshal round-trip, which is O(n) in the number of events and involves significant allocation and serialization overhead. For a stream with thousands of events, this blocks all other operations — including Append — for the duration of the clone.

The lock is only needed to snapshot the slice reference; the deep copy could be done outside the critical section. For example: snapshot the slice header under the lock, then clone outside the lock, or use a shallower copy strategy since the stored events are logically immutable after commit.

_{ccr:fp=a162c590705a}

ccr:label=minor — 属实；go/memory_store.go:123-125 与 :153-155 在全局 mutex 内执行整段 JSON 深拷贝，长 session 会不必要地阻塞并发 append；可在锁内复制 slice、锁外深拷贝

已在 0871630 修复：Load/ScanSession 只在 mutex 内复制 slice，JSON 深拷贝移到锁外。

qiankunli · 2026-08-09T04:37:18Z

+        -1,
+        str(uuid4()),
+        [event(second_session, second_run, event_id=event_id)],
+    )


🤖 devloop code-review · deepseek-v4-pro · ready in 11m 58s

The new test test_event_id_uniqueness_is_scoped_to_session has no explicit assertions. It verifies that the store does not raise DuplicateEvent when the same event_id is used across different sessions, but it never confirms that the second event was actually stored. A store bug that silently drops the second append would not be caught.

Add at least one assertion verifying the events were persisted, e.g.:

assert len(await collect(event_store.scan_session(first_session))) == 1 assert len(await collect(event_store.scan_session(second_session))) == 1

_{ccr:fp=eb1a093f4b61}

ccr:label=minor — 属实；python/tests/test_store_contract.py:109-127 只以不抛异常证明跨 session 复用 ID，未验证两次 append 均已持久化，补 scan/read 断言会使契约测试更完整

已在 0871630 修复：测试现在分别读取两个 session 的 stream，并断言相同 event_id 均已持久化。

qiankunli · 2026-08-09T04:38:36Z

🤖 devloop code-review · origin/main..HEAD · ccd9f66bb

6 finding(s)（6 条已作为独立 review thread 发布）

feat: add polyglot agent framework adapters

ccd9f66

qiankunli commented Aug 9, 2026

View reviewed changes

fix: support MySQL 8 authentication

b43e035

qiankunli commented Aug 9, 2026

View reviewed changes

fix: address adapter review findings

0871630

Uh oh!

Conversation

qiankunli commented Aug 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiankunli commented Aug 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants