Skip to content

Add Codebuff/Freebuff agent support - #1261

Draft
scross01 wants to merge 55 commits into
kenn-io:mainfrom
scross01:agents/codebuff
Draft

Add Codebuff/Freebuff agent support#1261
scross01 wants to merge 55 commits into
kenn-io:mainfrom
scross01:agents/codebuff

Conversation

@scross01

Copy link
Copy Markdown
Contributor

Parses Codebuff and Freebuff sessions from ~/.config/manicode/projects/<project>/chats/<timestamp>/. Both agents share the same on-disk layout; sessions are classified by the agentType field in run-state.json.

Highlights

  • Full transcript parsing: user/ai/error messages, tool calls with skill attribution, thinking blocks, subagent invocations
  • Block types: text, tool, agent, mode-divider, plan, ask-user, image
  • Credits tracking: extracts creditsUsed from run-state.json and maps to cost (1 credit = $0.01)
  • Freebuff auto-classification: sessions with "free" in agentType appear as a separate filter

Unique considerations

  • Freebuff shares the Codebuff provider (single registration) to avoid double-discovery and skip-cache contention
  • The agentType field is an agent template name (e.g. base2-free-deepseek), not the actual LLM model -- the real model is selected server-side
  • contextTokenCount provides context window size but per-message token breakdown is not available in the on-disk format

Limitations

  • Mid-session model switches are invisible -- the on-disk format has no per-turn model field
  • Freebuff has no credits (ad-supported); cost tracking only applies to Codebuff
  • The agent field on ChatMessage is not populated in the persisted format, so per-turn agent attribution relies on subagent blocks only

