Skip volatile config items when timestamp parsing fails by st3penta · Pull Request #3486 · conforma/cli

st3penta · 2026-08-10T12:51:45Z

collectVolatileConfigItems and isVolatileMatcherActive both defaulted
to the effective time on parse failure. The resulting time window check
always passed, so any volatile exclude with garbage timestamps
permanently suppressed its targeted rule. Since --strict only checks
failures and excludes suppress failures from the report, this was a
full bypass.

Add parseVolatileTime: tries RFC3339, then date-only ("2006-01-02"),
then rejects. On failure, log a warning and skip the item in
collectVolatileConfigItems (continue) or treat it as inactive in
isVolatileMatcherActive (return false). Follows the existing
fallback pattern in policy.ParseEffectiveTime.

Update the existing "invalid time formats" test to assert fail-closed
behavior. Add test cases for garbage timestamps, date-only fallback,
and mixed valid/invalid fields in both packages.

Ref: https://issues.redhat.com/browse/EC-1911

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

coderabbitai · 2026-08-10T12:52:10Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 3dd52d31-cc80-4c2d-a5aa-d9423a581704

📥 Commits

Reviewing files that changed from the base of the PR and between 05d850e and f9006b8.

📒 Files selected for processing (3)

internal/policy/equivalence/equivalence_test.go
internal/timeutil/timeutil.go
internal/timeutil/timeutil_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

internal/policy/equivalence/equivalence_test.go
internal/timeutil/timeutil.go
internal/timeutil/timeutil_test.go

📝 Walkthrough

Walkthrough

Volatile criteria and matchers now accept RFC3339 and date-only timestamps. Missing bounds use the effective time. Invalid supplied timestamps skip criteria or deactivate matchers.

Changes

Volatile time validation

Layer / File(s)	Summary
Shared volatile-time parser `internal/timeutil/timeutil.go`, `internal/timeutil/timeutil_test.go`	A shared parser accepts RFC3339 timestamps and date-only values. Invalid input returns an error. Tests cover time zones, empty input, and partial dates.
Evaluator volatile criteria validation `internal/evaluator/criteria.go`, `internal/evaluator/criteria_test.go`	The evaluator uses the shared parser. Missing bounds default to the effective time. Invalid supplied bounds and invalid `EffectiveOn` values skip criteria.
Equivalence matcher validation `internal/policy/equivalence/equivalence.go`, `internal/policy/equivalence/equivalence_test.go`	Volatile matchers use the shared parser. Invalid values generate warnings and deactivate the matcher. Tests cover open-ended, future, expired, date-only, and malformed values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the primary change: skipping volatile configuration items when timestamp parsing fails.
Description check	✅ Passed	The description explains what changed, why it changed, the test coverage, and the related EC-1911 ticket.
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 unit tests (beta)

Create PR with unit tests

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

fullsend-ai-review · 2026-08-10T12:52:59Z

🤖 Finished Review · ✅ Success · Started 12:52 PM UTC · Completed 1:06 PM UTC

Commit: 87c4a29 · View workflow run →

qodo-for-conforma · 2026-08-10T12:53:37Z

PR Summary by Qodo

Fail-close volatile timestamp parsing for volatile criteria/matchers

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Fail-close volatile criteria when EffectiveOn/Until timestamps are unparseable.
• Accept RFC3339 and date-only (YYYY-MM-DD) timestamps for volatile windows.
• Add/adjust tests to prevent malformed timestamps from suppressing targeted rules.

Diagram

graph TD
  VC{{"Volatile criteria"}} --> CV["collectVolatileConfigItems"] --> OUT["Apply volatile exclude"]
  VC --> VM["isVolatileMatcherActive"] --> OUT
  ET{{"Effective time"}} --> CV --> PVT["parseVolatileTime"] --> OUT
  CT{{"Checker time"}} --> VM --> PVT
  PVT -->|"error"| SKIP["Warn + skip"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize parseVolatileTime in a shared package

➕ Avoids duplicated parsing logic across evaluator and equivalence packages
➕ Reduces risk of behavior drift (e.g., formats accepted, error messages, logging)
➕ Enables single, focused unit test suite for parsing semantics
➖ Requires moving code or introducing a small shared util package
➖ May slightly expand dependency surface between internal packages

2. Reuse policy.ParseEffectiveTime (or extend it) for volatile parsing

➕ Leverages an established convention already referenced by this PR
➕ Keeps all effective-time parsing rules in one place
➖ May require changing policy.ParseEffectiveTime contract to support 'fail-closed on non-empty' semantics
➖ Could introduce unintended behavior changes for existing policy parsing callers

Recommendation: The fail-closed behavior and date-only fallback are the right security posture for volatile exclusions. Consider a follow-up to deduplicate parseVolatileTime (either shared helper or policy-owned parsing) to prevent the evaluator and equivalence paths from diverging over time.

Files changed (4) +279 / -15

Bug fix (2) +57 / -12

criteria.goFail-close volatile criteria collection on unparseable timestamps +30/-10

Fail-close volatile criteria collection on unparseable timestamps

• Introduces parseVolatileTime (RFC3339, then YYYY-MM-DD fallback) and updates collectVolatileConfigItems to skip an entire volatile criteria entry when a non-empty EffectiveOn/EffectiveUntil cannot be parsed. Empty fields continue to default to the effective time (open-ended behavior preserved).

internal/evaluator/criteria.go

equivalence.goFail-close volatile matcher activation on timestamp parse errors +27/-2

Fail-close volatile matcher activation on timestamp parse errors

• Updates isVolatileMatcherActive to use parseVolatileTime and to return inactive (false) when parsing fails for any non-empty time field, logging a warning instead of silently accepting. Adds the same RFC3339 + date-only parsing helper in this package.

internal/policy/equivalence/equivalence.go

Tests (2) +222 / -3

criteria_test.goExpand volatile criteria tests for fail-closed parsing behavior +127/-3

Expand volatile criteria tests for fail-closed parsing behavior

• Updates the prior 'invalid time formats' case to assert that malformed timestamps no longer add volatile items. Adds coverage for garbage timestamps, date-only acceptance, mixed valid/invalid fields, and unit tests for parseVolatileTime including offsets.

internal/evaluator/criteria_test.go

equivalence_test.goAdd tests for volatile matcher activation time-window parsing +95/-0

Add tests for volatile matcher activation time-window parsing
• Adds TestIsVolatileMatcherActive to validate correct activation across valid windows, open-ended windows, date-only windows, and fail-closed behavior on garbage timestamps.
internal/policy/equivalence/equivalence_test.go

coderabbitai

Actionable comments posted: 1

🤖 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 `@internal/evaluator/criteria.go`:
- Around line 199-201: Update computeIncludeExclude to track when a volatile
include criterion is invalid in the error branches around the existing “skipping
volatile criteria” warnings, and suppress the later "*" fallback when no valid
include remains but an invalid volatile include was encountered. Preserve
wildcard fallback for other empty-include cases, and add an integration test
exercising computeIncludeExclude with only an invalid volatile include
criterion.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 43a053b7-9696-4518-92f8-1186242e55e6

📥 Commits

Reviewing files that changed from the base of the PR and between 1ea841a and 1181292.

📒 Files selected for processing (4)

internal/evaluator/criteria.go
internal/evaluator/criteria_test.go
internal/policy/equivalence/equivalence.go
internal/policy/equivalence/equivalence_test.go

qodo-for-conforma · 2026-08-10T13:01:03Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. equivalence_test.go missing build tag ✓ Resolved 📜 Skill insight ≡ Correctness

Description

New tests were added to internal/policy/equivalence/equivalence_test.go, but the file has no
first-line //go:build ... tag, so these tests will run in the default suite. This violates the
requirement that all modified Go test files be gated by an explicit build tag.

Code

internal/policy/equivalence/equivalence_test.go[R2047-2050]

+func TestIsVolatileMatcherActive(t *testing.T) {
+	effectiveTime := time.Date(2025, 8, 18, 12, 0, 0, 0, time.UTC)
+	checker := NewEquivalenceChecker(effectiveTime, nil)
+

Relevance

●●● Strong
Deterministic compliance fix; teams usually add required build tags to tests.
PR-#3276

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 3743 requires every new/modified Go test file to have a build tag on the first
line. The file header for internal/policy/equivalence/equivalence_test.go shows it begins with
copyright comments and then package equivalence with no //go:build ... tag present.

internal/policy/equivalence/equivalence_test.go[1-20]
Skill: pr-checklist

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`internal/policy/equivalence/equivalence_test.go` is a modified `_test.go` file but does not have a build tag comment on the first line (e.g., `//go:build unit`). This makes the tests run in the default test suite.

## Issue Context
PR adds `TestIsVolatileMatcherActive` to `equivalence_test.go`, bringing this file under the build-tag compliance rules for modified test files.

## Fix Focus Areas
- internal/policy/equivalence/equivalence_test.go[1-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Equivalence depends on policy~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

internal/policy/equivalence now imports internal/policy just to reuse policy.DateFormat, which
creates a new dependency edge and forces equivalence consumers (e.g. VSA validator) to compile the
policy package and its transitive dependencies. This increases build coupling/compile cost and makes
equivalence harder to reuse independently.

Code

internal/policy/equivalence/equivalence.go[R31-34]
+	log "github.com/sirupsen/logrus"
+
+	"github.com/conforma/cli/internal/policy"
)

Relevance

●● Moderate
Dependency-edge/build coupling concern is subjective; no close precedent found.
PR-#2690

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

equivalence now directly imports internal/policy, and internal/validate/vsa imports
equivalence, so VSA validation will now compile whatever internal/policy depends on. The
policy package has a broad import set (cosign/k8s/yaml/etc.), demonstrating why this dependency
edge is heavyweight.

internal/policy/equivalence/equivalence.go[19-34]
internal/policy/policy.go[19-45]
internal/validate/vsa/validator.go[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`internal/policy/equivalence` imports `github.com/conforma/cli/internal/policy` solely to access `policy.DateFormat`. This introduces an avoidable dependency edge and compile-time coupling for all consumers of `equivalence`.

### Issue Context
`internal/policy` pulls in many unrelated dependencies; `equivalence` is used by other components (e.g. VSA validation) that may not otherwise need the policy package.

### Fix Focus Areas
- internal/policy/equivalence/equivalence.go[19-35]

### Suggested fix
- Stop importing `internal/policy` from `equivalence`.
- Replace `policy.DateFormat` usage with either:
 1) a local constant `const dateFormat = "2006-01-02"`, or
 2) a new lightweight shared package (e.g. `internal/timeutil`) that provides `DateFormat` / `ParseVolatileTime`, used by both `evaluator` and `equivalence`.
- Ensure tests still pass and no new import cycles are introduced.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Volatile filtering not in filters.go ✗ Dismissed 📘 Rule violation ⌂ Architecture

Description

New volatile-config filtering logic (parseVolatileTime and fail-closed skipping) was added to
internal/evaluator/criteria.go instead of internal/evaluator/filters.go. This violates the
requirement to keep filtering-related logic consolidated in internal/evaluator/filters.go.

Code

internal/evaluator/criteria.go[R181-184]

+// parseVolatileTime tries RFC3339 first, then date-only ("2006-01-02") as a
+// fallback, matching the convention used in policy.ParseEffectiveTime.
+func parseVolatileTime(s string) (time.Time, error) {
+	if t, err := time.Parse(time.RFC3339, s); err == nil {

Relevance

●● Moderate
Architecture-only move; no close precedent enforcing filters.go consolidation.
PR-#3276

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 133 requires filtering/matching logic to reside in internal/evaluator/filters.go.
The PR adds parseVolatileTime and uses it inside collectVolatileConfigItems in
internal/evaluator/criteria.go, which is filtering-related code placed outside filters.go.

Rule 133: All filtering code must reside in internal/evaluator/filters.go
internal/evaluator/criteria.go[181-214]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Filtering-related logic was added/modified outside `internal/evaluator/filters.go`.

## Issue Context
This PR introduces `parseVolatileTime` and changes `collectVolatileConfigItems` behavior to skip volatile criteria when timestamps are unparseable. This is part of the include/exclude filtering pipeline.

## Fix Focus Areas
- internal/evaluator/criteria.go[181-214]
- internal/evaluator/filters.go[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Duplicated time parsing helper~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

parseVolatileTime is implemented twice (in internal/evaluator and internal/policy/equivalence)
with identical semantics, which increases the risk of future divergence and inconsistent behavior
between volatile-config evaluation and equivalence checking. Changes to supported formats or error
handling would have to be updated in two places to stay consistent.

Code

internal/policy/equivalence/equivalence.go[R363-372]

+// parseVolatileTime tries RFC3339 first, then date-only ("2006-01-02") as a
+// fallback, matching the convention used in policy.ParseEffectiveTime.
+func parseVolatileTime(s string) (time.Time, error) {
+	if t, err := time.Parse(time.RFC3339, s); err == nil {
+		return t, nil
+	}
+	if t, err := time.Parse(policy.DateFormat, s); err == nil {
+		return t, nil
+	}
+	return time.Time{}, fmt.Errorf("unable to parse %q as RFC3339 or %s", s, policy.DateFormat)

Relevance

●● Moderate
Deduping helper implies new shared util; design choice without precedent.
PR-#3276

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both files contain a parseVolatileTime function with the same RFC3339 + date-only fallback logic,
introduced by this PR, demonstrating the duplication risk.
internal/evaluator/criteria.go[181-191]
internal/policy/equivalence/equivalence.go[363-373]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The PR adds the same `parseVolatileTime` helper in two different packages. This duplication increases the chance the two implementations drift over time.

### Issue Context
A shared helper should be placed in a lightweight package to avoid recreating/strengthening the dependency concern from `equivalence -> policy`.

### Fix Focus Areas
- internal/evaluator/criteria.go[181-191]
- internal/policy/equivalence/equivalence.go[363-373]

### Suggested fix
- Create a small shared internal package (e.g. `internal/timeutil` or `internal/datetime`) containing:
 - `const DateFormat = "2006-01-02"`
 - `func ParseVolatileTime(s string) (time.Time, error)`
- Update both callers to use the shared helper.
- Keep the helper dependency-free (stdlib only) so it can be imported by both packages without pulling in `internal/policy`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 36 rules

✅ Skills: build-and-lint, pr-checklist, write-tests

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

fullsend-ai-review · 2026-08-10T13:05:57Z

Review

Findings

Medium

[stale-doc] docs/modules/ROOT/pages/configuration.adoc:110 — The "Volatile inclusions and exclusions" section documents effectiveOn/effectiveUntil timestamps using only RFC3339 format in examples (e.g., 2024-12-31T00:00:00Z). This PR adds date-only format (YYYY-MM-DD) as an accepted fallback, but the documentation does not mention this newly supported format.
Remediation: Add a note explaining that both RFC3339 and date-only (YYYY-MM-DD) formats are accepted for effectiveOn/effectiveUntil.

Low

[edge-case] internal/timeutil/timeutil.go:28 — Date-only strings are parsed as midnight UTC (00:00:00Z). An EffectiveUntil of 2025-08-01 is interpreted as 2025-08-01T00:00:00Z (start of day), not end of day. This is consistent with the existing policy.ParseEffectiveTime convention but is undocumented.
Remediation: Document in ParseVolatileTime doc comment that date-only strings are interpreted as midnight UTC.
[pattern-inconsistency] internal/timeutil/timeutil.go:24 — DateFormat constant (2006-01-02) is duplicated between internal/timeutil and internal/policy (policy.DateFormat) with no compile-time or test-time enforcement of consistency.
Remediation: Add a test asserting timeutil.DateFormat == policy.DateFormat.
[pattern-inconsistency] internal/timeutil/timeutil.go — ParseVolatileTime duplicates the RFC3339-then-date-only fallback logic already in policy.ParseEffectiveTime. The duplication is justified by import cycle constraints, but could be consolidated by having policy.ParseEffectiveTime delegate to timeutil.ParseVolatileTime.

Previous run

Review

Findings

Low

[import grouping] internal/evaluator/criteria_test.go:25 — The internal import (github.com/conforma/cli/internal/policy) is merged into the same group as external imports. The project convention uses 3 import groups (stdlib, external, internal) separated by blank lines.
Remediation: Restore the blank-line separator before the internal import.
[build tag placement] internal/policy/equivalence/equivalence_test.go:1 — The //go:build unit tag is placed before the copyright header. The project convention places build tags after the copyright header.
Remediation: Move //go:build unit to after the SPDX-License-Identifier line.
[constant duplication] internal/timeutil/timeutil.go:24 — timeutil.DateFormat duplicates policy.DateFormat (same value "2006-01-02"). Importing from policy would create an undesirable dependency from the utility package.
Remediation: Add a sync comment (e.g., // Must match policy.DateFormat).
[duplication-vs-reuse] internal/timeutil/timeutil.go:28 — ParseVolatileTime duplicates the RFC3339-then-date-only parsing logic in policy.parseEffectiveTime, though the latter also handles sentinels and past-time rejection. Minor duplication; future refactoring opportunity.
[stale documentation] cmd/compare/README.md:244 — The filtering logic pseudocode does not reflect the new fail-closed behavior on timestamp parse errors or the date-only format fallback.
Remediation: Update pseudocode to show parse-error handling.
[incomplete documentation] docs/modules/ROOT/pages/configuration.adoc:110 — Volatile config examples show only RFC3339 format; the newly accepted date-only format (YYYY-MM-DD) is not mentioned.
Remediation: Add a note or example showing date-only format acceptance.

Previous run (2)

Review

Findings

Medium

[code-duplication] internal/policy/equivalence/equivalence.go — parseVolatileTime is defined identically in both internal/evaluator/criteria.go and internal/policy/equivalence/equivalence.go as package-private functions. Both packages already import internal/policy for policy.DateFormat, so extracting a shared ParseVolatileTime into internal/policy would eliminate the divergence risk with no new dependency edges.
Remediation: Export a single policy.ParseVolatileTime(s string) (time.Time, error) in internal/policy/policy.go and call it from both evaluator and equivalence.

Low

[incomplete-docs] cmd/compare/README.md:244 — The "Filtering Logic" pseudocode block shows only the happy-path time comparison for volatile matchers but does not mention the new fail-closed behavior (unparseable timestamps cause the matcher to be skipped) or the date-only format (YYYY-MM-DD) fallback.
[incomplete-docs] docs/modules/ROOT/pages/configuration.adoc:110 — The volatile config section documents effectiveOn and effectiveUntil with only RFC3339 timestamp examples. The PR adds date-only format support as a fallback, but this option is not mentioned in the docs.

Labels: PR fixes a fail-open bypass where garbage timestamps in volatile criteria permanently suppressed policy rules

Previous run (3)

Review

Findings

Medium

[code-organization] internal/policy/equivalence/equivalence.go:362 — parseVolatileTime is defined identically in two separate packages (internal/evaluator/criteria.go and internal/policy/equivalence/equivalence.go). Both are unexported and both import the policy package for policy.DateFormat. This helper could be defined once as an exported function in the policy package (e.g., policy.ParseVolatileTime) to avoid future divergence risk in a security-critical parsing function.
Remediation: Move parseVolatileTime into the policy package as an exported function and call it from both call sites.

Labels: PR fixes a fail-open security bug in volatile config timestamp parsing

codecov · 2026-08-10T13:09:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
acceptance	`54.38% <43.33%> (-0.04%)`	⬇️
generative	`12.28% <0.00%> (-4.08%)`	⬇️
integration	`23.60% <33.33%> (-3.97%)`	⬇️
unit	`72.20% <100.00%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
internal/evaluator/criteria.go	`98.00% <100.00%> (+1.12%)`	⬆️
internal/policy/equivalence/equivalence.go	`87.02% <100.00%> (+0.19%)`	⬆️
internal/timeutil/timeutil.go	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

collectVolatileConfigItems and isVolatileMatcherActive both defaulted to the effective time on parse failure. The resulting time window check always passed, so any volatile exclude with garbage timestamps permanently suppressed its targeted rule. Since --strict only checks failures and excludes suppress failures from the report, this was a full bypass. Add parseVolatileTime: tries RFC3339, then date-only ("2006-01-02"), then rejects. On failure, log a warning and skip the item in collectVolatileConfigItems (continue) or treat it as inactive in isVolatileMatcherActive (return false). Follows the existing fallback pattern in policy.ParseEffectiveTime. Update the existing "invalid time formats" test to assert fail-closed behavior. Add test cases for garbage timestamps, date-only fallback, and mixed valid/invalid fields in both packages. Ref: https://issues.redhat.com/browse/EC-1911 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fullsend-ai-review · 2026-08-10T14:22:13Z

🤖 Finished Review · ✅ Success · Started 2:22 PM UTC · Completed 2:36 PM UTC

Commit: 87c4a29 · View workflow run →

fullsend-ai-review · 2026-08-10T14:50:31Z

🤖 Review · ⚠️ Cancelled · Started 2:50 PM UTC · Ended 3:03 PM UTC

Commit: 87c4a29 · View workflow run →

coderabbitai

Actionable comments posted: 1

🤖 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 `@internal/evaluator/criteria_test.go`:
- Line 1147: Run the repository’s configured Go formatter on criteria_test.go
and retain its output, including the formatting correction reported near line
1147, so the file passes static analysis.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: c65c8490-69c6-4b6a-9c06-ae980ac8ede5

📥 Commits

Reviewing files that changed from the base of the PR and between 1181292 and d83652c.

📒 Files selected for processing (6)

internal/evaluator/criteria.go
internal/evaluator/criteria_test.go
internal/policy/equivalence/equivalence.go
internal/policy/equivalence/equivalence_test.go
internal/timeutil/timeutil.go
internal/timeutil/timeutil_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

internal/policy/equivalence/equivalence_test.go
internal/policy/equivalence/equivalence.go
internal/evaluator/criteria.go

fullsend-ai-review · 2026-08-10T15:04:27Z

🤖 Finished Review · ✅ Success · Started 3:04 PM UTC · Completed 3:16 PM UTC

Commit: 87c4a29 · View workflow run →

fullsend-ai-review · 2026-08-10T15:26:18Z

🤖 Review · ⚠️ Cancelled · Started 3:26 PM UTC · Ended 3:28 PM UTC

Commit: 87c4a29 · View workflow run →

Consolidates the duplicated helper into a single exported ParseVolatileTime in internal/timeutil, stdlib-only. Drops the internal/policy import from equivalence.go (was pulling in cosign/k8s/yaml transitively just for a date format constant). Adds //go:build unit tag to equivalence_test.go. Ref: https://issues.redhat.com/browse/EC-1911 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fullsend-ai-review · 2026-08-10T15:29:58Z

🤖 Finished Review · ✅ Success · Started 3:29 PM UTC · Completed 3:47 PM UTC

Commit: 87c4a29 · View workflow run →

fullsend-ai-review

See the review comment for full details.

github-actions Bot added the size: XL label Aug 10, 2026

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread internal/evaluator/criteria.go

qodo-for-conforma Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread internal/policy/equivalence/equivalence_test.go

Comment thread internal/evaluator/criteria.go Outdated

Comment thread internal/policy/equivalence/equivalence.go

Comment thread internal/policy/equivalence/equivalence.go Outdated

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added requires-manual-review Review requires human judgment bug Something isn't working labels Aug 10, 2026

st3penta force-pushed the EC-1911 branch from 1181292 to 5f2cc08 Compare August 10, 2026 14:20

st3penta changed the title ~~Fail-close volatile config items with unparseable timestamps~~ Skip volatile config items when timestamp parsing fails Aug 10, 2026

fullsend-ai-review Bot added the Possible security concern label Aug 10, 2026

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread internal/evaluator/criteria_test.go Outdated

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Aug 10, 2026

st3penta force-pushed the EC-1911 branch from 57a1fd4 to 05d850e Compare August 10, 2026 15:24

st3penta force-pushed the EC-1911 branch from 05d850e to f9006b8 Compare August 10, 2026 15:28

fullsend-ai-review Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread internal/timeutil/timeutil.go

Comment thread internal/timeutil/timeutil.go

fullsend-ai-review Bot removed the ready-for-merge All reviewers approved — ready to merge label Aug 10, 2026

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 10, 2026

Uh oh!

Conversation

st3penta commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-for-conforma Bot commented Aug 10, 2026

PR Summary by Qodo

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

qodo-for-conforma Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Review

Findings

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Uh oh!

This comment was marked as outdated.

Uh oh!

codecov Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

st3penta commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

qodo-for-conforma Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

codecov Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 10, 2026 •

edited

Loading