Skip to content

refactor: purge internal review artifacts and harden review flows#2

Merged
Boulea7 merged 1 commit intomainfrom
refactor/purge-internal-review-artifacts
Mar 18, 2026
Merged

refactor: purge internal review artifacts and harden review flows#2
Boulea7 merged 1 commit intomainfrom
refactor/purge-internal-review-artifacts

Conversation

@Boulea7
Copy link
Owner

@Boulea7 Boulea7 commented Mar 18, 2026

Summary

This PR collects the unpublished work after the repository history cleanup.

It does three things:

  • removes internal AI/reviewer artifacts from the tracked repository surface and keeps them local-only
  • hardens reviewer-facing continuity and memory surfaces without changing their JSON contracts
  • fixes and speeds up cam audit, including real CLI --no-history behavior and a faster history scan path with legacy fallback

Key changes

  • Added compact reviewer-history helpers and command-surface follow-through for cam session and cam memory
  • Added direct unit coverage for recovery-records and memory-sync-audit
  • Added wrapper coverage for mixed sessionContinuityAutoLoad / sessionContinuityAutoSave branches
  • Added Claude-style local continuity coverage for newest-mtime session tmp selection
  • Fixed real CLI cam audit --no-history semantics and optimized history scan with batched git grep plus fallback
  • Removed tracked internal review artifacts from the repository surface and updated remaining public docs/release checklist references

Validation

  • pnpm lint
  • pnpm test
  • pnpm build
  • pnpm exec tsx src/cli.ts audit --json --no-history
  • pnpm exec tsx src/cli.ts audit --json
  • pnpm exec tsx src/cli.ts doctor --json
  • pnpm exec tsx src/cli.ts session status --json
  • pnpm exec tsx src/cli.ts memory --json --recent 5

Current observed audit result in this branch:

  • high = 0
  • medium = 0

Review notes

  • Repository history has already been rewritten so the removed internal files are no longer present in branch history.
  • main is already the cleaned baseline; this PR contains only the unpublished code/doc/test changes on top of that baseline.
  • Production push review was done with parallel reviewer agents before opening this PR; no blocking findings remained.

Summary by Sourcery

Refine continuity and memory history presentation while tightening audit history scanning and CLI history flags.

Bug Fixes:

  • Ensure cam audit correctly honors --no-history and --history flags across both direct command helpers and the real CLI interface.
  • Fix history finding line number extraction when matched text contains colon-digit-colon sequences.
  • Align session continuity and memory sync displays to avoid duplicate latest entries in compact history previews and to coalesce repeated events.

Enhancements:

  • Introduce compact history preview utilities and apply them to session continuity and memory sync text output while preserving raw JSON history surfaces.
  • Adjust session continuity status output to surface the latest rollout separately from prior generations and clarify empty or singular history cases.
  • Optimize git history scanning for audits using a batched git grep path with a safe legacy fallback and correct parsing of complex match lines.

Documentation:

  • Update English and Chinese READMEs and session continuity docs to describe the new continuity rollout and compact prior preview behavior, and to remove obsolete internal reviewer docs from public navigation.
  • Simplify the release checklist to focus on public documentation updates and remove references to internal review artifacts.

Summary by cubic

Purges internal review artifacts from the repo (now local-only via .gitignore) and hardens reviewer flows. Improves cam session/cam memory text outputs with compact, readable history while keeping JSON contracts unchanged, and fixes/speeds up cam audit history scanning.

  • Refactors

    • Added reviewer-history helper to coalesce consecutive entries; used in cam session and cam memory text UIs.
    • cam session save|load|status: shows latest rollout, plus a compact prior preview that excludes the latest and groups repeats; JSON still returns raw recent entries.
    • cam memory --recent: text output groups repeated sync events; JSON remains raw; reads extra entries to build a compact view.
    • Session continuity: prefers newest mtime for Claude-style local tmp files; docs updated to reflect “latest rollout” and “prior preview”.
    • Removed internal review docs from public surfaces and added ignores (AGENTS.md, CHANGELOG.md, and related internal guides); cleaned docs index and release checklist.
  • Bug Fixes

    • cam audit: --no-history and --history behave correctly on the CLI and helper; status line reflects the actual mode.
    • History scan is faster via batched git grep -P -z with case handling; auto-falls back to legacy walk on error; preserves accurate line numbers even with colon patterns.
    • Broader tests added for recovery records, memory sync audit, compact history grouping, wrapper auto-load/save flows, and CLI history flag behavior.

Written for commit 4252aec. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced session continuity displays with latest rollout information and compact prior generation previews.
    • Improved audit history scanning with fallback support for legacy scanning methods.
    • Better grouping and compaction of repeated sync events in memory audit output.
  • Documentation

    • Updated README with clarified descriptions of session save/load/status behavior.
    • Reorganized documentation structure with updated maintenance guidance.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 18, 2026

Reviewer's Guide

Refactors reviewer-facing continuity and memory history surfaces, tightens CLI audit history semantics and performance, and removes internal review artifacts and their public references without altering existing JSON contracts.

Sequence diagram for cam audit history resolution and scanning

sequenceDiagram
  actor User
  participant AuditCLI as AuditCLI
  participant AuditCommand as runAudit
  participant AuditScan as runAuditScan
  participant HistoryScanner as scanHistory
  participant Git as git

  User->>AuditCLI: cam audit [--history|--no-history] [--json]
  AuditCLI->>AuditCommand: runAudit(options)
  AuditCommand->>AuditCommand: resolveIncludeHistory(options)
  AuditCommand->>AuditScan: runAuditScan(cwd, includeHistory)
  alt includeHistory == false
    AuditScan-->>AuditCommand: skip history scan
  else includeHistory == true
    AuditScan->>HistoryScanner: scanHistory(cwd)
    HistoryScanner->>Git: listCommits()
    Git-->>HistoryScanner: commits[]
    alt commits.length == 0
      HistoryScanner-->>AuditScan: []
    else commits.length > 0
      HistoryScanner->>HistoryScanner: scanHistoryWithGitGrep(cwd, commits)
      alt git grep path succeeds
        HistoryScanner-->>AuditScan: findings[]
      else git grep path fails
        HistoryScanner->>HistoryScanner: scanHistoryLegacy(cwd)
        HistoryScanner-->>AuditScan: findings[]
      end
    end
  end
  AuditScan-->>AuditCommand: AuditReport
  alt options.json == true
    AuditCommand-->>AuditCLI: JSON report
  else options.json == false
    AuditCommand-->>AuditCLI: formatted text with History scan: enabled|disabled
  end
  AuditCLI-->>User: Output
Loading

Sequence diagram for cam session status continuity preview flow

sequenceDiagram
  actor User
  participant SessionCLI as SessionCLI
  participant SessionCommand as runSession
  participant ContinuityStore as SessionContinuityStore
  participant ReviewerHistory as buildCompactHistoryPreview

  User->>SessionCLI: cam session status [--json]
  SessionCLI->>SessionCommand: runSession(options)
  SessionCommand->>ContinuityStore: getLocation(project)
  SessionCommand->>ContinuityStore: getLocation(project-local)
  SessionCommand->>ContinuityStore: readState(project)
  SessionCommand->>ContinuityStore: readState(project-local)
  SessionCommand->>ContinuityStore: readRecentAuditEntries(previewReadLimit)
  ContinuityStore-->>SessionCommand: recentContinuityAuditPreviewEntries[]
  SessionCommand->>SessionCommand: recentContinuityAuditEntries = slice(0, recentContinuityAuditLimit)
  SessionCommand->>SessionCommand: latestContinuityAuditEntry = recentContinuityAuditPreviewEntries[0]
  SessionCommand->>ContinuityStore: readRecoveryRecord()
  ContinuityStore-->>SessionCommand: pendingContinuityRecovery

  alt options.json == true
    SessionCommand-->>SessionCLI: JSON including latestContinuityAuditEntry and recentContinuityAuditEntries
  else options.json == false
    SessionCommand->>ReviewerHistory: buildCompactHistoryPreview(entries, getSignature, maxGroups, excludeLeadingCount)
    ReviewerHistory-->>SessionCommand: CompactHistoryPreview
    SessionCommand->>SessionCommand: formatRecentGenerationLines(preview.groups)
    SessionCommand-->>SessionCLI: text output with Latest rollout and Recent prior generations
  end

  SessionCLI-->>User: Output
Loading

Sequence diagram for cam memory recent sync audit grouping

sequenceDiagram
  actor User
  participant MemoryCLI as MemoryCLI
  participant MemoryCommand as runMemory
  participant Runtime as RuntimeContext
  participant MemoryStore as MemoryStore
  participant ReviewerHistory as buildCompactHistoryPreview

  User->>MemoryCLI: cam memory --recent N
  MemoryCLI->>MemoryCommand: runMemory(options)
  MemoryCommand->>Runtime: buildRuntimeContext(cwd, configScope)
  Runtime-->>MemoryCommand: runtime
  MemoryCommand->>Runtime: syncService.memoryStore.readRecentSyncAuditEntries(N * 2)
  MemoryStore-->>MemoryCommand: recentSyncAuditPreviewEntries[]
  MemoryCommand->>MemoryCommand: recentSyncAudit = slice(previewEntries, 0, N)

  alt recentSyncAuditPreviewEntries.length > 0
    MemoryCommand->>ReviewerHistory: buildCompactHistoryPreview(entries, getSignature, maxGroups)
    ReviewerHistory-->>MemoryCommand: CompactHistoryPreview
    MemoryCommand->>MemoryCommand: formatRecentSyncAuditLines(preview)
    MemoryCommand-->>MemoryCLI: text output with Recent sync events (grouped)
  else no recent entries
    MemoryCommand-->>MemoryCLI: text output without recent sync events section
  end

  MemoryCLI-->>User: Output
Loading

Class diagram for reviewer history utilities and CLI consumers

classDiagram
  class CompactHistoryGroup_T {
    +T latest
    +int rawCount
  }

  class CompactHistoryPreview_T {
    +CompactHistoryGroup_T groups[]
    +int omittedRawCount
    +int totalRawCount
  }

  class BuildCompactHistoryPreviewOptions_T {
    +function getSignature(entry)
    +int maxGroups
    +int excludeLeadingCount
  }

  class ReviewerHistory {
    +buildCompactHistoryPreview(entries, options) CompactHistoryPreview_T
  }

  class SessionContinuityAuditEntry {
    +string rolloutPath
    +string sourceSessionId
    +string scope
    +string preferredPath
    +string actualPath
    +string fallbackReason
    +int codexExitCode
    +EvidenceCounts evidenceCounts
    +string[] writtenPaths
    +string generatedAt
  }

  class EvidenceCounts {
    +int successfulCommands
    +int failedCommands
    +int fileWrites
    +int nextSteps
    +int untried
  }

  class MemorySyncAuditEntry {
    +string rolloutPath
    +string sessionId
    +string status
    +string skipReason
    +bool isRecovery
    +string configuredExtractorMode
    +string configuredExtractorName
    +string actualExtractorMode
    +string actualExtractorName
    +int appliedCount
    +string[] scopesTouched
    +string resultSummary
  }

  class SessionCommandModule {
    +runSession(options) string
    +formatRecentGenerationLines(entries) string[]
  }

  class MemoryCommandModule {
    +runMemory(options) string
    +formatRecentSyncAuditLines(entries, maxGroups) string[]
  }

  SessionCommandModule ..> ReviewerHistory : uses
  MemoryCommandModule ..> ReviewerHistory : uses

  SessionCommandModule ..> SessionContinuityAuditEntry : formats
  MemoryCommandModule ..> MemorySyncAuditEntry : formats

  SessionContinuityAuditEntry o--> EvidenceCounts

  ReviewerHistory ..> CompactHistoryPreview_T : returns
  ReviewerHistory ..> BuildCompactHistoryPreviewOptions_T : consumes
  CompactHistoryPreview_T o--> CompactHistoryGroup_T
Loading

File-Level Changes

Change Details Files
Compact and clarify session continuity history surfaces while preserving JSON fields.
  • Introduce a generic buildCompactHistoryPreview helper and use it to group consecutive continuity history entries while tracking omitted counts.
  • Change session text output to distinguish latest generation and rollout from prior generations, including new 'Latest rollout' line and 'Recent prior generations' section.
  • Limit JSON continuity history to a fixed recent count while reading a wider window for text compaction, keeping latestContinuityDiagnostics and recentContinuityAuditEntries contracts intact.
  • Adjust tests and docs to reflect the new compact prior-generation behavior and wording.
src/lib/domain/reviewer-history.ts
src/lib/commands/session.ts
docs/session-continuity.md
README.md
README.en.md
test/session-command.test.ts
test/session-continuity.test.ts
test/reviewer-history.test.ts
Compact memory sync audit history in text mode while keeping JSON audit entries raw and unchanged.
  • Add syncAuditSignature and formatRecentSyncAuditLines helpers to group repeated MemorySyncAuditEntry records using buildCompactHistoryPreview.
  • Fetch a larger recent sync audit window for preview while truncating the JSON response to the requested count only.
  • Update text output to show grouped recent sync events with repeat and omission counts and align tests with the new behavior.
src/lib/commands/memory.ts
test/memory-command.test.ts
test/memory-sync-audit.test.ts
Harden audit history scanning semantics, implement real CLI --no-history behavior, and optimize history scanning with batched git grep plus legacy fallback.
  • Introduce resolveIncludeHistory to consistently resolve includeHistory based on history/noHistory flags and wire it through runAudit.
  • Add a batched git grep-based scanHistoryWithGitGrep path that parses NUL-delimited matches into AuditFinding objects, with robust parsing of colon-digit-colon content and early null on non-zero exit codes.
  • Extract a scanHistoryLegacy implementation that keeps the old per-commit scan as fallback when git grep fails.
  • Add helper utilities for building grep args, parsing result lines, chunking commits, and a historyRevisionBatchSize constant.
  • Extend tests to cover real CLI --no-history and --history behavior, colon-digit-colon line parsing, and forced fallback to legacy history walk via a vi spy on runCommandCapture.
src/lib/commands/audit.ts
src/lib/security/audit.ts
test/audit.test.ts
src/lib/util/process.ts
Clarify and expand wrapper continuity behavior across combinations of auto-load and auto-save flags.
  • Add writeWrapperMockCodex to synthesize Codex sessions and capture wrapper arguments in session tests.
  • Add coverage for all combinations of sessionContinuityAutoLoad and sessionContinuityAutoSave (both off, load-only, save-only, both on) verifying injection, auto-save, and absence of continuity artifacts when disabled.
  • Assert correct interaction between wrapper instructions, continuity store state, audit entries, and recovery records.
test/session-command.test.ts
Tighten documentation and remove tracked internal reviewer artifacts from the public surface.
  • Update bilingual docs landing pages to drop internal progress/review docs and instead point to release checklist and ClaudeCode patch audit.
  • Simplify release checklist to focus on user-facing docs and remove references to internal review packets and local AI handoff files.
  • Trim README maintainer docs and contributing sections to remove references to removed internal docs and CHANGELOG.
  • Ensure language policy sections in docs reflect the reduced set of public design docs.
docs/README.en.md
docs/README.md
docs/release-checklist.md
README.md
README.en.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link

coderabbitai bot commented Mar 18, 2026

📝 Walkthrough

Walkthrough

This pull request refactors history preview and audit rendering across session and memory commands, introducing a new generic buildCompactHistoryPreview utility for grouping consecutive audit entries. It updates command descriptions, removes deprecated documentation references, adds git grep-based history scanning with legacy fallback, and includes comprehensive test coverage for the new domain logic and command behaviors.

Changes

Cohort / File(s) Summary
Configuration & Documentation
.gitignore, README.en.md, README.md, docs/README.en.md, docs/README.md, docs/release-checklist.md, docs/session-continuity.md
Removes references to deprecated internal docs (progress-log, review-guide, reviewer-handoff); updates session/memory command descriptions to reflect "latest rollout" and "compact prior preview" semantics; reorganizes maintainer documentation structure.
Domain Logic - History Preview
src/lib/domain/reviewer-history.ts
Exports new generic interfaces CompactHistoryGroup<T> and CompactHistoryPreview<T>; implements buildCompactHistoryPreview() to batch entries by signature, coalesce consecutive duplicates, and track omitted/total raw counts for compact rendering.
Audit History Scanning
src/lib/security/audit.ts
Refactors history scanning to use git grep-based approach (scanHistoryWithGitGrep) with legacy fallback (scanHistoryLegacy); adds buildHistoryGrepArgs, parseHistoryGrepLine, and chunkArray utilities; introduces historyRevisionBatchSize batching constant.
Command Updates
src/lib/commands/audit.ts
Introduces resolveIncludeHistory() helper to centralize history flag resolution logic; updates runAudit to use resolved history value.
Memory Command
src/lib/commands/memory.ts
Adds syncAuditSignature and formatRecentSyncAuditLines helpers; integrates buildCompactHistoryPreview to render grouped recent sync events with omission indicators; reads double the entries then slices to configured limit.
Session Command
src/lib/commands/session.ts
Replaces simple slice-based preview with buildCompactHistoryPreview for generation entries; adds latest rollout path tracking in status outputs; renames display label to "Recent prior generations"; implements two-step read (10 entries, then slice to 5) for preview consistency.
Audit Tests
test/audit.test.ts
Adds CLI test utilities (runCli, sourceCliPath, tsxBinaryPath); introduces tests for history toggling via direct helper and CLI surface; adds git grep fallback failure scenario test with process mocking.
Memory Command Tests
test/memory-command.test.ts
Adds two tests validating grouped sync event rendering in text output (e.g., "Recent sync events (2 grouped)") while preserving raw JSON output; verifies omission and repetition indicators.
Memory Sync Audit Tests
test/memory-sync-audit.test.ts
New test file exercising memory-sync-audit domain: tests parseMemorySyncAuditEntry, buildMemorySyncAuditEntry, and formatMemorySyncAuditEntry with various extractor and skip-reason scenarios.
Recovery Records Tests
test/recovery-records.test.ts
New test file validating sync and continuity recovery record builders and matchers; covers positive validation, negative shape rejection, and identity-field matching.
Reviewer History Tests
test/reviewer-history.test.ts
New test file for buildCompactHistoryPreview; validates grouping by signature, leading-count exclusion, max-groups enforcement, and omitted/total raw count calculations.
Session Command Tests
test/session-command.test.ts
Extensive additions: new writeWrapperMockCodex helper; tests for save/load/status/clear continuity flows with rollout tracking; wrapped Codex continuity injection/recovery scenarios; session deduplication and coalescing behavior; updates to "Recent prior generations" label.
Session Continuity Tests
test/session-continuity.test.ts
Updates for newest-mtime selection logic; adds newer session tmp file and corresponding timestamp assertions; verifies cleanup of both older and newer files.

Sequence Diagram

sequenceDiagram
    actor User
    participant Session Command
    participant buildCompactHistoryPreview
    participant Memory/Domain
    participant Audit History
    
    User->>Session Command: runSession (load/status)
    Session Command->>Audit History: read recentContinuityPreviewReadLimit (10)
    Audit History-->>Session Command: raw recent entries
    Session Command->>buildCompactHistoryPreview: entries + getSignature callback
    buildCompactHistoryPreview->>buildCompactHistoryPreview: batch by signature
    buildCompactHistoryPreview->>buildCompactHistoryPreview: coalesce consecutive duplicates
    buildCompactHistoryPreview->>buildCompactHistoryPreview: calculate omitted/total counts
    buildCompactHistoryPreview-->>Session Command: CompactHistoryPreview{groups, omittedRawCount}
    Session Command->>Memory/Domain: extract latest rollout, generation details
    Session Command-->>User: status output (Recent prior generations + latest rollout)
    
    User->>Session Command: runSession (save)
    Session Command->>Audit History: write new continuity entry
    Session Command->>buildCompactHistoryPreview: render compact preview
    buildCompactHistoryPreview-->>Session Command: grouped preview with omission notice
    Session Command-->>User: saved with rollout path
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • fix: harden sync recovery and reviewer contracts #1: Adds isRecovery field and related handling to MemorySyncAuditEntry and memory-sync-audit logic, complementing the main PR's introduction of grouped sync event rendering and MemorySyncAuditEntry integration in the memory command surface.

Poem

🐰 A rabbit's ode to history's embrace:

Grouped entries dance in compact grace,
Rollout paths trace their rightful place,
Where dupes collapse and previews shine—
No more the sprawl, just essence fine!
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: removing internal review artifacts and hardening review flows, which aligns with the core objectives described in the PR.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering summary, key changes, validation steps, and review notes. It addresses all major template sections including Claude parity implications and risks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/purge-internal-review-artifacts
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

Migrating from UI to YAML configuration.

Use the @coderabbitai configuration command in a PR comment to get a dump of all your UI settings in YAML format. You can then edit this YAML file and upload it to the root of your repository to configure CodeRabbit programmatically.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In memory-command.test.ts, the test that overwrites process.env.HOME doesn’t restore the original value, which can leak into other tests; capture and reset HOME in beforeEach/afterEach or inside the test.
  • In audit.ts, scanHistory calls listCommits and then, on git grep failure, scanHistoryLegacy calls listCommits again; consider passing the already-fetched commit list into the legacy path to avoid double work on large histories.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `memory-command.test.ts`, the test that overwrites `process.env.HOME` doesn’t restore the original value, which can leak into other tests; capture and reset `HOME` in `beforeEach`/`afterEach` or inside the test.
- In `audit.ts`, `scanHistory` calls `listCommits` and then, on `git grep` failure, `scanHistoryLegacy` calls `listCommits` again; consider passing the already-fetched commit list into the legacy path to avoid double work on large histories.

## Individual Comments

