Skip to content

feat: add tape recording system for workspace analyze sessions#30

Open
STRRL wants to merge 3 commits intomasterfrom
tape-opus-4.6
Open

feat: add tape recording system for workspace analyze sessions#30
STRRL wants to merge 3 commits intomasterfrom
tape-opus-4.6

Conversation

@STRRL
Copy link
Copy Markdown
Owner

@STRRL STRRL commented Mar 20, 2026

Implement a Go port of republic's tape system that records agent
execution as append-only JSONL entries. Each workspace analyze run
generates a .tape.jsonl file capturing messages, tool calls, results,
errors, and events with token usage metadata.

  • pkg/tape: entry model, JSONL store, eino callback handler, slog handler
  • Integrate JSONL tape recorder in analyzer via Config.TapePath
  • Register slog tape handler globally in main.go for live logging

Implement a Go port of republic's tape system that records agent
execution as append-only JSONL entries. Each workspace analyze run
generates a .tape.jsonl file capturing messages, tool calls, results,
errors, and events with token usage metadata.

- pkg/tape: entry model, JSONL store, eino callback handler, slog handler
- Integrate JSONL tape recorder in analyzer via Config.TapePath
- Register slog tape handler globally in main.go for live logging
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 311196d385

ℹ️ 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".

Comment thread pkg/analyzer/analyzer.go
Comment on lines +110 to +113
if config.TapePath != "" {
jsonlStore := tape.NewJSONLStore(config.TapePath)
callbacks.AppendGlobalHandlers(tape.NewHandler(jsonlStore))
slog.Info("Tape recording enabled", "path", config.TapePath)
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 Badge Avoid appending tape callbacks to global handler state

RunAgentWithPrompt installs a new tape callback via callbacks.AppendGlobalHandlers on every invocation, but global handlers are process-wide and never removed. If this function is called more than once in a process (e.g., repeated analyzer runs in tests or embedded usage), later runs will invoke all previously registered tape handlers, causing duplicated events and writes to stale tape files from earlier sessions.

Useful? React with 👍 / 👎.

Comment thread pkg/tape/store.go Outdated
func NewJSONLStore(path string) *JSONLStore {
return &JSONLStore{
path: path,
nextID: 1,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve unique entry IDs when appending to existing tape

The store always starts nextID at 1, but writes are append-only, so rerunning workspace analyze against an existing .tape.jsonl file creates duplicate IDs in the same tape. Any consumer that relies on Entry.ID uniqueness or monotonic ordering will get ambiguous records after the first rerun.

Useful? React with 👍 / 👎.

NewJSONLStore now scans existing JSONL file for the maximum entry ID
and resumes from there, preventing duplicate IDs across reruns.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 241ca19df1

ℹ️ 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".

Comment thread pkg/tape/handler.go
Comment on lines +110 to +117
var lastOutput callbacks.CallbackOutput
for {
chunk, err := output.Recv()
if err != nil {
break
}
lastOutput = chunk
}
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 Badge Aggregate streamed model chunks before writing tape

onEndWithStream overwrites lastOutput for every Recv() and only records the final chunk, so streamed chat responses lose earlier chunks (including incremental content/tool-call arguments) and the resulting tape can be incomplete or wrong for multi-chunk generations. Because Eino streaming callbacks deliver per-chunk outputs, this should either append each chunk or merge chunks before calling recordModelOutput.

Useful? React with 👍 / 👎.

Comment thread pkg/tape/slog_handler.go
}

func slogOnStart(ctx context.Context, info *callbacks.RunInfo, input callbacks.CallbackInput) context.Context {
if info.Component == components.ComponentOfChatModel {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard global slog callbacks against nil RunInfo

This handler dereferences info without a nil check; if any component triggers callbacks without injecting RunInfo (which Eino allows for standalone/misconfigured components), the global handler will panic and can crash the CLI. Since NewSlogHandler is appended globally in main, add a defensive info == nil path before reading info.Component/info.Name in these callbacks.

Useful? React with 👍 / 👎.

…sit)

Add nil checks for RunInfo in all tape handler and slog handler callbacks.
Eino allows callbacks to be triggered without RunInfo for standalone or
misconfigured components; without these guards, the global handlers would
panic and crash the CLI.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 511195df4f

ℹ️ 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".

Comment thread pkg/tape/store.go
Comment on lines +53 to +57
}
if maxID == 0 {
return 1
}
return maxID + 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Check scanner errors before reusing tape IDs

scanMaxID never checks sc.Err() after the scan loop, so it treats a truncated scan as success and returns maxID+1. Because Append can write entries larger than the scanner's 10MB limit, a later reopen can stop early with ErrTooLong, miss the true highest ID, and then assign duplicate IDs on subsequent appends, corrupting tape ordering.

Useful? React with 👍 / 👎.

Comment thread pkg/tape/handler.go
Comment on lines +109 to +111
if info == nil {
return
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Close stream copies even when RunInfo is missing

This early return skips closing the output stream copy when info is nil, but OnEndWithStreamOutput handlers are required to close their stream copies to let the original stream fully release resources. In standalone or misconfigured callback contexts where RunInfo can be absent, this can leak goroutines and keep stream cleanup from completing (the same pattern appears in pkg/tape/slog_handler.go).

Useful? React with 👍 / 👎.

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