Skip to content

feat(parser): add aider session parser - #740

Merged
wesm merged 10 commits into
kenn-io:mainfrom
youdie006:feat/aider-parser
Jun 19, 2026
Merged

feat(parser): add aider session parser#740
wesm merged 10 commits into
kenn-io:mainfrom
youdie006:feat/aider-parser

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

Summary

Adds an aider session parser. aider stores per-repo chat history as Markdown at <repo>/.aider.chat.history.md, where each file accumulates multiple # aider chat started at ... runs.

What it does

  • Splits a history file into one session per run, using the existing virtual-path fan-out (<historyFile>#<runIdx>) already used by the Shelley and Zed parsers, so a physical file maps to multiple sessions rather than one flattened blob.
  • Parses #### user prompts, assistant prose, and > tool/edit lines. aider has no tool role in agentsview, so tool/edit lines surface as assistant transcript content (the > Applied edit to ... lines are preserved verbatim), mirroring the gptme parser.
  • Discovers history files with a bounded, time-budgeted (2s) rootless walk under a configurable root — default $HOME, override via AIDER_DIR / aider_dirs — with a skip-set and a depth cap. The agent uses a shallow file watch so it does not recursively watch the whole home directory; new runs are picked up by the periodic sync.

Identity and limitations

  • Session IDs derive from the history-file path, the run's header timestamp, and an ordinal among same-header runs, so appends and different-header edits never re-key existing sessions. Residual: runs that share a byte-identical header timestamp (same repo, same second) disambiguate by position, so removing an earlier same-header run re-keys its later same-header siblings — rare given aider's 1-second header resolution, and pinned by a test.
  • Per the multi-session-per-file model (same as Shelley/Zed), a run removed from a still-present file is reconciled on the next full resync rather than incrementally.

Where to look

internal/parser/aider.go (parser + discovery), internal/sync/engine.go (processAider fan-out and the virtual-path stat resolution that lets a single session re-sync via the live watcher), internal/parser/types.go (registry entry, shallow watch).

aider stores per-repo chat history as Markdown at
<repo>/.aider.chat.history.md, accumulating multiple "# aider chat started
at" runs per file. Add a parser that splits a history file into one session
per run (via the existing virtual-path fan-out used by Shelley and Zed),
parsing the #### user prompts, assistant prose, and > tool/edit lines.

Discovery is a bounded, time-budgeted rootless walk under a configurable root
(default $HOME, AIDER_DIR to override); the agent uses a shallow file watch so
it does not recursively watch the whole home directory. Session IDs derive
from the history-file path, the run header timestamp, and an ordinal among
same-header runs, so appends and different-header edits never re-key existing
sessions. processFile resolves the aider virtual path for stat so a single
session re-syncs through the live watcher.
@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (7b745c5)

Medium severity issues found; no Critical or High findings.

Medium

  • Location: internal/parser/aider.go:273, internal/sync/engine.go:7790
    Problem: Aider session IDs are intended to remain stable when earlier runs are removed, but stored File.Path values are positional (history#idx). FindSourceFile returns the stored virtual path while the physical history file exists, so SyncSingleSession can parse the wrong run after an insertion/removal before the session, or silently no-op if the old index no longer exists.
    Fix: For Aider, resolve by raw session ID via FindAiderSourceFile before trusting the stored virtual path, or validate that the stored virtual path still recomputes to the requested raw ID before returning it.

  • Location: internal/parser/aider.go:565
    Problem: The depth cap is off by one. Directories at aiderMaxWalkDepth are skipped before their .aider.chat.history.md files are visited, so repos exactly four levels under the configured root are not discovered despite the documented four-level scan.
    Fix: Only skip when depth > aiderMaxWalkDepth, or otherwise allow visiting files in directories at the maximum depth while preventing descent below them.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 3m54s), codex_security (codex/security, done, 14s) | Total: 4m16s

Addresses automated review feedback:

- The stored "<historyFile>#<idx>" path is positional, so a single-session
  re-sync could parse the wrong run after an earlier run was inserted or
  removed. The source-path lookup now validates that the stored index still
  recomputes to the requested raw ID (AiderRawIDAt) and otherwise re-resolves
  by raw ID via FindAiderSourceFile.
- The discovery depth cap was off by one: a directory exactly
  aiderMaxWalkDepth levels under the root was skipped before its history
  files were visited. Skip only below the cap so the documented N-level scan
  reaches files at the maximum depth.
@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (2c77ea7)

Summary verdict: Two medium findings remain; no high or critical issues were reported.

Medium

  • internal/sync/engine.go:5706 - The skip check uses the physical .aider.chat.history.md path, but Aider sessions are stored with virtual file_path values like <history>#<idx>, so GetFileInfoByPath(file.Path) will not match successfully indexed Aider sessions. Unchanged Aider histories may be reparsed, rehashed, and rewritten on every full or periodic SyncAll.

    • Suggested fix: Add Aider-specific skip logic keyed by per-run virtual paths, or store/check a physical-file sentinel. Cover with an engine-level sync test.
  • internal/parser/aider.go:147 - splitAiderRuns repeatedly concatenates strings into cur.body inside the line loop, which can make large Aider logs quadratic in copy cost.

    • Suggested fix: Accumulate run body lines with strings.Builder or a slice, then materialize once per run.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 6m55s), codex_security (codex/security, done, 1m47s) | Total: 8m49s