### Comment 1
<location path="src/lib/security/audit.ts" line_range="259-261" />
<code_context>
+  );
+}
+
+function chunkArray<T>(items: T[], size: number): T[][] {
+  const chunks: T[][] = [];
+  for (let index = 0; index < items.length; index += size) {
+    chunks.push(items.slice(index, index + size));
+  }
</code_context>
<issue_to_address>
**issue:** Guard against a zero or negative `size` in `chunkArray` to avoid a potential infinite loop.

Because `historyRevisionBatchSize` is hard-coded and positive this works today, but `chunkArray` is a generic helper and may be reused with other inputs. If `size` is 0 or negative, `index += size` will never progress and the loop will not terminate. Consider guarding with `if (size <= 0) throw` or returning `[items]` to make this utility safe for future callers.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +259 to +261
function chunkArray<T>(items: T[], size: number): T[][] {
const chunks: T[][] = [];
for (let index = 0; index < items.length; index += size) {
Copy link

Choose a reason for hiding this comment

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

issue: Guard against a zero or negative size in chunkArray to avoid a potential infinite loop.

Because historyRevisionBatchSize is hard-coded and positive this works today, but chunkArray is a generic helper and may be reused with other inputs. If size is 0 or negative, index += size will never progress and the loop will not terminate. Consider guarding with if (size <= 0) throw or returning [items] to make this utility safe for future callers.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

4 issues found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/release-checklist.md">

<violation number="1" location="docs/release-checklist.md:35">
P2: The renamed section removes the CHANGELOG update checklist item, but the "Release decision" section at the bottom of this file still gates releases on "changelog is updated" and "review artifacts are in place." Either add a CHANGELOG bullet back under this section or update the release decision criteria to match.</violation>
</file>

<file name="src/lib/security/audit.ts">

<violation number="1" location="src/lib/security/audit.ts:162">
P2: `scanHistoryLegacy` calls `listCommits` again despite the caller (`scanHistory`) having already fetched the commits. Pass `commits` as a parameter to avoid the redundant `git rev-list --all` invocation on the fallback path.</violation>

<violation number="2" location="src/lib/security/audit.ts:259">
P2: Guard against `size <= 0` in `chunkArray` to prevent an infinite loop. Currently safe because the only caller passes a hard-coded positive constant, but the function signature accepts any number and a zero or negative value will hang the process.</violation>
</file>

<file name=".gitignore">

<violation number="1" location=".gitignore:30">
P2: `CHANGELOG.md` is a standard project-level file name that most release tooling and contributors expect to be tracked. The other entries in this block are clearly internal artifacts, but gitignoring `CHANGELOG.md` will silently prevent any future public changelog from being committed. Consider using a more specific name for the internal artifact (e.g., `AI_CHANGELOG.md`) or removing this entry now that the internal file is already purged from history.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- `cam doctor`

## Review packet checks
## Documentation checks
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 18, 2026

Choose a reason for hiding this comment

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

P2: The renamed section removes the CHANGELOG update checklist item, but the "Release decision" section at the bottom of this file still gates releases on "changelog is updated" and "review artifacts are in place." Either add a CHANGELOG bullet back under this section or update the release decision criteria to match.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/release-checklist.md, line 35:

<comment>The renamed section removes the CHANGELOG update checklist item, but the "Release decision" section at the bottom of this file still gates releases on "changelog is updated" and "review artifacts are in place." Either add a CHANGELOG bullet back under this section or update the release decision criteria to match.</comment>

<file context>
@@ -32,15 +32,10 @@ Use this checklist before cutting any alpha or beta release of `codex-auto-memor
   - `cam doctor`
 
-## Review packet checks
+## Documentation checks
 
-- Update `CHANGELOG.md` with the new milestone and commit hash.
</file context>
Fix with Cubic

return grepFindings;
}

return scanHistoryLegacy(cwd);
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 18, 2026

Choose a reason for hiding this comment

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

P2: scanHistoryLegacy calls listCommits again despite the caller (scanHistory) having already fetched the commits. Pass commits as a parameter to avoid the redundant git rev-list --all invocation on the fallback path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/security/audit.ts, line 162:

<comment>`scanHistoryLegacy` calls `listCommits` again despite the caller (`scanHistory`) having already fetched the commits. Pass `commits` as a parameter to avoid the redundant `git rev-list --all` invocation on the fallback path.</comment>

<file context>
@@ -148,6 +149,50 @@ async function scanWorkingTree(cwd: string): Promise<AuditFinding[]> {
+    return grepFindings;
+  }
+
+  return scanHistoryLegacy(cwd);
+}
+
</file context>
Fix with Cubic

);
}

function chunkArray<T>(items: T[], size: number): T[][] {
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 18, 2026

Choose a reason for hiding this comment

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

P2: Guard against size <= 0 in chunkArray to prevent an infinite loop. Currently safe because the only caller passes a hard-coded positive constant, but the function signature accepts any number and a zero or negative value will hang the process.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/security/audit.ts, line 259:

<comment>Guard against `size <= 0` in `chunkArray` to prevent an infinite loop. Currently safe because the only caller passes a hard-coded positive constant, but the function signature accepts any number and a zero or negative value will hang the process.</comment>

<file context>
@@ -168,6 +213,57 @@ async function scanHistory(cwd: string): Promise<AuditFinding[]> {
+  );
+}
+
+function chunkArray<T>(items: T[], size: number): T[][] {
+  const chunks: T[][] = [];
+  for (let index = 0; index < items.length; index += size) {
</file context>
Suggested change
function chunkArray<T>(items: T[], size: number): T[][] {
function chunkArray<T>(items: T[], size: number): T[][] {
if (size <= 0) {
throw new RangeError(`chunkArray size must be positive, got ${size}`);
}
Fix with Cubic


# Internal AI / review artifacts should stay local-only
AGENTS.md
CHANGELOG.md
Copy link

@cubic-dev-ai cubic-dev-ai bot Mar 18, 2026

Choose a reason for hiding this comment

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

P2: CHANGELOG.md is a standard project-level file name that most release tooling and contributors expect to be tracked. The other entries in this block are clearly internal artifacts, but gitignoring CHANGELOG.md will silently prevent any future public changelog from being committed. Consider using a more specific name for the internal artifact (e.g., AI_CHANGELOG.md) or removing this entry now that the internal file is already purged from history.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .gitignore, line 30:

<comment>`CHANGELOG.md` is a standard project-level file name that most release tooling and contributors expect to be tracked. The other entries in this block are clearly internal artifacts, but gitignoring `CHANGELOG.md` will silently prevent any future public changelog from being committed. Consider using a more specific name for the internal artifact (e.g., `AI_CHANGELOG.md`) or removing this entry now that the internal file is already purged from history.</comment>

<file context>
@@ -25,6 +25,14 @@ CLAUDE.md
 
+# Internal AI / review artifacts should stay local-only
+AGENTS.md
+CHANGELOG.md
+docs/progress-log.md
+docs/review-guide.md
</file context>
Fix with Cubic

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
test/recovery-records.test.ts (1)

54-88: Optional: Consider adding a test case for undefined sessionId matching.

The isSyncRecoveryRecord guard allows sessionId to be undefined, and matchesSyncRecoveryRecord compares it via strict equality. A test case verifying that a record with sessionId: undefined matches an identity with sessionId: undefined (and doesn't match one with a defined sessionId) would strengthen edge-case coverage.

💡 Example test case
it("matches sync recovery records when sessionId is undefined in both", () => {
  const record = buildSyncRecoveryRecord({
    projectId: "project-1",
    worktreeId: "worktree-1",
    rolloutPath: "/tmp/rollout.jsonl",
    sessionId: undefined,
    configuredExtractorMode: "heuristic",
    configuredExtractorName: "heuristic",
    actualExtractorMode: "heuristic",
    actualExtractorName: "heuristic",
    status: "no-op",
    appliedCount: 0,
    scopesTouched: [],
    failedStage: "audit-write",
    failureMessage: "",
    auditEntryWritten: false
  });

  expect(
    matchesSyncRecoveryRecord(record, {
      projectId: "project-1",
      worktreeId: "worktree-1",
      rolloutPath: "/tmp/rollout.jsonl",
      sessionId: undefined
    })
  ).toBe(true);

  expect(
    matchesSyncRecoveryRecord(record, {
      projectId: "project-1",
      worktreeId: "worktree-1",
      rolloutPath: "/tmp/rollout.jsonl",
      sessionId: "session-1"
    })
  ).toBe(false);
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/recovery-records.test.ts` around lines 54 - 88, Add a test that covers
the undefined sessionId edge case: create a record via buildSyncRecoveryRecord
with sessionId: undefined (and same required identity fields) and assert
matchesSyncRecoveryRecord(record, { projectId, worktreeId, rolloutPath,
sessionId: undefined }) returns true and that matchesSyncRecoveryRecord returns
false when given a defined sessionId (e.g., "session-1"); this validates
behavior permitted by isSyncRecoveryRecord and ensures strict equality on
sessionId is exercised.
src/lib/commands/audit.ts (1)

11-21: Consider handling conflicting history/noHistory flags explicitly.

Right now precedence is implicit. A small guard (or parser-level mutual exclusivity) would make CLI behavior clearer when both flags are provided.

Possible implementation
 function resolveIncludeHistory(options: AuditCommandOptions): boolean {
+  if (typeof options.history === "boolean" && options.noHistory === true) {
+    throw new Error("Cannot use both --history and --no-history.");
+  }
+
   if (typeof options.history === "boolean") {
     return options.history;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/commands/audit.ts` around lines 11 - 21, resolveIncludeHistory
currently uses implicit precedence between options.history and
options.noHistory; update it to explicitly detect the conflict when both flags
are set (e.g., options.history !== undefined && options.noHistory === true or
both true) and fail fast with a clear error message (or throw a CLI-specific
error) instead of silently choosing one; modify the resolveIncludeHistory
function (and AuditCommandOptions handling) to check for both flags and
throw/report a mutually-exclusive flags error, otherwise proceed with the
existing boolean resolution logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/release-checklist.md`:
- Around line 35-39: The "Documentation checks" checklist underspecifies
release-gate prerequisites; add a concrete prep step under the "## Documentation
checks" heading that instructs maintainers to (1) prepare the changelog and
review artifacts (e.g., ensure a drafted CHANGELOG entry and linked PR/review
notes are ready) and (2) perform paired bilingual public-doc verification of
docs/README.md and docs/README.en.md using companion-first wording (confirm
public reading paths and companion-focused phrasing). Update the checklist text
to explicitly require both the changelog/review artifacts be prepared before
release and the paired bilingual review be completed.

---

Nitpick comments:
In `@src/lib/commands/audit.ts`:
- Around line 11-21: resolveIncludeHistory currently uses implicit precedence
between options.history and options.noHistory; update it to explicitly detect
the conflict when both flags are set (e.g., options.history !== undefined &&
options.noHistory === true or both true) and fail fast with a clear error
message (or throw a CLI-specific error) instead of silently choosing one; modify
the resolveIncludeHistory function (and AuditCommandOptions handling) to check
for both flags and throw/report a mutually-exclusive flags error, otherwise
proceed with the existing boolean resolution logic.

In `@test/recovery-records.test.ts`:
- Around line 54-88: Add a test that covers the undefined sessionId edge case:
create a record via buildSyncRecoveryRecord with sessionId: undefined (and same
required identity fields) and assert matchesSyncRecoveryRecord(record, {
projectId, worktreeId, rolloutPath, sessionId: undefined }) returns true and
that matchesSyncRecoveryRecord returns false when given a defined sessionId
(e.g., "session-1"); this validates behavior permitted by isSyncRecoveryRecord
and ensures strict equality on sessionId is exercised.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bd46496-223e-4ec1-b0e1-3c9c31f0c333

📥 Commits

Reviewing files that changed from the base of the PR and between ca576e3 and 4252aec.

📒 Files selected for processing (19)
  • .gitignore
  • README.en.md
  • README.md
  • docs/README.en.md
  • docs/README.md
  • docs/release-checklist.md
  • docs/session-continuity.md
  • src/lib/commands/audit.ts
  • src/lib/commands/memory.ts
  • src/lib/commands/session.ts
  • src/lib/domain/reviewer-history.ts
  • src/lib/security/audit.ts
  • test/audit.test.ts
  • test/memory-command.test.ts
  • test/memory-sync-audit.test.ts
  • test/recovery-records.test.ts
  • test/reviewer-history.test.ts
  • test/session-command.test.ts
  • test/session-continuity.test.ts

Comment on lines +35 to 39
## Documentation checks

- Update `CHANGELOG.md` with the new milestone and commit hash.
- Update `docs/progress-log.md` to reflect the current phase and remaining gaps.
- Update `docs/review-guide.md` if a new high-risk area or review order is introduced.
- Update `docs/reviewer-handoff.md` so external review tools can pick up the current state quickly.
- Update the bilingual docs entry pages (`docs/README.md` and `docs/README.en.md`) if the public reading path changed.
- Re-check the current official Codex and Claude public docs before changing migration wording; if the public posture is unchanged, say so explicitly in the handoff.
- Refresh the local ignored AI handoff file `AI_REVIEW.local.md` with current review/test instructions before handing off to another agent.
- Ensure the latest milestone commit is focused enough to review independently.
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Documentation checks now underspecify release-gate prerequisites.

The checklist still gates release on changelog/review artifacts later, but this section no longer tells maintainers to prepare them explicitly. Please add a direct prep step here to avoid release-time ambiguity.

Suggested patch
 ## Documentation checks
 
 - Update the bilingual docs entry pages (`docs/README.md` and `docs/README.en.md`) if the public reading path changed.
 - Re-check the current official Codex and Claude public docs before changing migration wording; if the public posture is unchanged, say so explicitly in the handoff.
+- Confirm where release notes/changelog updates are recorded for this milestone and update them before tagging.
+- Confirm required reviewer artifacts for this milestone are present at their current canonical location.
 - Ensure the latest milestone commit is focused enough to review independently.

Based on learnings: "Keep release hygiene explicit about paired bilingual public-doc checks and companion-first wording".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Documentation checks
- Update `CHANGELOG.md` with the new milestone and commit hash.
- Update `docs/progress-log.md` to reflect the current phase and remaining gaps.
- Update `docs/review-guide.md` if a new high-risk area or review order is introduced.
- Update `docs/reviewer-handoff.md` so external review tools can pick up the current state quickly.
- Update the bilingual docs entry pages (`docs/README.md` and `docs/README.en.md`) if the public reading path changed.
- Re-check the current official Codex and Claude public docs before changing migration wording; if the public posture is unchanged, say so explicitly in the handoff.
- Refresh the local ignored AI handoff file `AI_REVIEW.local.md` with current review/test instructions before handing off to another agent.
- Ensure the latest milestone commit is focused enough to review independently.
## Documentation checks
- Update the bilingual docs entry pages (`docs/README.md` and `docs/README.en.md`) if the public reading path changed.
- Re-check the current official Codex and Claude public docs before changing migration wording; if the public posture is unchanged, say so explicitly in the handoff.
- Confirm where release notes/changelog updates are recorded for this milestone and update them before tagging.
- Confirm required reviewer artifacts for this milestone are present at their current canonical location.
- Ensure the latest milestone commit is focused enough to review independently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/release-checklist.md` around lines 35 - 39, The "Documentation checks"
checklist underspecifies release-gate prerequisites; add a concrete prep step
under the "## Documentation checks" heading that instructs maintainers to (1)
prepare the changelog and review artifacts (e.g., ensure a drafted CHANGELOG
entry and linked PR/review notes are ready) and (2) perform paired bilingual
public-doc verification of docs/README.md and docs/README.en.md using
companion-first wording (confirm public reading paths and companion-focused
phrasing). Update the checklist text to explicitly require both the
changelog/review artifacts be prepared before release and the paired bilingual
review be completed.

@Boulea7 Boulea7 merged commit 532aff1 into main Mar 18, 2026
5 checks passed
@Boulea7 Boulea7 deleted the refactor/purge-internal-review-artifacts branch March 23, 2026 15:43
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