Skip to content

fix: record dirty checkout state in operation provenance - #216

Merged
vishr merged 2 commits into
mainfrom
fix/dirty-git-provenance
Oct 5, 2026
Merged

vishr merged 2 commits into
mainfrom
fix/dirty-git-provenance

Conversation

@vishr

@vishr vishr commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

What this changes

Record SHA+dirty for a checkout with modified, staged, deleted, or untracked files instead of silently recording a clean HEAD. The CLI and shared Onebox service now use one Git-state implementation. If HEAD or status cannot be read, provenance is omitted rather than reported as clean.

Release IDs render the same marker as SHA-dirty within their existing safe alphabet, and the audit table accommodates the longer revision. The website explains the marker and its limits: ignored files are excluded, and the staged-payload digest remains separate evidence.

Closes #149.

Why this is correct

Regression tests failed against the original service and CLI helpers. Tests exercise clean and dirty states, user settings hiding untracked files, a corrupted index, unavailable/unborn repositories, cancellation, ignored files, unsafe ID inputs, valid rollback/retention IDs, and preservation/alignment in human and structured audit output. Real submodule tests verify ignored files remain clean, while tracked/untracked changes and advanced commits remain dirty even when user preferences hide submodule changes. Status uses normal untracked mode because provenance only needs to know whether changes exist, without enumerating every file. Independent local agent review found no remaining issues.

Validation: targeted regressions and just ci pass locally, including all Go tests, vet, lint, vulnerability/workflow checks, generated references, and website build. All five GitHub CI checks pass on the updated head, including Docker E2E and native Linux/macOS/Windows smoke tests. Copilot's final Lite review reports no findings and recommends approval. Docker/remote-host E2E was not run locally.

Effect on the safety envelope

Corrects misleading provenance without adding a deploy refusal or policy. Existing clean IDs remain unchanged; dirty IDs stay recognizable by rollback and retention.

Checklist

  • just check passes locally (included in just ci).
  • Tests cover the new behaviour, including the failure paths.
  • Generated documentation is current (just check verifies this).
  • CLA acceptance is managed by the contributor and repository bot.

@vishr
vishr requested a balanced review from Copilot October 5, 2026 04:52

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The focused changes preserve release-ID compatibility, cover key failure paths, and have no unresolved review findings.

Review effort: Balanced
Findings: None

What changed in this PR

Corrects checkout provenance for #149 by recording dirty state consistently across the CLI and shared service, without adding deployment restrictions.

Changes:

  • Centralizes Git-state detection and omits provenance when inspection fails.
  • Preserves dirty markers in safe release IDs and aligned audit output.
  • Adds regression coverage and documents provenance limits.
File Description
site/​src/​content/​docs/​explanation/​evidence-not-declaration.mdx Explains dirty markers and evidence limits.
internal/​release/​release.go Converts +dirty to ID-safe -dirty.
internal/​release/​release_test.go Tests dirty IDs and unsafe inputs.
internal/​onebox/​service.go Uses shared Git-state detection.
internal/​onebox/​git_test.go Covers checkout states and inspection failures.
internal/​gitinfo/​revision.go Implements shared checkout provenance.
internal/​engine/​audit.go Sizes the Git column dynamically.
internal/​engine/​audit_test.go Checks marker preservation and alignment.
cmd/​ob/​git_test.go Tests CLI dirty-state reporting.
cmd/​ob/​commands.go Uses shared Git-state detection.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@vishr
vishr requested a balanced review from Copilot October 5, 2026 05:00

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The focused changes have relevant regression coverage, preserve release-ID compatibility, and have no unresolved findings.

Review effort: Balanced
Findings: None

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Address untracked-tree traversal overhead and incorrect dirty detection for ignored files in submodules.

Review effort: Lite
Findings: None

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

No unresolved review issues remain, and all approval assessments indicate readiness.

Review effort: Lite
Findings: None

@vishr
vishr merged commit ea5d860 into main Oct 5, 2026
6 checks passed
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.

fix(journal): git_sha records a clean revision for a dirty working tree, contradicting its own contract

2 participants