scross01 added 3 commits July 18, 2026 21:32
Review the codebuff/freebuff parser against the public upstream source
(https://github.com/CodebuffAI/codebuff, rev b285b56) and fix gaps:

- Handle error variant messages (API failures, rate limits, country blocks)
  as system messages instead of silently dropping them
- Extract creditsUsed/directCreditsUsed from run-state.json and map to
  CostUSD in usage events (1 credit = /bin/bash.01)
- Handle ask-user blocks (agent questions) as system messages
- Handle plan blocks as system messages with plan content
- Handle image blocks in both user and AI messages
- Update session-format-sources.md with source evidence, billing
  documentation, and model name limitations
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (6399713)

High-severity issues prevent this change from safely handling Freebuff sessions; three findings require attention.

High

  • internal/parser/codebuff.go:109, internal/parser/types.go:828 — Freebuff sessions use a distinct agent identity and freebuff: ID, but Freebuff is absent from Registry. This breaks prefix lookup and single-session resync, excludes Freebuff ownership during reconciliation, and can leave duplicate rows when classification changes between Codebuff and Freebuff.
    • Fix: Persist both variants as Codebuff and distinguish them via AgentLabel, or implement first-class alias handling across registry lookup, ownership, reconciliation, and file-path replacement. Add Freebuff sync, resync, and deletion tests.

Medium

  • internal/parser/codebuff.go:570 — Tool output is double-encoded by marshaling output.Raw. Strings gain extra quotes, while objects and arrays are displayed as stringified JSON rather than raw result content.

    • Fix: Store output.Raw directly, calculate ContentLength from decoded content, and assert the decoded result in the existing tool-call test.
  • internal/parser/types.go:828 — The Codebuff registry entry lacks usage capabilities despite having no per-message token data and reporting costs in credits. Zero-cost sessions miss unsupported-usage messaging, and credit totals stay zero because AgentNameUsesAICredits returns false.

    • Fix: Set NoPerMessageTokenData and AICreditsDenominated, with equivalent behavior for Freebuff.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 6m41s

scross01 added 2 commits July 24, 2026 16:08
High:
- Add freebuff: prefix alias in AgentByPrefix so single-session resync,
  prefix lookup, and reconciliation work correctly for Freebuff sessions

Medium:
- Fix tool output double-encoding: use output.Raw directly instead of
  json.Marshal(output.Raw) which wraps strings in extra quotes
- Add UsageCapabilities (NoPerMessageTokenData, AICreditsDenominated)
  to Codebuff registry entry for correct usage messaging and cost display
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (95ceb2b)

The Codebuff/Freebuff integration has three medium-severity correctness issues involving provider lifecycle handling, transcript ordering, and malformed input.

Medium

  • internal/parser/codebuff.go:111 — Freebuff lifecycle operations use the wrong provider identity. Freebuff sessions are stored with agent = "freebuff", while discovery and reconciliation are registered only under Codebuff. Agent-keyed lifecycle operations therefore may not tombstone deleted Freebuff sources, and tier changes can leave duplicate active sessions. Add first-class provider-alias handling across reconciliation, baselines, file-path cleanup, capability lookup, and raw-ID normalization, with integration tests covering deletion and tier changes.

  • internal/parser/codebuff.go:535 — AI block ordering is not preserved. Blocks are grouped by type and emitted in a fixed system/thinking/text/results order, so sequences such as text → tool/output → text are rendered incorrectly. Process blocks sequentially, flushing accumulated messages at tool results and other block-type transitions.

  • internal/parser/codebuff.go:428 — Non-array JSON roots are accepted as empty transcripts. A valid JSON document with an unexpected root type produces a successful session with no messages, potentially replacing previously stored messages. Reject non-array roots with a parse error and add a regression test for an existing session encountering this input shape.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 10m22s

… validation

- Freebuff sessions now use agent=AgentCodebuff (not AgentFreebuff) so
  that lifecycle operations keyed by agent type (reconciliation, deletion,
  baselines) work correctly. AgentLabel distinguishes UI display.
- Rewrote parseCodebuffAIMessage to process blocks sequentially, flushing
  at block-type transitions to preserve text → tool → text ordering.
- Reject non-array JSON roots in chat-messages.json with parse error
  instead of silently returning empty transcript.
- Simplified provider_effects.go by removing Freebuff-specific agent
  mismatch exception (no longer needed since both use AgentCodebuff).
- Removed Freebuff prefix exception from validateProviderSessionID.
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (5ad8bc2)

The review identified three medium-severity correctness issues in session identity, transcript ordering, and timestamp reconstruction.

Medium

  • internal/parser/codebuff.go:114 — Freebuff sessions lack a distinct agent identity. They are persisted with agent = "codebuff" and differ only by agent_label. Because metadata, filters, and analytics group by agent, Freebuff does not appear separately, and filtering for "freebuff" returns no sessions. Persist a distinct Freebuff identity while supporting the shared provider lifecycle, or make filtering and aggregation consistently label-aware across all storage backends.

  • internal/parser/codebuff.go:610 — AI block source ordering is not preserved. Subagent output is buffered and flushed before its tool call, while mode, plan, and ask-user blocks are deferred until the turn ends. This can display results and control events beside the wrong content. Emit blocks in source order, associate subagent results with their tool calls, and emit control blocks immediately.

  • internal/parser/codebuff.go:803 — Cross-midnight timestamps receive incorrect dates. Every time-only timestamp uses the session directory’s calendar date, producing incorrect activity windows for sessions spanning midnight. Reconstruct timestamps sequentially, advancing the date when the clock rolls past midnight, and add a cross-midnight regression test.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m11s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (5ad8bc2)

The Codebuff/Freebuff parser has five medium-severity correctness issues; no security vulnerability was identified.

Medium

  • internal/parser/codebuff.go:114 — Every Freebuff session is stored as AgentCodebuff; AgentLabel changes only display text. Because agent lists and filters use the agent column, Freebuff never gets an independent filter, while Codebuff includes both tiers. Implement Freebuff as an end-to-end filterable identity while preserving shared-provider lifecycle handling, or remove the separate-filter claim and unused frontend entry.

  • internal/parser/codebuff.go:46agentType is assigned as the model for every message and usage event, even though it represents an agent template and the server-selected model can change during a session. This makes model headers, filters, analytics, and usage breakdowns inaccurate. Leave the model unknown unless the actual model is persisted, and emit reported-credit usage independently.

  • internal/parser/codebuff.go:610 — AI block ordering is not preserved: subagent output precedes its tool call, alternating reasoning/text blocks are regrouped, and mode/plan/ask-user blocks are deferred. Emit block runs in encounter order and add ordering assertions for alternating text/reasoning, agent output, and system blocks.

  • internal/parser/codebuff.go:789 — Time-only messages always use the session directory’s initial date. Sessions crossing midnight therefore assign post-midnight messages to the previous day, corrupting timestamps and activity bounds. Track timestamps in transcript order and advance the date when the time-of-day wraps past midnight.

  • internal/parser/codebuff.go:181 — The final contextTokenCount snapshot is recorded as PeakContextTokens, although the field represents the current per-step context count. Compaction can make the final value lower than the true peak. Derive a genuine maximum from persisted history, expose it as current-context metadata, or leave peak context unavailable.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 4m2s

1. Freebuff identity: Reverted to AgentFreebuff for distinct filtering,
   with prefix alias in AgentByPrefix for lifecycle operations.

2. Model name accuracy: Left model unknown since actual LLM model is
   selected server-side and can change mid-session. Credits usage
   events are emitted independently of model.

3. AI block ordering: Agent output now emitted immediately with tool
   call, system blocks (mode/plan/ask-user) emitted in source order
   instead of deferred to end.

4. Cross-midnight timestamps: Track date in parseCodebuffMessages and
   advance when time-of-day wraps past midnight.

5. PeakContextTokens: Removed incorrect peak context assertion since
   contextTokenCount from run-state.json is the final per-step value,
   not the peak (compaction can reduce it).
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (340668f)

Verdict: Three medium-severity lifecycle, discovery, and freshness issues should be addressed before merging.

Medium

  • Freebuff lifecycle handling is incompleteinternal/parser/codebuff.go:115
    Freebuff sessions are stored with the distinct AgentFreebuff identity, but only Codebuff is registered. Although prefix lookup is aliased, reconciliation, source baselines, and path cleanup remain keyed by agent type. Deleted Freebuff sources may never be tombstoned, and Freebuff-to-Codebuff reclassification can leave stale rows.
    Fix: Use one canonical stored agent/ID and distinguish Freebuff through metadata, or make every agent-keyed lifecycle path alias-aware. Add deletion and bidirectional reclassification integration tests.

  • Discovery is neither bounded nor reliably error-reportinginternal/parser/codebuff_provider.go:22
    The provider advertises streaming discovery, but WithFileDiscovery materializes every session twice before yielding, while os.ReadDir buffers entire directories. Memory scales with total archive size, and directory errors can appear as successful partial discovery.
    Fix: Implement WithStreamingFileDiscovery using bounded directory traversal helpers, propagate incomplete-discovery errors, and add small-versus-large archive scaling coverage.

  • Same-stat source changes can be skippedinternal/parser/codebuff_provider.go:146, internal/sync/engine.go:7530
    The composite fingerprint hashes all three source files, but Codebuff is not registered for hash-based freshness. Same-size changes with an unchanged maximum mtime—particularly companion-file updates—may be skipped despite changed content.
    Fix: Add AgentCodebuff to the hash-aware freshness/cache policy and test same-stat changes in run-state.json and chat-meta.json.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m20s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (340668f)

Codebuff/Freebuff support has three medium-severity correctness and scalability issues that should be addressed before merging.

Medium

  • internal/parser/codebuff.go:115 — Freebuff sessions may not be tombstoned or found after moving. Freebuff sessions are stored as AgentFreebuff, but only AgentCodebuff is registered and configured. Reconciliation checks ownership using the configured agent, so deleted Freebuff sources are missed. Prefix normalization also strips only codebuff:, blocking fallback lookup for moved freebuff: sessions. Define provider-owned aliases throughout reconciliation, lookup, baselining, and capability resolution, or store both tiers as AgentCodebuff with the tier represented separately. Add an engine-level deletion/resync test.

  • internal/parser/codebuff.go:564 — Mixed text and reasoning blocks are reordered. Reasoning and regular text use separate buffers, while flushText always emits reasoning first. Sequences such as regular text → reasoning → regular text therefore lose transcript chronology. Preserve a single ordered block stream or flush the active buffer whenever textType changes, with tests for alternating block types.

  • internal/parser/codebuff_provider.go:50 — Discovery memory scales with the entire archive. Discovery materializes all session directories and then creates a second complete match slice, making periodic reconciliation consume memory proportional to archive size. Stream matches during traversal with WithStreamingFileDiscovery, propagate unexpected traversal errors, and add small-versus-large archive cardinality coverage.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 6m52s

…ss, streaming

1. Freebuff lifecycle: Added alias handling in logicalRootsForAgentWatchRoots,
   tombstoneMissingWatchSourcesForAgentLocked, and baselineEligibleProviders
   so Freebuff sessions are properly tombstoned and baselined.

2. Text/reasoning interleaving: Replaced separate thinkingBuf/textBuf with
   unified textEntry buffer that preserves source order of alternating
   reasoning and regular text blocks.

3. Hash-aware freshness: Added AgentCodebuff to providerFingerprintHashInCacheKey
   and providerFingerprintHashRequiredForFreshness so same-stat changes in
   companion files are caught by hash comparison.

4. Streaming discovery: Converted Codebuff provider to use
   WithStreamingFileDiscovery with codebuffDiscoverEach that streams
   directory entries instead of materializing the entire archive.
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (71dc0e5)

Codebuff/Freebuff support has two medium-severity correctness issues involving shared-provider lifecycle handling and usage capabilities.

Medium

  • internal/parser/codebuff.go:151 — Identity transitions can leave stale or incorrectly revived sessions. A source can switch between freebuff: and codebuff: when run-state.json changes or disappears. Because applyProviderFilePathPolicies queries only Codebuff rows, a Freebuff-to-Codebuff transition can leave the old Freebuff row active or recreate a trashed Freebuff source as Codebuff.

    • Fix: Perform stale-row and trash checks across both agents. Add tests for transitions in both directions and for a missing run-state.json.
  • internal/parser/types.go:835 — Freebuff usage capabilities are not resolved through Codebuff. Freebuff sessions are stored as AgentFreebuff, but usage capabilities exist only on the Codebuff provider definition. As a result, AgentFilterLacksPerMessageTokenData("freebuff") returns false and the UI silently reports zero usage instead of an unsupported state.

    • Fix: Treat Freebuff as a Codebuff alias during capability lookup without registering another discovery provider, and add coverage for Freebuff usage filters.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 6m15s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (71dc0e5)

Codebuff/Freebuff support has four medium-severity correctness issues that should be addressed before merge.

Medium

  • Mutable transcript updates are ignoredinternal/parser/codebuff_provider.go:178, internal/sync/engine.go:10937
    Codebuff reparses the entire mutable JSON transcript, but normal sync uses the append-only message writer. Changes to existing tool outputs, subagent status/content, or shifted ordinals are ignored even though session counts change. Force full message replacement for both AgentCodebuff and AgentFreebuff, with an integration test covering an in-place block update.

  • Session IDs can collide across projectsinternal/parser/codebuff.go:61, internal/parser/codebuff.go:151
    IDs contain only the agent and timestamp directory name, but timestamps are scoped beneath projects. Identical timestamps in different projects collide, while codebuffFindFile ambiguously returns the first match. Include a stable project identifier in the session ID and raw lookup key, and test identical timestamps across two projects.

  • Codebuff/Freebuff ownership transitions can leave duplicate sessionsinternal/sync/engine.go:7410
    File-path lifecycle policy queries only the Codebuff agent. If a Freebuff session becomes Codebuff, such as after permanent removal of run-state.json, the existing freebuff: row is not found or excluded, leaving duplicate active sessions for one source. Apply stale-row and resurrection checks across both ownership types for the shared provider.

  • Freebuff raw-ID fallback strips the wrong prefixinternal/parser/types.go:951
    AgentByPrefix recognizes freebuff: but returns a definition whose IDPrefix is codebuff:. Callers such as FindSourceFile therefore fail to strip the Freebuff prefix when the stored source path is stale or absent. Return an alias definition with Type: AgentCodebuff and IDPrefix: "freebuff:", or make prefix stripping alias-aware.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 8m28s

…, replacement

1. Session ID collision: Include project name in session ID to prevent
   collisions across projects with identical timestamps.

2. Freebuff raw-ID fallback: AgentByPrefix now returns definition with
   freebuff: IDPrefix so callers can strip the prefix correctly.

3. Freebuff usage capabilities: Added alias handling in
   AgentNameLacksPerMessageTokenData and AgentNameUsesAICredits so
   Freebuff is treated as a Codebuff alias for capability lookup.

4. ForceReplaceOnParse: Set ForceReplaceOnParse capability for Codebuff
   since it reparses the entire mutable transcript on every sync.
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (4fe008d)

Codebuff/Freebuff support has three medium-severity synchronization and lookup issues that can leave stale or duplicate sessions.

Medium

  • internal/parser/codebuff_provider.go:217 — Force-replace behavior is not activated. ForceReplaceOnParse is advertised only as a capability, while singleFileSourceSet.Parse never sets ParseOutcome.ForceReplace. Edits, deletions, and streaming updates at existing ordinals are therefore ignored by the append-only writer, leaving stale transcripts. Set ForceReplace on Codebuff parse outcomes and add an integration test covering modification or removal of an existing message.

  • internal/parser/codebuff.go:116 — Freebuff ownership is excluded from generic lifecycle policies. Freebuff sessions are stored as AgentFreebuff, but source cleanup, force-replace ownership lookup, and resurrection policies query only AgentCodebuff. ID changes or reclassification can leave old rows active, create duplicates, or bypass trash/resurrection guards. Add provider-owned agent aliases and apply both Codebuff and Freebuff consistently, with sync tests for both reclassification directions.

  • internal/parser/codebuff_provider.go:104 — Raw-ID fallback constructs the wrong path. Parsed IDs contain codebuff:<project>:<timestamp>, but codebuffFindFile treats <project>:<timestamp> as the timestamp directory. It therefore cannot recover normally parsed sessions when the stored path is unavailable. Split the raw ID into project and timestamp and search <root>/<project>/chats/<timestamp>/chat-messages.json, retaining timestamp-only compatibility if required.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 6m34s

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (4fe008d)

High-severity sync and medium-severity identity, usage, and source-recovery issues need correction before merge.

High

  • internal/parser/codebuff_provider.go:217ForceReplaceOnParse is declared as a capability, but successful full parses still return ParseOutcome.ForceReplace == false. Normal sync therefore uses the append-only writer, leaving stale messages when mutable Codebuff transcripts edit, remove, or reorder existing blocks. Make successful full parses explicitly request replacement and add an integration test covering modified and removed blocks between syncs.

Medium

  • internal/parser/codebuff.go:189 — Reported-credit usage events have an empty Model, while SQLite, PostgreSQL, and DuckDB usage queries exclude events where model == ''. Codebuff costs are consequently omitted from session usage, dashboards, and reports. Use an honest non-empty placeholder or consistently support model-less explicit-cost events across all three backends, with persisted-usage tests.

  • internal/sync/engine.go:7410 — Same-path identity cleanup checks only the provider job’s codebuff agent even though the provider can emit Session.Agent == freebuff. Classification or canonical-ID changes can leave old Freebuff rows active, and trashed alternate identities may be missed. Make file-path policies alias-aware for both Codebuff and Freebuff, with transition and trash-resurrection tests.

  • internal/parser/codebuff_provider.go:104 — Parsed raw IDs use <project>:<timestamp>, but codebuffFindFile treats the entire raw ID as the timestamp directory. Source lookup fails when the stored path becomes unavailable, breaking recovery after moves. Align stable ID generation and decoding with the on-disk project key, and test FindSource using a real parsed full ID.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 9m45s

…el, lifecycle

1. ForceReplace: Created codebuffSourceSet wrapper that overrides Parse
   to set ForceReplace: true on successful parses, ensuring full message
   replacement for the mutable transcript.

2. codebuffFindFile: Updated to split project:timestamp in raw ID for
   the new session ID format, with legacy timestamp-only fallback.

3. Usage events: Added placeholder model name "codebuff" so usage
   queries filter correctly and cost display works.

4. File-path policies: Made alias-aware for both Codebuff and Freebuff
   in ListSessionIDsByFilePath and HasTrashedSessionByFilePath.
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (b5363ab)

Codebuff/Freebuff integration has three medium-severity correctness issues that should be addressed before merging.

Medium

  • Freebuff sessions are not tombstoned after source deletioninternal/sync/engine.go:3173
    Reconciliation candidates and admitted sources remain keyed as codebuff, so Freebuff sessions never receive exact (freebuff, file_path) baseline rows. Deleted Freebuff source files therefore remain active indefinitely.
    Fix: Record baselines under the parsed session’s actual agent, or duplicate Codebuff candidate/admission pairs for Freebuff. Add an integration test covering sync, source removal, and tombstoning.

  • Normalized project names make session IDs unstable and break source lookupinternal/parser/codebuff.go:151, internal/parser/codebuff_provider.go:130
    Session IDs use the cwd-derived display project, but lookup treats that component as the literal source directory. Git-root detection or normalization can change the name (for example, my-project to my_project), causing lookup failures and possible collisions between distinct directories.
    Fix: Build stable IDs from projectHint or another encoded source-directory identity, while using the cwd-derived value only for ParsedSession.Project.

  • A fictitious codebuff model contaminates model reportinginternal/parser/codebuff.go:189, internal/parser/codebuff_provider.go:271
    Although the parser cannot determine the model, it declares model support and assigns "codebuff" as the model for cost events. This produces misleading model totals and filters and conflicts with messages whose model is empty.
    Fix: Mark model capability unsupported or not applicable, update the documentation, and aggregate reported cost-only events without inventing a model identity.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 5m52s

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (b5363ab)

The change has two medium-severity correctness issues affecting session identity and rename reconciliation.

Medium

  • Session IDs can break source lookup and archive stabilityinternal/parser/codebuff.go:151, internal/parser/codebuff_provider.go:130
    IDs embed the git-derived normalized project name, but source lookup treats that segment as the literal storage directory. For example, my-project may become my_project, causing raw-ID lookup to search a nonexistent path. Changes in git-root resolution can also change the same source’s ID, triggering deletion and recreation of its archived session. Build IDs from the stable projectHint/storage-directory component, use the CWD-derived value only for display metadata, and test cases where the storage directory and derived project differ.

  • Freebuff-only directory renames may not trigger reconciliationinternal/sync/engine.go:3005
    The rename probe queries only the watcher’s codebuff agent, while Freebuff sessions are stored under freebuff. A missing directory containing only Freebuff sessions can therefore skip provider-wide reconciliation, potentially tombstoning or missing a surviving moved session on platforms that report only the old rename endpoint. For Codebuff watcher events, query active descendants for both Codebuff and Freebuff and add a rename/reconciliation integration test.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 9m22s

…es, rename

1. Session ID stability: Use projectHint (storage directory name) for
   session IDs instead of cwd-derived project name. Cwd-derived project
   is used only for display metadata.

2. Model capability: Marked Model as CapabilityNotApplicable since the
   actual LLM model is unknown and can change mid-session. Removed
   fictitious "codebuff" model placeholder from usage events.

3. Baseline rows: Use parsed session's agent type for baseline rows
   instead of provider agent type, so Freebuff sessions receive correct
   (freebuff, file_path) baseline entries.

4. Rename probe: Made HasActiveSessionSourceBelow alias-aware for both
   Codebuff and Freebuff to catch rename events for Freebuff-only
   directories.
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (cbc5799)

Codebuff/Freebuff support has one medium-severity usage-reporting issue; no security regressions were identified.

Medium

  • internal/parser/codebuff.go:200 — Credit usage events leave Model empty, but usage aggregation requires ue.model != ''. As a result, Codebuff costs and AI credits are stored but omitted from usage reports. Allow model-less cost events consistently across SQLite, PostgreSQL, and DuckDB, or assign a documented non-empty sentinel model. Add an aggregation-level regression test.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 7m29s

Modified usageEventEligibility to include events with either a model
or a cost (CostUSD). Codebuff/Freebuff usage events have empty model
but carry CostUSD, which was being excluded from usage reports.
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (f0cfb28)

Changes require fixes: three medium-severity correctness and backend-parity issues remain.

Medium

  • internal/db/usage.go:378 — SQL eligibility expression lacks parentheses.
    When concatenated into queries, operator precedence can allow cost-only events to satisfy EXISTS for unrelated sessions, while modeled events bypass timestamp and model filters. Wrap the expression as (ue.model != '' OR ue.cost_usd IS NOT NULL) and test unrelated cost-only events plus bounded model filters.

  • internal/postgres/usage.go:37; internal/duckdb/analytics_usage.go:3166 — Model-less cost events are excluded outside SQLite.
    PostgreSQL and DuckDB still require ue.model != '', causing Codebuff cost events without a model to disappear from usage summaries, session usage, and activity reports. Apply (ue.model != '' OR ue.cost_usd IS NOT NULL) consistently and add backend-parity coverage.

  • internal/sync/engine.go:3380, internal/sync/engine.go:6115 — Codebuff/Freebuff reconciliation ownership is inconsistent.
    Reconciliation candidates remain keyed as Codebuff while stored baselines may be keyed as Freebuff, and freshness skips query only codebuff for CWD admission. A CWD filter may therefore fail to revoke an existing Freebuff deletion baseline, allowing a later missing-source reconciliation to tombstone a filtered session. Treat both agent keys as candidate owners, query both during skipped-source admission, and add a filter-then-delete Freebuff lifecycle test.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m22s

… DuckDB

- Added parentheses to usageEventSourceEligibility for operator precedence
- Updated PostgreSQL usageEventEligibility to include cost-only events
- Updated DuckDB usageEventSourceEligibility and activity report
- Removed 'or' constraint from usage tests to allow OR in eligibility
@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (c167746)

Codebuff/Freebuff support has four medium-severity correctness and scalability issues that should be addressed before merging.

Medium

  • internal/sync/engine.go:6115 — Incorrect deletion baseline for freshness-skipped Freebuff sources. With no parse results, baseline bookkeeping falls back to Codebuff, and the CWD-filter check queries only Codebuff rows. An existing Freebuff baseline can remain active after a session becomes excluded, allowing later source deletion to tombstone an archived session that the ingestion-only filter should preserve. Resolve stored ownership across both aliases and update or reject the baseline under the actual agent; make skippedSourceAllowsCwdFilter query both.

  • internal/parser/codebuff_provider.go:69 — Recursive watching does not scale to large archives. Creating a native watch for every timestamped session directory can exhaust fsnotify/inotify limits, including the common global 8,192-watch budget. Use shallow or project-level watches with periodic reconciliation, and add a cardinality-scaling regression for large archives.

  • internal/sync/engine.go:12018 — Source freshness misses companion-file-only changes. SourceMtime checks only chat-messages.json, while Codebuff freshness also depends on run-state.json and chat-meta.json. If filesystem events are lost, updates to final credit totals, agent classification, or metadata can be missed. Compute a stat-only composite mtime across all three files and add a sibling-only update test.

  • internal/parser/codebuff.go:203 — Credited sessions can emit year-0001 usage timestamps. When no message timestamp is parsable, OccurredAt is derived from a zero startedAt, causing normal date-bounded usage reports to omit the cost. Fall back to the timestamp encoded in the session directory or the source mtime.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 10m32s

scross01 added 2 commits July 25, 2026 13:25
1. Usage timestamp fallback: When no message timestamp is parsable,
   fall back to session directory timestamp, then source mtime, instead
   of producing year-0001 timestamps.

2. Source freshness: Added Codebuff-specific SourceMtime that computes
   composite max mtime across chat-messages.json, run-state.json, and
   chat-meta.json so companion-file-only changes are caught.
1. Shallow watching: Changed codebuffWatchRoots to enumerate project
   directories and emit shallow watches on <root>/*/chats/ instead of
   recursive watches on <root>/. Reduces inotify usage from O(N*M) to
   O(N) where N=projects and M=sessions.

2. CWD filter: Made skippedSourceAllowsCwdFilter query both
   AgentCodebuff and AgentFreebuff so CWD filtering works for sources
   containing only Freebuff sessions.
@roborev-ci

roborev-ci Bot commented Jul 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (1847631)

Codebuff/Freebuff support is promising, but two medium-severity integration issues should be fixed before merging.

Medium

  • Metadata fallback counts are overwritten during syncinternal/parser/codebuff.go:131
    Counts loaded from chat-meta.json are assigned while CountsAuthoritative remains false. Sync then recalculates counts from the empty parsed-message slice and overwrites them with zero, potentially hiding the session. Mark metadata-derived counts authoritative or preserve them during session writes, and add a sync-level regression test.

  • Bare Codebuff/Freebuff session IDs cannot resolve correctlyinternal/parser/codebuff.go:159, cmd/agentsview/session_get.go:76
    Stored IDs use codebuff:<project>:<timestamp>, but raw-ID resolution only prefixes the timestamp with codebuff:. This cannot match the canonical stored ID, and Freebuff is absent from the registry fallback. Resolve raw IDs through SourceSessionID or provider lookup, return the actual canonical ID, and handle duplicate timestamps explicitly.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 11m40s

parser/codebuff.go sets CountsAuthoritative=true when the on-disk
chat-messages.json is empty but chat-meta.json reports a non-zero
messageCount, so the engine no longer zeros MessageCount for
meta-only sessions. Pinned by TestSyncCodebuffMetaOnlySessionKeepsCounts.

Add parser.FindCodebuffFreebuffMatches to walk the configured
codebuff/freebuff storage layer for bare-timestamp lookup. The
newSessionGetCommand RunE pre-flight uses each match CanonicalID
to map a bare timestamp into the canonical ID; one match returns
the ID, many returns an explicit ambiguity list, none falls
through to the standard resolver. Freebuff resolves through its
own roots list even though parser.Registry does not enumerate it.
@roborev-ci

roborev-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (a55100e)

High-severity compile failure and two medium-severity session-resolution/timestamp bugs require fixes before merging.

High

  • cmd/agentsview/session_get.go:73resolveServiceSessionID now requires a *config.Config, but callers in session_messages.go:33, session_tool_calls.go:29, session_watch.go:29, and session_usage.go:138,268 still pass only three arguments, so the command package will not compile.
    • Fix: Pass the appropriate config argument at every call site, or preserve the original signature as a wrapper around a config-aware helper.

Medium

  • cmd/agentsview/session_get.go:152 — Bare Freebuff IDs cannot resolve correctly. Freebuff is absent from parser.Registry, so AgentDirs[AgentFreebuff] is never populated and ResolveDirs(AgentFreebuff) is empty. Matches under the shared CODEBUFF_DIR are classified as Codebuff without inspecting run-state.json, yielding nonexistent Codebuff IDs for Freebuff sessions.

    • Fix: Scan shared Codebuff roots once and classify candidates using run-state.json, or test both canonical prefixes against the session service.
  • internal/parser/codebuff.go:496 — An RFC3339 timestamp resets prevHour without advancing currentDate. In mixed-format sessions spanning midnight, a later time-only timestamp can be assigned to the previous date.

    • Fix: Update currentDate from each absolute timestamp in the session timezone, and add a mixed-format midnight regression test.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 14m4s

scross01 added 4 commits July 28, 2026 07:37
parseCodebuffTimestamp uses the "03:04 PM" layout, which requires a
two-digit hour. The trailing row in
TestParseCodebuffMixedFormatMidnightRollover used "2:00 PM", which
the parser rejected and fell through to the zero time.Time, so
msgs[4].Timestamp.Day() returned 1 for the expected 17. Pad it to
"02:00 PM" so the time-only row actually parses and exercises the
RFC3339 anchoring the test was meant to pin.
Add parser.FindCodebuffFreebuffMatches to walk the configured
codebuff/freebuff storage layer for bare-timestamp lookup,
returning one match per (Agent, project, rawID) location for
ambiguity surfacing rather than silently picking one.

newSessionGetCommand's RunE now pre-resolves bare on-disk
Codebuff/Freebuff timestamps via resolveBareCodebuffID using
both codebuff: and freebuff: prefixes against svc.Get per
matched project, so Freebuff sessions resolve even when the
Freebuff roots list is empty because Freebuff is absent from
parser.Registry.

parseCodebuffMessages anchors currentDate to each absolute
(RFC3339) timestamp's local calendar date so subsequent
time-only messages do not drift back to the session
directory's original date across midnight. prevHour resets
to -1 alongside the anchor.
Pin the (chat-messages, chat-meta, run-state) trio for the
stat-only freshness gate. Three sync tests touch only one
sibling file (chat-meta, run-state) or both, bump the mtime,
and assert the engine reparses exactly one session.

The composite test uses a shared mtime bump across the two
supporting files so a coarse-resolution host filesystem cannot
mask one leg's drift; the meta-only and run-state-only tests
isolate each leg's contribution to the composite mtime max.
@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (163c033)

High-severity compilation failures block the change; five additional medium-severity correctness issues remain.

High

  • Compilation failureinternal/db/usage.go:49,64-65,2188; internal/postgres/usage.go:1638,1840-1842; internal/duckdb/analytics_usage.go:4008,4517; cmd/agentsview/session_messages.go:33
    aiCreditUSD, aggregateCost, mbd, m, allUnpricedPriced, and hasContributingCost are undefined; NoTokenData is missing &&; and several callers omit the new cfg argument to resolveServiceSessionID. Correct the fields and declarations, restore the operator, implement DuckDB state tracking, and update every resolver caller.

Medium

  • Named-model usage totals are doubledinternal/db/usage.go:2182-2215,2386-2417; internal/postgres/usage.go:1632-1663
    Token and cost totals are accumulated in both the new all-model loop and the existing model-breakdown loop. Accumulate totals only in the all-model loop; use the filtered loop solely to build ModelBreakdowns.

  • DuckDB exposes empty model namesinternal/duckdb/analytics_usage.go:4457,4470
    Model-less Codebuff events can return Models: [""] and UnpricedModels: [""]. Guard both map insertions with r.model != "" while separately tracking whether all contributing rows were priced.

  • Bare Freebuff IDs cannot resolvecmd/agentsview/session_get.go:152-158; internal/parser/codebuff_resolve.go:49-50
    Freebuff has no resolved directories, while sessions in the shared Codebuff root are always tagged as Codebuff without inspecting run-state.json. Inspect each candidate’s agentType or probe both canonical IDs through the session service, with a shared-root regression test.

  • Model-less reported costs lose provenanceinternal/db/usage.go:1481-1483; internal/postgres/usage.go:965-967
    Calling RecordReported("", ...) is a no-op, so SQLite and PostgreSQL omit reported-cost provenance for Codebuff-only reports. Use RecordUnattributedReported() for model-less reported costs.

  • Documentation misstates model extractiondocs/configuration.md:386; internal/parser/codebuff.go:47-50
    The documentation says the model comes from agentType, but the parser intentionally leaves it empty because the actual model is unknown. Describe agentType as a template/classification value and state that the actual model is unavailable.


Reviewers: 2 done | Synthesis: codex, 17s | Total: 7m49s

scross01 added 2 commits July 28, 2026 10:25
The post-merge frontend staleness - the codebuffAICredits summary
card in UsageSummaryCards, the generated DbUsageTotals field, the
usage_summary_codebuff_ai_credits locale key across five locales -
is removed. The CodebuffAICredits column in the SQLite, PostgreSQL,
and DuckDB usage pipelines disappears; cost-only reported sessions
now ride as microdollars on money.Money, like every other agent,
with no per-credit shim.

The UsageEvent.Model emitted by the parser now mirrors rs.AgentType
(e.g. base2-deepseek, base2-free-minimax-m3) rather than the agent
name so the daily ModelBreakdown can bucket by template. Emitting
is skipped when rs.AgentType is empty so corrupt sessions don't
surface as ghost model rows; sess.Agent still partitions codebuff
vs freebuff in the agent breakdown.
docs/configuration.md clarifies that the agent template shown in
the session detail header is the classification label used
server-side to pick the per-step LLM; the literal LLM is not
persisted by the CLI and is not visible in the UI. A new paragraph
notes that codebuff/freebuff sessions report cost only, and
per-model rates for base2-* templates aren't in the embedded
pricing tables so cache savings for these rows resolve to zero
by design rather than an aggregator bug.

The package ABOUTME comment in internal/parser/codebuff.go mirrors
this contract.
@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (0e6b9c6)

Medium-severity issues remain in incremental sync and remote bare-ID lookup.

Medium

  • internal/sync/engine.go:5329 — Incremental cutoff checks can miss deletion of run-state.json or chat-meta.json. The surviving files’ maximum mtime may predate the cutoff, filtering out the session before its composite fingerprint is checked and leaving stale classification, cost, CWD, or metadata. Include the session directory mtime as a cutoff signal, consistent with the Kilo Legacy branch, and add companion-file deletion tests.

  • cmd/agentsview/session_get.go:42 — Bare-ID lookup now depends on local configuration and Codebuff files even when using --server. Remote Codebuff/Freebuff IDs cannot resolve without a matching local archive, and unrelated bare IDs may fail through mustLoadConfig despite remote mode not requiring local configuration. Resolve Codebuff-family IDs through the selected SessionService, such as FindSessionIDsByPartial with canonical-suffix filtering, or limit filesystem resolution to direct local-service mode and return configuration errors normally.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 13m38s

scross01 added 2 commits July 28, 2026 13:10
…and bare-ID lookup

Two medium-severity issues addressed:

1. Incremental cutoff checks can miss deletion of run-state.json or
   chat-meta.json. Added session directory mtime as a cutoff signal
   in discoveredFileEffectiveMtime, consistent with the Kilo Legacy
   branch. Companion-file deletions now trigger reparse via the
   directory mtime bump even when surviving files mtimes are old.

2. Bare-ID lookup depends on local configuration even when --server
   is set. Added resolveBareCodebuffIDRemote to resolve Codebuff-family
   IDs through FindSessionIDsByPartial with canonical-suffix filtering,
   skipping mustLoadConfig for remote mode. Extracted resolveCodebuffBareID
   helper to simplify newSessionGetCommand.

Tests added:
- TestSyncCodebuffCompanionFileDeletionReparsesSession
- 7 unit tests for resolveBareCodebuffIDRemote
- Extracted codebuffRemoteLookupLimit constant
@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (cddfbb9)

Codebuff/Freebuff integration needs two medium-severity fixes in remote session resolution.

Medium

  • cmd/agentsview/session_get.go:234 — Remote bare-ID resolution rejects host-prefixed IDs such as host~codebuff:project:<timestamp> because it treats host~codebuff as the agent prefix. This can miss remote-synced sessions or incorrectly report a single local match instead of ambiguity. Call parser.StripHostPrefix(m) before extracting and validating the agent prefix, while retaining the full ID as the candidate.

  • cmd/agentsview/session_get.go:183--pg reads still use local filesystem discovery for bare Codebuff/Freebuff IDs. Sessions originating on other machines, or whose local source no longer exists, cannot be resolved despite being present in PostgreSQL. Use service-based resolution when pgReadRequested(cmd) is true, as already done for explicit --server requests.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 10m44s

Bare timestamps reach `session get` via three routes: local
archives, `--server` daemons, and `--pg` stores. The previous
resolver routed both remote routes through `findSessionIDsByPartial`
plus a suffix filter, which silently mis-classified host-prefixed
candidates and could fold multiple matches into a single local
row when an ambiguity error should have fired. Drops the remote
bare bridge entirely: a non-canonical ID against `--server` or
`--pg` now returns an explicit error pointing at `session list`
and the four canonical ID shapes.

Local reads add a `--machine` flag (default `local`) that gates the
dual-prefix probe by `sessions.Machine`, so remote-synced rows in
the local archive no longer masquerade as local matches.
`isCanonicalServiceSessionID` also recognises the `freebuff:`
prefix (Freebuff is intentionally absent from `parser.Registry`,
mirroring `parser.AgentByPrefix`'s special case) so freebuff
canonical IDs are not misclassified as bare timestamps and trip
the new error path. Removes the now-dead `resolveBareCodebuffIDRemote`
and `codebuffRemoteLookupLimit`, registers `--machine` as a local
flag on `newSessionGetCommand` to avoid colliding with the
independent `--machine` flags on `session list`/`search`/export,
and updates `docs/session-api.md` to document the new transport
split. Adds regression tests for the four-shape error enum, the
empty-filter arm of the machine matcher, and Freebuff canonical-id
recognition.
@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (22e7b9d)

Codebuff/Freebuff support has one medium-severity CLI resolution regression.

Medium

  • cmd/agentsview/session_get.go:247 — Under --server or --pg, every non-canonical ID is rejected, including bare UUIDs for Codex and other agents that resolveServiceSessionID previously resolved by trying registered prefixes. Preserve generic bare-ID resolution for remote stores, and emit the Codebuff-specific error only after determining the input is an unresolved Codebuff/Freebuff timestamp.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 11m35s

… shape

`resolveCodebuffBareID` previously rejected every non-canonical ID
under `--server` / `--pg` with the Codebuff-specific error, including
bare UUIDs for Codex, Copilot, Gemini, and other agents that
`resolveServiceSessionID` resolves by retrying each registered agent
prefix.  This is a regression from the transport-scope commit.

Add `parser.IsCodebuffTimestamp` — a syntactic predicate that checks
whether a string matches the four ISO-8601 shapes the Codebuff parser
accepts (`YYYY-MM-DDTHH-MM-SS.fffZ`, `...Z`, `...fff`, `YYYY-MM-DD`).
Use it as an early-exit gate in `resolveCodebuffBareID` so that inputs
which are not Codebuff/Freebuff timestamps pass through to the generic
prefix resolver unchanged.  The Codebuff-specific error now fires only
when the input is actually a Codebuff timestamp that failed to resolve
locally.

Also:
- Fix `dependriving` typo in doc comment.
- Scope `docs/session-api.md` subsection to the Codebuff/Freebuff
  timestamp shape so users know bare UUIDs for other agents still
  resolve on remote reads.
- Add `TestIsCodebuffTimestamp` (9 subcases) and `_ServerBareUUIDForOtherAgent`
  / `_PGBareUUIDForOtherAgent` regression-fix tests.
- Switch bare-input tests from numeric epoch to real ISO timestamps.
@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (ab050f8)

Medium-severity issues remain in DuckDB token reporting and Codebuff freshness detection.

Medium

  • internal/duckdb/analytics_usage.go:4477 — A Codebuff cost-only event sets hasRows, causing DuckDB to report HasTokenData: true despite containing no token data. SQLite and PostgreSQL derive this flag only from session token fields, creating backend divergence. Track token-bearing rows separately from cost-bearing rows and add a behavioral test for a cost-only Codebuff session.

  • internal/sync/engine.go:8444 — Codebuff freshness reduces three files to total size and maximum modification time. A same-size companion-file rewrite, or offsetting size changes, can leave both values unchanged, causing reconciliation to skip content fingerprinting and retain stale metadata, credits, or agent classification. Persist and compare each component’s size and modification time, with a regression test covering a same-size companion rewrite while another file retains the maximum modification time.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 11m48s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (888663c)

The change is not ready to merge: two build-breaking defects and one baseline reconciliation bug must be fixed.

High

  • internal/parser/types.go:850 — The Codebuff registry entry is missing its opening {, leaving keyed fields outside an AgentDef literal and preventing compilation. Add { before the entry and format the composite literal.

  • internal/sync/engine.go:3319baselineEligibleProviders is referenced without being declared, preventing the sync package from compiling. The existing eligibility call also still receives authoritativeProviders, so merely declaring the new map would not make Freebuff baselines eligible. Populate and pass a consistently named eligibility map, or add the Freebuff alias directly to authoritativeProviders.

Medium

  • internal/sync/engine.go:6394 — The baseline source computed from the parsed or stored Freebuff agent is immediately overwritten by baselineSourceForJob(job), restoring the provider agent (codebuff). Warm reconciliation can then reject and remove the Freebuff ownership baseline, preventing later missing-source reconciliation from tombstoning that session. Preserve the already computed source by removing the reassignment.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 13m47s

scross01 added 2 commits July 29, 2026 07:53
…erge damage

The merge of main into agents/codebuff (#888663c4) brought in
Omnigent support alongside the new reconciliation-baseline
pipeline.  Three structural casualties plus a Freebuff/Codebuff
keying correctness gap had to be repaired for the package to
compile and behave correctly:

- `internal/parser/types.go:870` lost the Codebuff registry
  entry's opening `{` during the merge, leaving its keyed
  fields dangling outside the Registry slice literal.

- The Omniagent-derived `if persistentArchive / if ok /
  if valid { ... } else if ReconciliationSourceResolver { ... }`
  chain in `tombstoneMissingWatchSourcesForAgentLocked` had its
  `if !present { ... ContainsSource(... physicalPath) ... }`
  block displaced into the else-if branch, where `physicalPath`
  is not in scope.  Moved the block back into `if valid { ... }`
  so it keys off the persistentArchive resolver's `physicalPath`
  value as intended.

- The new `authoritativeProviders` map was added alongside a
  now-redundant `baselineEligibleProviders` map;
  `eligibleReconciliationBaselines` consumes
  `authoritativeProviders` but `baselineEligibleProviders` was
  never read.  Collapsed to one consistently named map
  (`authoritativeProviders`) and aliased Freebuff in alongside
  Codebuff — both admission sites (the eligibility call and
  the per-write `authoritativeProviders[write.agent]` check)
  now see Freebuff when Codebuff completes, since Freebuff has
  no `parser.Registry` entry and shares the codebuff root.

`baselineProcessedSource` was double-constructing the session
source — first with the parsed Freebuff agent (correct) and
then again with `baselineSourceForJob(job)` which restored the
provider agent, dropping the Freebuff key on disk.  Drop the
second call; the inline Freebuff/Codebuff resolution remains
where it belongs.

Lint: collapse `isCanonicalServiceSessionID`'s final
`if cond { return true }; return false` into a direct
`return cond` per staticcheck S1008.
SourceMtime for Codebuff now hashes each of chat-messages.json,
run-state.json, and chat-meta.json by (size, mtime) so per-file
size or mtime changes always trigger fingerprinting. DuckDB
HasTokenData tracks token-bearing rows separately from cost-only
rows, matching SQLite and Postgres when a Codebuff row reports
only cost.
@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (0f5a8d9)

Codebuff/Freebuff support is broadly sound, but three medium-severity correctness issues should be addressed.

Medium

  • internal/sync/engine.go:7898 — Errors from ListSessionIDsByFilePath are treated as empty results, allowing processing with incomplete identity data. This can bypass resurrection guards or leave stale session identities. Propagate the error, or conservatively retain/skip the parsed result when any agent lookup fails.

  • internal/sync/engine.go:8872 — Codebuff freshness relies only on total file size and maximum modification time. Aggregate collisions can skip content fingerprinting after companion-file changes, leaving metadata, costs, or lifecycle state stale. Track each companion’s presence, size, and timestamp, and add reconciliation tests for colliding aggregate statistics.

  • cmd/agentsview/session_get.go:197 — When multiple disk locations exist but none resolves for the selected store or machine, the resolver incorrectly reports ambiguity with zero matches. Additionally, tryBoth discards lookup errors, potentially presenting backend failures as ambiguity. Return no resolution for zero valid IDs, deduplicate successful IDs, and propagate lookup errors.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 17m15s

scross01 added 2 commits July 30, 2026 08:54
- Propagate ListSessionIDsByFilePath errors with per-agent tracking
  and one-shot retry instead of treating failures as zero matches.
- Replace aggregate size/mtime freshness with per-component stat-hash
  digest via MultiFileStatHasher and provider_freshness side-table.
- Fix bare-ID resolver: deduplicate IDs, return error on zero matches
  with lookup failures instead of false ambiguity.
@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (e7fab6f)

The Codebuff/Freebuff integration has three medium-severity freshness-cache correctness issues that should be fixed before merging.

Medium

  • Freshness hashing is disconnected from the default providerinternal/parser/codebuff_provider.go:101, internal/sync/engine.go:623
    ComputeMultiFileStatHash is implemented on codebuffSourceSet, but the factory returns *SourceSetProvider, so the engine’s type assertion never succeeds and provider_freshness is unused. A digest mismatch also falls through to the lossy size/max-mtime shortcut, potentially missing per-file changes. Expose the hasher on the returned provider, make a missing or mismatched digest force fingerprinting/parsing, and add an end-to-end test using the default factory.

  • Freshness digests are persisted before successful processinginternal/sync/engine.go:8073
    The digest is stored before CWD filtering and before the downstream session transaction commits. A filtered or failed write can therefore mark an absent or stale session as fresh indefinitely. Stage the digest with the process result and persist it only after the corresponding session and exclusion writes commit successfully.

  • Remote imports hash logical paths instead of materialized filesinternal/sync/engine.go:8965, internal/sync/engine.go:8092
    Rewritten paths such as host:/remote/path are used both as cache keys and as inputs to the stat hasher. Because those logical paths do not exist locally, all sources hash as missing and later imports may appear unchanged despite changed downloaded content. Hash the physical materialized path while storing and retrieving the digest under the rewritten logical key.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 17m18s

Forward SourceSetProvider.ComputeMultiFileStatHash so the engines
factory type assertion succeeds, register hashers only when the new
MultiFileStatHash capability is declared, and stage per-component
digests on the process result with a per-row flushPending gate so
CWD-filtered or failed writes cannot mark an absent session as fresh.

Hash the physical materialized path but persist under the logical
rewritten key so remote imports observe downloaded changes. Expose a
test-only stagedProviderStatHashes counter on Engine so the new
CWD-filter regression test can prove the staging block actually ran.
@roborev-ci

roborev-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

roborev: Combined Review (49c2f2b)

Changes need revision: four medium-severity freshness and synchronization issues could leave Codebuff/Freebuff sessions stale or prevent revival.

Medium

  • internal/parser/codebuff_provider.go:351 — Codebuff leaves FingerprintHashRequiredForFreshness disabled. A same-size, sub-max-mtime companion rewrite can pass the aggregate size/mtime check despite a changed content fingerprint, leaving costs, metadata, or Freebuff classification stale. Enable fingerprint-hash freshness and add a regression test using identical file sizes and aggregate mtimes.

  • internal/sync/engine.go:745 — The freshness digest is computed after parsing and committing. If the source changes between those operations, the new digest is stored with old parsed data and future syncs may incorrectly consider the row current. Capture the digest before parsing and persist that value after a successful write, or verify it is unchanged before recording it.

  • internal/sync/engine.go:4506 — Tombstoning does not remove provider_freshness. If a byte-identical Codebuff directory is removed and later restored, its stale digest can cause the source-missing session to be skipped instead of revived. Delete the freshness row before tombstoning, map Freebuff ownership to the shared Codebuff provider key, and test byte-identical restoration.

  • internal/sync/engine.go:9247, 13560 — Missing freshness rows fall back to lossy aggregate size/mtime checks, while successful single-session syncs do not persist res.providerStatHash. Such sessions may never gain per-component freshness coverage and can miss same-size companion rewrites. Persist the staged digest after successful single-session writes and avoid aggregate-stat skipping for multi-file providers without a freshness row unless a full fingerprint validates and backfills it.


Reviewers: 2 done | Synthesis: codex, 19s | Total: 18m28s

- Forward SourceSet.MultiFileStatHash from the default codebuff
  factory so the engine hasher type-asserts.
- Stage the per-component digest at provider policy; persist after
  a successful session write and clear on tombstone; canonicalize
  freebuff to codebuff for the side-table key.
- Skip the freshness-gate cold-stamp while cwdFilter is active so
  filtered sources do not get a stale row.
@roborev-ci

roborev-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

roborev: Combined Review (66c9c07)

High-risk freshness logic can permanently suppress retries; one additional medium-risk case can miss content changes.

High

  • internal/sync/engine.go:9194 — When no freshness row exists, the current digest is persisted before fingerprinting, parsing, or session writing succeeds. A transient failure can therefore leave a matching digest, causing future normal syncs to skip the source indefinitely. The hashErr branch is also unreachable because errors return hasStored=false.
    • Fix: Check hashErr first, and persist the digest only after a successful write or after confirming an existing current session row matches the fingerprint.

Medium

  • internal/sync/engine.go:9243 — A matching multi-file stat digest skips content fingerprinting even though the digest contains only sizes and mtimes. Same-size rewrites with preserved or coarse-grained mtimes can remain undetected, leaving classification, costs, or metadata stale despite FingerprintHashRequiredForFreshness.
    • Fix: Include a reliable signal such as ctime in the digest, or verify the content hash before declaring the source fresh when stat tuples are unchanged.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 14m25s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant