Skip to content

Bugfix/git - #3

Open
jmikedupont2 wants to merge 3 commits into
mainfrom
bugfix/git
Open

Bugfix/git#3
jmikedupont2 wants to merge 3 commits into
mainfrom
bugfix/git

Conversation

@jmikedupont2

@jmikedupont2 jmikedupont2 commented Jul 10, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Versioning output now reports the short commit identifier for successfully versioned projects.
    • Project versioning uses metadata timestamps for commit dates when available.
  • Bug Fixes

    • Improved handling of failed, skipped, or unchanged repositories during versioning.
    • Prevented generated archives and nested output files from being included in synchronized project repositories.
    • Added automatic exclusion rules for generated files in versioned projects.
    • Improved compatibility with projects containing unexpected file paths or timestamp formats.

mike dupont and others added 3 commits July 9, 2026 21:21
When cmd_fetch downloads new projects, it now creates the git repos in
config.git_base/git-versions/ instead of directly in config.git_base.

This prevents conflicts with existing files and git repos that may already
be in config.git_base (which is set to the project root directory when
running from /mnt/data1/time-2026/05-may/07/arist).

Each project still gets its own git repo, now properly isolated in the
git-versions subdirectory.

Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
When cmd_fetch downloads new projects, it now creates the git repos in
config.git_base/git-versions/ instead of directly in config.git_base.

Additionally:
- Added .gitignore to exclude .tar.gz files from git repos
- Skip nested _aristotle directories during file copy to avoid conflicts
- Each project still gets its own git repo, now properly isolated in the
git-versions subdirectory with proper ignores for tarballs.

Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Git versioning now uses a dedicated git-versions root, preserves parsed metadata dates in commits, filters nested generated files during synchronization, returns short commit hashes, and reports committed project revisions.

Changes

Git versioning updates

Layer / File(s) Summary
Version metadata and commit message inputs
src/version.rs
Metadata timestamps are converted to git-compatible dates with fallbacks, and commit messages now use the full description.
Project repository synchronization
src/version.rs
Git initialization and synchronization skip tarballs and nested Aristotle outputs, create a .gitignore when absent, and return skipped results consistently.
Commit results and versioning wiring
src/version.rs, src/fetch.rs
Commits write messages through .git/COMMIT_EDITMSG, return short hashes, report project revisions, and use the git-versions directory from fetch.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant cmd_fetch
  participant cmd_version
  participant commit_project
  participant Git
  cmd_fetch->>cmd_version: invoke with git-versions root
  cmd_version->>commit_project: synchronize and commit project
  commit_project->>Git: stage and commit with metadata date
  Git-->>commit_project: return short HEAD hash
  commit_project-->>cmd_version: return short hash or skip
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is too vague and does not clearly describe the git/versioning bugfixes in this change. Rename it to something specific like "Fix git versioning and commit handling in version sync".
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 bugfix/git

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/version.rs`:
- Around line 77-79: Update commit_project so git init, add, commit, and hash
lookup failures are propagated as errors instead of returning Ok(None) or being
treated as skips; use the existing command-handling logic around commit_project
and cmd_version. Ensure cmd_version aggregates failed repositories and returns
an error when any repository operation fails, rather than reporting them as
skipped or committed.
- Around line 98-104: Before staging in the version-generation flow, remove
existing matching tarballs and nested `_aristotle` project files/directories
from `repo`, not just during copying. Update the `.gitignore` handling in the
relevant `src/version.rs` logic to merge and persist the required ignore rules
even when the file already exists, then ensure cleanup occurs before `git add
-A`.
- Around line 29-39: The fallback parsing in the git date logic can panic on
short extracted_at values. In the parse_from_rfc3339 error branch within the
git_date initialization, use extracted_at.get(..19) and only call
NaiveDateTime::parse_from_str when that slice exists; otherwise return
extracted_at.to_string() as the fallback.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 378164ae-64c8-4798-bfe4-4129ed26a76b

📥 Commits

Reviewing files that changed from the base of the PR and between 2fe30d4 and 8c99553.

📒 Files selected for processing (2)
  • src/fetch.rs
  • src/version.rs

Comment thread src/version.rs
Comment on lines +29 to +39
let git_date = if extracted_at != "unknown" {
match chrono::DateTime::parse_from_rfc3339(extracted_at) {
Ok(dt) => format!("@{}", dt.timestamp()),
Err(_) => {
// Try parsing as bare ISO 8601 (no timezone)
match chrono::NaiveDateTime::parse_from_str(
&extracted_at[..19],
"%Y-%m-%dT%H:%M:%S",
) {
Ok(ndt) => format!("@{}", ndt.and_utc().timestamp()),
Err(_) => extracted_at.to_string(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline src/version.rs --match commit_project --view expanded
rg -n -C3 'extracted_at|parse_from_rfc3339|NaiveDateTime' src -g '*.rs'
rg -n -C3 'extracted_at|malformed.*date|short.*date' . -g '*test*' -g '*.rs'

Repository: meta-introspector/aristotle-cli-rs

Length of output: 12338


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the versioning flow and the metadata writers/consumers around extracted_at.
sed -n '1,220p' src/version.rs
printf '\n---\n'
sed -n '1,220p' src/fetch.rs
printf '\n---\n'
sed -n '1450,1505p' src/main.rs
printf '\n---\n'
sed -n '1,120p' src/replay.rs

# Look for any other writers or parsers of extracted_at that might emit non-RFC3339 values.
rg -n 'extracted_at|to_rfc3339|parse_from_rfc3339|NaiveDateTime|parse_from_str\(' src -g '*.rs'

Repository: meta-introspector/aristotle-cli-rs

Length of output: 24471


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the surrounding versioning control flow to see whether a panic aborts the whole run.
sed -n '220,360p' src/version.rs
rg -n 'commit_project\(|cmd_version\(' src/version.rs src/main.rs -g '*.rs'

Repository: meta-introspector/aristotle-cli-rs

Length of output: 4228


Handle short or malformed extracted_at values
&extracted_at[..19] will panic if the RFC3339 parse fails and the string is shorter than 19 bytes, aborting the whole versioning run on a bad metadata record. Use a safe slice like get(..19) before parsing the fallback format.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/version.rs` around lines 29 - 39, The fallback parsing in the git date
logic can panic on short extracted_at values. In the parse_from_rfc3339 error
branch within the git_date initialization, use extracted_at.get(..19) and only
call NaiveDateTime::parse_from_str when that slice exists; otherwise return
extracted_at.to_string() as the fallback.

Comment thread src/version.rs
Comment on lines 77 to +79
if !out.status.success() {
warn!(project = %project_name, "git init failed: {}", String::from_utf8_lossy(&out.stderr));
return Ok(false);
return Ok(None);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline src/version.rs --match 'commit_project|cmd_version' --view expanded
rg -n -C4 'Ok\(None\)|Ok\(Some|rev-parse|Committed|Skipped|Failed' src/version.rs src/fetch.rs

Repository: meta-introspector/aristotle-cli-rs

Length of output: 5284


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- src/fetch.rs (around versioning call) ---'
sed -n '180,250p' src/fetch.rs | cat -n

echo
echo '--- src/version.rs (commit_project and cmd_version) ---'
sed -n '1,330p' src/version.rs | cat -n

Repository: meta-introspector/aristotle-cli-rs

Length of output: 15737


Keep git command failures as errors, not skips. commit_project turns init/add/commit failures into Ok(None) and hash lookup failures into ?, so cmd_version can finish with failed repos counted as skipped or committed. Propagate these command failures and surface an aggregate failure instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/version.rs` around lines 77 - 79, Update commit_project so git init, add,
commit, and hash lookup failures are propagated as errors instead of returning
Ok(None) or being treated as skips; use the existing command-handling logic
around commit_project and cmd_version. Ensure cmd_version aggregates failed
repositories and returns an error when any repository operation fails, rather
than reporting them as skipped or committed.

Comment thread src/version.rs
Comment on lines +98 to +104
// Write .gitignore to exclude tarballs and nested projects
let gitignore_path = repo.join(".gitignore");
if !gitignore_path.exists() {
let gitignore_content = "*_aristotle.tar.gz\n*.tar.gz\n";
fs::write(&gitignore_path, gitignore_content)?;
changed = true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline src/version.rs --match commit_project --view expanded
rg -n -C4 'gitignore|tar\.gz|_aristotle|remove_dir|remove_file|git.*rm' src/version.rs

Repository: meta-introspector/aristotle-cli-rs

Length of output: 2971


🏁 Script executed:

#!/bin/bash
sed -n '1,220p' src/version.rs

Repository: meta-introspector/aristotle-cli-rs

Length of output: 7904


Remove stale generated content before staging. src/version.rs:98-130 only skips tarballs and nested _aristotle paths on copy; anything already in repo/ stays tracked, and an existing .gitignore is never updated. Delete matching destination files/dirs and merge these ignore rules before git add -A.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/version.rs` around lines 98 - 104, Before staging in the
version-generation flow, remove existing matching tarballs and nested
`_aristotle` project files/directories from `repo`, not just during copying.
Update the `.gitignore` handling in the relevant `src/version.rs` logic to merge
and persist the required ignore rules even when the file already exists, then
ensure cleanup occurs before `git add -A`.

@itoqa

itoqa Bot commented Jul 10, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: 8c99553: 10 test cases ran, 3 failed ❌, 5 passed ✅, 2 additional findings ⚠️.

Summary

This run covers core command-line workflows for fetching results, versioning them into git repositories, and re-running syncs with no changes, alongside edge cases around path routing and timestamp handling. Healthy baseline behavior is present on standard flows, but robustness issues appear when metadata or configuration inputs are malformed or unusual.

Merge with caution — this PR introduces attributable reliability issues, including a medium-severity crash path in timestamp handling plus minor routing/date-handling defects that can misroute output or turn commit problems into silent skips. There is also an unrelated high-severity sync integrity issue in the broader codebase, but it is not attributable to this PR.

Tests run by Ito

View full run

Result Severity Type Description
Medium severity Timestamp Expected behavior: invalid or short timestamps should be handled without crashing, allowing other projects to continue. Actual behavior: the process panics while slicing extracted_at and aborts the entire run.
Minor severity Routing Expected no extra path segment when git_base already targets git-versions, but fetch routed version output to a double-nested git-versions/git-versions directory.
Minor severity Timestamp Invalid extracted_at input is forwarded to git commit --date, commit creation fails for the project, and cmd_version aggregates that failure into the Skipped counter without a distinct failed count.
Routing Running fetch with git_base set to /tmp/qa-batch1/git-root created the versioned repo at /tmp/qa-batch1/git-root/git-versions/P001, which matches the intended nested routing behavior.
Sync Running version creates .gitignore with tarball exclusions and keeps archive.tar.gz out of tracked repository content.
Sync A second consecutive version run correctly skips all projects with no new commits when repository content and git status are unchanged.
Timestamp The version command successfully committed T100 and preserved the RFC3339 extracted_at value as the commit author date.
Timestamp With an invalid extracted_at value that makes git reject --date, the version command logs a warning, skips only the affected project, and continues to completion with exit status 0.
⚠️ High severity Sync The command treats git add operational failure as a skip-equivalent outcome after file copy has already mutated repo_dir, so retry commits blend stranded changes instead of preserving each attempt as a distinct state.
⚠️ Minor severity Routing fetch and pipeline resolved git output roots differently, so the same project was committed into separate repositories instead of one consistent location.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟠 git add failure leaves mutated but uncommitted repo state
  • Severity: High High severity
  • Description: The command treats git add operational failure as a skip-equivalent outcome after file copy has already mutated repo_dir, so retry commits blend stranded changes instead of preserving each attempt as a distinct state.
  • Impact: Maintainers can lose a distinct intermediate repository state when a sync attempt fails during staging and a later retry collapses multiple attempts into one commit. This harms change auditability and makes rollback or root-cause debugging significantly harder.
  • Steps to Reproduce:
    1. Run version once on a fixture project to establish a baseline commit (for example S400 at d400594).
    2. Modify source files, then force .git/index failure (for example replace .git/index with a directory) and run version again.
    3. Observe a WARN for git add failed with run summary Committed: 0, Skipped: 4, while repo files are updated to v2 content but HEAD stays on the baseline commit.
    4. Restore the git index, modify source again to v3, and run version a third time.
    5. Inspect the recovery commit (079b6a5) showing a single blended delta (2 files changed, 2 insertions, 2 deletions) that skips a distinct v2 commit in history.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: In commit_project, files are copied and changed is set before staging (src/version.rs lines 94-153). If git add -A fails, the code logs a warning and returns Ok(None) (src/version.rs lines 170-178) instead of returning an error or rolling back copied files. The caller in cmd_version maps Ok(None) to skipped += 1 with "No changes or no metadata" logging (src/version.rs lines 278-286), which masks a real operational failure as a normal skip. A targeted fix is to return Err(...) on git-add failure once copy has occurred (or explicitly mark a distinct failure state) so retries do not silently absorb stranded edits.
Evidence Package
⚪ Fetch and pipeline split repo roots
  • Severity: Minor Minor severity
  • Description: fetch and pipeline resolved git output roots differently, so the same project was committed into separate repositories instead of one consistent location.
  • Impact: A routing flow can intermittently hang and stop progressing until the user refreshes and retries.
  • Steps to Reproduce:
    1. Configure git_base to a path that differs from results_dir.
    2. Run cargo run -- fetch so versioning executes from cmd_fetch.
    3. Run cargo run -- pipeline on the same workspace and compare repo roots under both output paths.
  • Stub / mock content: The run redirected API calls to a local non-production endpoint, but no per-test bypass altered the fetch/pipeline path-resolution logic under test.
  • Code Analysis: src/fetch.rs:216 passes an explicit output directory derived from config.git_base.join("git-versions"), while src/pipeline.rs:183 calls cmd_version(None, None). In src/version.rs:229, the default path for that pipeline call becomes results_dir.join("git-versions"), so command order can split history across git_base-based and results_dir-based roots. The smallest practical fix is to have pipeline pass an explicit output dir based on config (or centralize one shared resolver) so both commands use identical git base derivation.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread src/fetch.rs
if downloaded > 0 {
println!("\n Versioning new results...");
crate::version::cmd_version(Some(config.results_dir.clone()), Some(config.git_base.clone()))?;
crate::version::cmd_version(Some(config.results_dir.clone()), Some(config.git_base.join("git-versions")))?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

View All Evidence

Minor severity Fetch double-nests git versions path

What failed: Expected no extra path segment when git_base already targets git-versions, but fetch routed version output to a double-nested git-versions/git-versions directory.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Minor Minor severity
  • Impact: Some users may hit a routing session that stops responding after completion and need to restart the app/session to continue.
  • Steps to Reproduce:
    1. Configure git_base to a path whose final segment is already git-versions.
    2. Run cargo run -- fetch so fetch triggers versioning after downloads.
    3. Inspect the printed git base and created repo path; they appear under .../git-versions/git-versions/<project_id> instead of .../git-versions/<project_id>.
  • Stub / mock content: The test run used a local mock Aristotle API endpoint and fixture directories to drive fetch/versioning in a non-production environment; no bypass changed the git path-routing logic under test.
  • Code Analysis: I inspected src/fetch.rs and src/version.rs. Fetch now calls cmd_version(..., Some(config.git_base.join("git-versions"))) at line 216, while cmd_version uses the provided output directory directly as git_base (src/version.rs lines 229 and 275). There is no guard in fetch to avoid appending when config.git_base already ends with git-versions, so this path construction deterministically produces double nesting for the ROUTING-2 configuration. The smallest practical fix is to normalize in fetch before calling cmd_version (for example, reuse config.git_base as-is when its last segment is git-versions).
  • Why this is likely a bug: This is a production code path mismatch, not a harness artifact: the constructed path is wrong by string/path composition alone and reproduces with a normal valid config value. The logic can strand versioned repos under an unintended root even when fetch/download succeeds.
Relevant code

src/fetch.rs:216

crate::version::cmd_version(Some(config.results_dir.clone()), Some(config.git_base.join("git-versions")))?;

src/version.rs:229

let git_base = output_dir.unwrap_or_else(|| results_dir.join("git-versions"));

src/version.rs:275

let repo_dir = git_base.join(repo_name);
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Minor severity — Fetch double-nests git versions path**

**What failed:** Expected no extra path segment when `git_base` already targets `git-versions`, but fetch routed version output to a double-nested `git-versions/git-versions` directory.

- **Impact:** Some users may hit a routing session that stops responding after completion and need to restart the app/session to continue.
- **Steps to reproduce:**
  1. Configure `git_base` to a path whose final segment is already `git-versions`.
  2. Run `cargo run -- fetch` so fetch triggers versioning after downloads.
  3. Inspect the printed git base and created repo path; they appear under `.../git-versions/git-versions/<project_id>` instead of `.../git-versions/<project_id>`.
- **Stub / mock content:** The test run used a local mock Aristotle API endpoint and fixture directories to drive fetch/versioning in a non-production environment; no bypass changed the git path-routing logic under test.
- **Code analysis:** I inspected `src/fetch.rs` and `src/version.rs`. Fetch now calls `cmd_version(..., Some(config.git_base.join("git-versions")))` at line 216, while `cmd_version` uses the provided output directory directly as `git_base` (`src/version.rs` lines 229 and 275). There is no guard in fetch to avoid appending when `config.git_base` already ends with `git-versions`, so this path construction deterministically produces double nesting for the ROUTING-2 configuration. The smallest practical fix is to normalize in fetch before calling `cmd_version` (for example, reuse `config.git_base` as-is when its last segment is `git-versions`).
- **Why this is likely a bug:** This is a production code path mismatch, not a harness artifact: the constructed path is wrong by string/path composition alone and reproduces with a normal valid config value. The logic can strand versioned repos under an unintended root even when fetch/download succeeds.

**Relevant code:**

`src/fetch.rs:216`

~~~rust
crate::version::cmd_version(Some(config.results_dir.clone()), Some(config.git_base.join("git-versions")))?;
~~~

`src/version.rs:229`

~~~rust
let git_base = output_dir.unwrap_or_else(|| results_dir.join("git-versions"));
~~~

`src/version.rs:275`

~~~rust
let repo_dir = git_base.join(repo_name);
~~~

Comment thread src/version.rs
Ok(dt) => format!("@{}", dt.timestamp()),
Err(_) => {
// Try parsing as bare ISO 8601 (no timezone)
match chrono::NaiveDateTime::parse_from_str(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

View All Evidence

Medium severity Short extracted_at string crashes version command

What failed: Expected behavior: invalid or short timestamps should be handled without crashing, allowing other projects to continue. Actual behavior: the process panics while slicing extracted_at and aborts the entire run.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: The version command can crash and stop processing remaining projects when commit timestamp metadata is shorter than expected, blocking version output for affected runs.
  • Steps to Reproduce:
    1. Create an Aristotle project metadata file with extracted_at set to 2026-07-10T12:34:5.
    2. Run aristotle-manager version --results-dir <results> --output-dir <git-output>.
    3. Observe panic at src/version.rs:35:34 with byte index 19 is out of bounds, and process exit 101.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: In src/version.rs, the PR-added git_date fallback slices extracted_at using &extracted_at[..19] before parsing %Y-%m-%dT%H:%M:%S. Rust string slicing panics on out-of-bounds byte indices, so an 18-byte value like 2026-07-10T12:34:5 crashes at runtime. The panic occurs inside commit_project before error-return paths and is not recoverable through the surrounding Result handling in cmd_version. Smallest practical fix: guard the slice length (or avoid slicing entirely) and treat unparsable values as a non-fatal per-project skip/error path.
  • Why this is likely a bug: The failure is directly explained by production code semantics: out-of-bounds Rust string slicing panics deterministically for short inputs. No test harness stubs or environment manipulation are needed to trigger it, so this is a real application defect.
Relevant code

src/version.rs:34-37

match chrono::NaiveDateTime::parse_from_str(
    &extracted_at[..19],
    "%Y-%m-%dT%H:%M:%S",
) {

src/version.rs:278-291

match commit_project(&repo_dir, project) {
    Ok(Some(rev)) => { ... }
    Ok(None) => { ... }
    Err(e) => {
        skipped += 1;
        warn!(project = %name, error = %e, "Failed");
    }
}
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Short extracted_at string crashes version command**

**What failed:** Expected behavior: invalid or short timestamps should be handled without crashing, allowing other projects to continue. Actual behavior: the process panics while slicing `extracted_at` and aborts the entire run.

- **Impact:** The version command can crash and stop processing remaining projects when commit timestamp metadata is shorter than expected, blocking version output for affected runs.
- **Steps to reproduce:**
  1. Create an Aristotle project metadata file with `extracted_at` set to `2026-07-10T12:34:5`.
  2. Run `aristotle-manager version --results-dir <results> --output-dir <git-output>`.
  3. Observe panic at `src/version.rs:35:34` with `byte index 19 is out of bounds`, and process exit 101.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** In `src/version.rs`, the PR-added `git_date` fallback slices `extracted_at` using `&extracted_at[..19]` before parsing `%Y-%m-%dT%H:%M:%S`. Rust string slicing panics on out-of-bounds byte indices, so an 18-byte value like `2026-07-10T12:34:5` crashes at runtime. The panic occurs inside `commit_project` before error-return paths and is not recoverable through the surrounding `Result` handling in `cmd_version`. Smallest practical fix: guard the slice length (or avoid slicing entirely) and treat unparsable values as a non-fatal per-project skip/error path.
- **Why this is likely a bug:** The failure is directly explained by production code semantics: out-of-bounds Rust string slicing panics deterministically for short inputs. No test harness stubs or environment manipulation are needed to trigger it, so this is a real application defect.

**Relevant code:**

`src/version.rs:34-37`

~~~rust
match chrono::NaiveDateTime::parse_from_str(
    &extracted_at[..19],
    "%Y-%m-%dT%H:%M:%S",
) {
~~~

`src/version.rs:278-291`

~~~rust
match commit_project(&repo_dir, project) {
    Ok(Some(rev)) => { ... }
    Ok(None) => { ... }
    Err(e) => {
        skipped += 1;
        warn!(project = %name, error = %e, "Failed");
    }
}
~~~

Comment thread src/version.rs
let extracted_at = metadata["extracted_at"]
.as_str()
.unwrap_or("unknown");
let git_date = if extracted_at != "unknown" {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

View All Evidence

Minor severity Raw extracted_at fallback feeds invalid git --date

What failed: Invalid extracted_at input is forwarded to git commit --date, commit creation fails for the project, and cmd_version aggregates that failure into the Skipped counter without a distinct failed count.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Minor Minor severity
  • Impact: Version generation can skip creating the expected commit when an extracted timestamp is malformed, while the run still appears successful.
  • Steps to Reproduce:
    1. Create a *_aristotle fixture with aristotle_metadata.json containing extracted_at='this-date-string-should-fail-git-date'.
    2. Run aristotle-manager version --results-dir <results_dir> --output-dir <git_output>.
    3. Observe git reporting fatal: invalid date format and the final summary counting the project under Skipped with exit code 0.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: In src/version.rs, lines 29-40 parse extracted_at and fall back to extracted_at.to_string() on parse failure, and lines 186-193 pass that value to git commit --date. For non-git-compatible strings this predictably fails and returns Ok(None) via lines 215-218, which cmd_version handles as skipped at lines 283-286; the summary at lines 295-296 only prints Committed and Skipped, so commit failures are not surfaced as their own outcome class.
  • Why this is likely a bug: The new fallback path treats an invalid external date as normal input, but git rejects it and the project commit is lost. A small practical fix is to validate fallback dates before calling git and track commit errors in a dedicated failed counter in the summary.
Relevant code

src/version.rs:29-40

let git_date = if extracted_at != "unknown" {
    match chrono::DateTime::parse_from_rfc3339(extracted_at) {
        Ok(dt) => format!("@{}", dt.timestamp()),
        Err(_) => {
            match chrono::NaiveDateTime::parse_from_str(&extracted_at[..19], "%Y-%m-%dT%H:%M:%S") {
                Ok(ndt) => format!("@{}", ndt.and_utc().timestamp()),
                Err(_) => extracted_at.to_string(),
            }
        }
    }
}

src/version.rs:186-193

let out = Command::new("git")
    .args([
        "-c", "user.name=aristotle-manager",
        "-c", "user.email=aristotle@harmonic.fun",
        "commit",
        "-F", &msg_path.to_string_lossy(),
        "--date", &git_date,
    ])

src/version.rs:283-296

Ok(None) => {
    skipped += 1;
    debug!(project = %name, "No changes or no metadata");
}
...
println!("  Committed: {}", committed);
println!("  Skipped:   {}", skipped);
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Minor severity — Raw extracted_at fallback feeds invalid git --date**

**What failed:** Invalid extracted_at input is forwarded to git commit --date, commit creation fails for the project, and cmd_version aggregates that failure into the Skipped counter without a distinct failed count.

- **Impact:** Version generation can skip creating the expected commit when an extracted timestamp is malformed, while the run still appears successful.
- **Steps to reproduce:**
  1. Create a *_aristotle fixture with aristotle_metadata.json containing extracted_at='this-date-string-should-fail-git-date'.
  2. Run `aristotle-manager version --results-dir <results_dir> --output-dir <git_output>`.
  3. Observe git reporting `fatal: invalid date format` and the final summary counting the project under Skipped with exit code 0.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** In src/version.rs, lines 29-40 parse extracted_at and fall back to extracted_at.to_string() on parse failure, and lines 186-193 pass that value to `git commit --date`. For non-git-compatible strings this predictably fails and returns Ok(None) via lines 215-218, which cmd_version handles as skipped at lines 283-286; the summary at lines 295-296 only prints Committed and Skipped, so commit failures are not surfaced as their own outcome class.
- **Why this is likely a bug:** The new fallback path treats an invalid external date as normal input, but git rejects it and the project commit is lost. A small practical fix is to validate fallback dates before calling git and track commit errors in a dedicated failed counter in the summary.

**Relevant code:**

`src/version.rs:29-40`

~~~rust
let git_date = if extracted_at != "unknown" {
    match chrono::DateTime::parse_from_rfc3339(extracted_at) {
        Ok(dt) => format!("@{}", dt.timestamp()),
        Err(_) => {
            match chrono::NaiveDateTime::parse_from_str(&extracted_at[..19], "%Y-%m-%dT%H:%M:%S") {
                Ok(ndt) => format!("@{}", ndt.and_utc().timestamp()),
                Err(_) => extracted_at.to_string(),
            }
        }
    }
}
~~~

`src/version.rs:186-193`

~~~rust
let out = Command::new("git")
    .args([
        "-c", "user.name=aristotle-manager",
        "-c", "user.email=aristotle@harmonic.fun",
        "commit",
        "-F", &msg_path.to_string_lossy(),
        "--date", &git_date,
    ])
~~~

`src/version.rs:283-296`

~~~rust
Ok(None) => {
    skipped += 1;
    debug!(project = %name, "No changes or no metadata");
}
...
println!("  Committed: {}", committed);
println!("  Skipped:   {}", skipped);
~~~

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