Addresses roborev review findings:

- splitAiderRuns accumulated each run body via repeated string concatenation
  (quadratic on large history files); build the body with strings.Builder and
  materialize once per run.
- processAider skipped via the physical history-file path, but aider sessions
  are stored under virtual "<history>#<idx>" paths, so the generic lookup
  never matched and an unchanged history was re-parsed/re-hashed/re-written on
  every sync. Add aiderFileUnchanged, which checks the per-run virtual paths'
  stored mtime at the current data version (mirroring kiroSQLitePendingSessionIDs).
@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (642dd46)

High-level verdict: changes need fixes before merge due to remote sync and Aider indexing correctness regressions.

High

  • internal/parser/types.go:630
    Aider's default directory resolves to $HOME, and the SSH remote resolver tars every file-based default directory wholesale. With no remote AIDER_DIR, enabling this agent makes remote sync archive and download the entire home directory before bounded Aider discovery can run.
    Fix: Do not include Aider's $HOME default in the generic SSH directory tar path; either require an explicit remote AIDER_DIR or add Aider-specific remote discovery that transfers only matching .aider.chat.history.md files.

Medium

  • internal/parser/aider.go:456
    Aider session IDs hash filepath.Abs(path). During SSH sync the path is under a random local temp extraction directory, so the same remote Aider run gets a different aider: ID on each sync; the later remote path rewrite only changes file_path, not the already-derived ID.
    Fix: Derive Aider raw IDs from a canonical source identity, such as the rewritten remote physical history path, by passing an identity path into the parser or recomputing before write.

  • internal/sync/engine.go:5698
    aiderFileUnchanged skips a whole multi-run history file when any one virtual run row has the current mtime/data version. If a previous batch partially wrote, or one run was resynced after a data-version bump while other runs are stale or missing, future syncs will skip the file and never repair the remaining sessions.
    Fix: Only skip when all expected run rows are known current, or record a physical-file sync marker only after every run from that file is written successfully.


Panel: ci_default_security | Synthesis: codex, 11s | Members: codex_default (codex/default, done, 5m41s), codex_security (codex/security, done, 2m19s) | Total: 8m11s

Addresses roborev review findings:

- Remote SSH sync tarred the entire home directory because aider's default dir
  resolves to $HOME. The resolve script now emits aider only when AIDER_DIR is
  explicitly set (and rejects AIDER_DIR=$HOME), so an unscoped aider agent no
  longer tars the remote home. Local discovery is unchanged.
- Aider session IDs hashed the on-disk path, a random temp dir during SSH
  extraction, so the same remote run got a new ID each sync. Thread a canonical
  identity path (the rewritten remote path) into the ID hash; local sync (no
  rewriter) is byte-for-byte unchanged.
- aiderFileUnchanged skipped a file when ANY run row was current; it now skips
  only when every content-bearing run row is current (HasMessages marks which
  runs produce sessions), so a partial write no longer strands the rest.
@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (14c7dfe)

Medium findings need fixes before merge.

