Skip to content

fix(lifecycle): reject contradictory zombie history - #180

Merged
aparragithub merged 3 commits into
mainfrom
feat/sp-resource-lifecycle-adapter
Aug 7, 2026
Merged

fix(lifecycle): reject contradictory zombie history#180
aparragithub merged 3 commits into
mainfrom
feat/sp-resource-lifecycle-adapter

Conversation

@aparragithub

@aparragithub aparragithub commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Closes #176

PR type

  • New feature

Summary

  • Reject returned registry records that contradict immutable quarantine history.
  • Require proven DEV/QA class and elapsed quarantine time before mutation.
  • Add explicit QA, PROD, timestamp and zero-mutation regression coverage.

Changes

File Change
src/odoo_forge/resource_lifecycle/types.py Record class and quarantine timestamp
src/odoo_forge/resource_lifecycle/service.py Validate history identity, class and elapsed wait
tests/resource_lifecycle/test_service.py Add contradiction and class/time safety tests

Chain context

Follow-up to merged PR #179. PR4 production authority and PR5 composition remain out of scope.

Test plan

  • uv run pytest tests/resource_lifecycle/test_service.py — 49 passed
  • uv run pytest — 1863 passed, 46 deselected
  • uv run mypy --no-incremental — 300 files, no issues
  • Ruff, import-linter and git diff --check passed

Contributor checklist

  • Linked approved issue Resource lifecycle: fail-closed service orchestration #176
  • Exactly one PR type selected
  • No shell scripts changed
  • Strict-TDD regressions exercised
  • No documentation change required for this internal correction
  • Conventional commit used
  • No AI attribution or co-author trailers

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened lifecycle recovery validation using resource class, policy approval, quarantine timestamps, and required waiting periods.
    • Detects conflicting or incomplete quarantine history and routes affected operations for human intervention.
    • Prevents invalid zombie revalidation, confirmation, and deletion actions.
    • Ensures quarantine records match the associated resource and operation history.
  • Tests

    • Added coverage for mismatched histories, invalid timestamps, insufficient wait times, and resource-class inconsistencies.

@aparragithub aparragithub added the type:feature New feature label Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@aparragithub, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 28 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a373185f-6c54-4f93-9eca-dfa023edaea6

📥 Commits

Reviewing files that changed from the base of the PR and between f9cafb5 and 9a3ee2e.

📒 Files selected for processing (3)
  • src/odoo_forge/resource_lifecycle/service.py
  • src/odoo_forge/resource_lifecycle/types.py
  • tests/resource_lifecycle/test_service.py
📝 Walkthrough

Walkthrough

Lifecycle recovery now validates quarantine history, resource classes, policy approval, timestamps, and elapsed waits. Contradictory evidence returns human-intervention results. Tests cover rejected mutations and valid delayed deletion.

Changes

Lifecycle recovery validation

Layer / File(s) Summary
Quarantine history contract
src/odoo_forge/resource_lifecycle/types.py, src/odoo_forge/resource_lifecycle/service.py
QuarantineHistory stores resource class and quarantine time. History creation passes the current timestamp.
Recovery validation flow
src/odoo_forge/resource_lifecycle/service.py
The service validates record lineage, zombie evidence, approved resource classes, timestamps, and quarantine waits before mutation. Contradictions produce history-mismatch.
Validation coverage
tests/resource_lifecycle/test_service.py
Tests cover identity and operation mismatches, resource-class conflicts, invalid timestamps, wait enforcement, and zero-mutation outcomes.

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

Sequence Diagram(s)

sequenceDiagram
  participant LifecycleService
  participant Registry
  participant Provider
  LifecycleService->>Registry: compare history-derived record
  Registry-->>LifecycleService: registry evidence
  LifecycleService->>Provider: validate resource class and zombie evidence
  Provider-->>LifecycleService: provider observation
  LifecycleService->>LifecycleService: validate quarantine timestamp and wait
  LifecycleService->>Provider: quarantine and delete after wait
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses history validation and quarantine waits but does not show all orchestration objectives required by issue [#176]. Add or provide evidence for joins, adoption, reconciliation, bounded retries, residual preservation, and the remaining required zero-mutation tests.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: rejecting contradictory zombie history.
Out of Scope Changes check ✅ Passed The service, type, and test changes directly support fail-closed lifecycle history validation described in issue [#176].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 feat/sp-resource-lifecycle-adapter

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/odoo_forge/resource_lifecycle/service.py`:
- Around line 413-422: Validate the quarantine wait at the start of
LifecycleService.run and reject values below timedelta(), before any lifecycle
processing or fresh-observation guard runs. Add a regression test covering a
negative wait and assert that neither quarantine nor delete is called.

In `@src/odoo_forge/resource_lifecycle/types.py`:
- Around line 126-127: Update the QuarantineHistory model’s quarantined_at field
to enforce timezone-aware datetimes, using AwareDatetime or an equivalent
Pydantic v2 validator. Ensure model construction rejects naive datetime values
while preserving valid timezone-aware timestamps.

In `@tests/resource_lifecycle/test_service.py`:
- Around line 667-669: Update the history event selection in the relevant test
to choose the last event with a non-None history, ensuring it validates the
quarantine event emitted by the service rather than the input appended by
_history_journal(qa_history). Keep the existing resource_class assertion against
that selected event.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f2035eba-027e-4237-af3e-e2e4732b1014

📥 Commits

Reviewing files that changed from the base of the PR and between e0e9cfd and f9cafb5.

📒 Files selected for processing (3)
  • src/odoo_forge/resource_lifecycle/service.py
  • src/odoo_forge/resource_lifecycle/types.py
  • tests/resource_lifecycle/test_service.py

Comment thread src/odoo_forge/resource_lifecycle/service.py
Comment thread src/odoo_forge/resource_lifecycle/types.py Outdated
Comment thread tests/resource_lifecycle/test_service.py Outdated
@aparragithub
aparragithub merged commit b8c7dc6 into main Aug 7, 2026
5 checks passed
@aparragithub
aparragithub deleted the feat/sp-resource-lifecycle-adapter branch August 8, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resource lifecycle: fail-closed service orchestration

1 participant