Medium

  • internal/sync/engine.go:5766
    Aider's custom unchanged check still runs when e.forceParse is set, so parse-diff can skip already-synced Aider files instead of reparsing them. Gate the check with !e.forceParse and add parse-diff coverage for an unchanged Aider file.

  • internal/sync/parsediff.go:343
    stripVirtualSourceSuffix does not handle Aider virtual paths, so parse-diff cannot map stored <history>#<idx> rows back to the physical history file for missing-run or parse-error reporting. Add a parser.ParseAiderVirtualPath case that returns the physical history path.

  • internal/sync/engine.go:5715
    aiderFileUnchanged ignores stored file_size, so a same-mtime append/truncate can be skipped and leave new or removed runs unsynced. Compare the stored size from GetFileInfoByPath with info.Size() alongside mtime and data version.


Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 8m16s), codex_security (codex/security, done, 2m29s) | Total: 10m52s

Addresses roborev review findings:
- gate aiderFileUnchanged behind !forceParse so a forced parse-diff reparses
  already-synced aider files instead of skipping them.
- handle aider <history>#<idx> virtual paths in stripVirtualSourceSuffix so
  parse-diff maps run rows back to the physical history file.
- compare stored file_size alongside mtime/data-version in aiderFileUnchanged,
  so a same-mtime append/truncate is re-parsed instead of skipped.
@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (f3e88b6)

Summary verdict: One medium issue remains; no high or critical findings were reported.

Medium

  • internal/sync/engine.go:5717 - aiderFileUnchanged looks up stored rows using the temp/local virtual path from ListAiderRunMetas. During SSH sync, stored file_path values are rewritten to host:/remote/path#idx, so unchanged remote Aider files never match and get fully reparsed and rewritten on every sync.

    Fix: Apply e.pathRewriter to each m.VirtualPath before GetFileInfoByPath and GetDataVersionByPath, matching the Shelley remote-skip pattern.


Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 6m20s), codex_security (codex/security, done, 2m50s) | Total: 9m16s

@wesm

wesm commented Jun 19, 2026

Copy link
Copy Markdown
Member

working on this now

@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (c42036c)

Aider support has two medium issues around shared-history scoping; no security findings were reported.

Medium

  • cmd/agentsview/session_export.go:81
    Raw source export for an Aider session resolves <history>#<idx> to the physical .aider.chat.history.md file and exports the entire multi-run repo history, not just the selected session/run.
    Fix: Add an Aider-specific branch before ResolveSourceFilePath that exports only the indexed run from the virtual path, similar to the Visual Studio Copilot shared-file special case.

  • internal/ssh/resolve.go:43
    Remote Aider sync with AIDER_DIR emits the scoped directory as a tar target, so the transfer archives the whole code root/repo tree even though only .aider.chat.history.md files are needed.
    Fix: Make remote Aider resolution transfer only discovered .aider.chat.history.md files, or add an Aider-specific remote discovery/transfer path that preserves canonical identity paths without tarring the full source directory.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 8m26s), codex_security (codex/security, done, 3m19s) | Total: 11m53s

@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (95d652e)

Medium security issue found; low-severity findings omitted per instructions.

Medium

  • internal/ssh/resolve.go:37: buildAiderResolveSnippet emits remote Aider paths using newline-delimited echo "aider:$av_entry", and parseResolvedDirs treats each stdout line as an authoritative transfer target. A malicious repository under scoped AIDER_DIR could create a path containing embedded newlines, injecting an additional target such as aider:/home/victim. That injected path can then reach buildTarCommand, bypassing the guard against resolving/tarring the whole remote home directory and allowing out-of-scope files to be transferred and potentially imported.

    Remediation: Avoid line-delimited text for remote path records. Use an unambiguous encoding such as NUL-delimited records, base64/JSON strings, or length-prefixed records. Also validate parsed Aider targets before tarring: they should be regular .aider.chat.history.md files discovered under the scoped root, not arbitrary injected directories or paths containing record separators.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 13m4s), codex_security (codex/security, done, 4m2s) | Total: 17m14s

@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (615b1ec)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 9m15s), codex_security (codex/security, done, 4m3s) | Total: 13m18s

@roborev-ci

roborev-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown

roborev: Combined Review (6f5674b)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 11m5s), codex_security (codex/security, done, 4m54s) | Total: 15m59s

@wesm
wesm merged commit f734c69 into kenn-io:main Jun 19, 2026
10 checks passed
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.

2 participants