Skip to content

fix(data): join ClearRotationForProject through namespace_entries (P6-DATA-02)#39

Merged
Reederey87 merged 2 commits into
mainfrom
fix/p6-data-02
Jul 3, 2026
Merged

fix(data): join ClearRotationForProject through namespace_entries (P6-DATA-02)#39
Reederey87 merged 2 commits into
mainfrom
fix/p6-data-02

Conversation

@Reederey87

@Reederey87 Reederey87 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

Pass 6 P2 quick-win wave, PR 2 of 4 (P6-DATA-02, docs/audits/AUDIT_RECOMMENDATIONS_2026-07-01_PASS6.md).

Store.ClearRotationForProject filtered on the non-existent env_profiles.namespace_id column, so the one-arg devstrap env rotate <path> (flag-clear-only) form failed on every invocation with a SQL logic error — env rotate --all was the only working path, and per-project rotation-flag clearing was dead. The subquery now joins through namespace_entries.env_profile_id (the actual project→profile link; the CLI already passes the namespace-entry ID):

AND env_profile_id IN (
  SELECT env_profile_id FROM namespace_entries
  WHERE id = ? AND env_profile_id IS NOT NULL)

Tests

  • TestClearRotationForProject (internal/state): two projects with encrypted profiles, both flagged; clearing project A returns A's binding count, leaves B flagged.
  • TestEnvRotateProjectClearsRotationFlag (internal/cli): one-arg env rotate work/x exits 0, prints "Cleared the needs-rotation flag on 2 binding(s) for work/x", zero flagged bindings remain.

Validation

  • gofmt -w cmd internal
  • golangci-lint run (0 issues)
  • go run ./cmd/spec-drift --base origin/main --head HEAD (pass)
  • GOCACHE=/tmp/devstrap-gocache go test -race ./... (green)

Specs: 09 + 12 (P6-DATA-02 sections → shipped), 18 work log; ledger reconciled. Note: the ledger open-count line will be reconciled against PR #38 at merge time (both PRs decrement it).

Implementation delegated to gpt-5.5 (Codex) per the model policy; diff reviewed line-by-line.

Follow-up (deferred): a CI lint that db.Prepares every static store query against a migrated in-memory DB.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed per-project environment rotation so it clears the “needs rotation” flag only for the selected project’s secret bindings, leaving other projects unchanged.
    • Corrected env rotate <path> to scope properly and report the accurate cleared-binding count.
  • Tests

    • Added coverage for per-project rotation clearing and for the single-argument CLI rotation flow.
  • Documentation

    • Updated the Pass 6 audit backlog and refreshed environment/data model documentation to reflect the shipped fix and updated review dates.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: de0784d5-8263-4770-bf78-e9140d8d0d66

📥 Commits

Reviewing files that changed from the base of the PR and between 4fd16cb and a6271a4.

📒 Files selected for processing (2)
  • docs/audits/README.md
  • spec/18_WORK_LOG.md
✅ Files skipped from review due to trivial changes (1)
  • spec/18_WORK_LOG.md

📝 Walkthrough

Walkthrough

This PR fixes per-project env rotation clearing through namespace_entries.env_profile_id, adds store and CLI regression tests, and updates the related spec, audit, and work-log entries to mark P6-DATA-02 as shipped.

Changes

P6-DATA-02: Env Rotation Clear Fix

Layer / File(s) Summary
ClearRotationForProject SQL fix and store test coverage
internal/state/store.go, internal/state/store_test.go
The rotation-clearing query now derives eligible env_profile_id values from namespace_entries for the project, and the new store test verifies only that project’s bindings are cleared.
CLI env rotate regression test
internal/cli/env_test.go
Adds a CLI test that runs env rotate work/x, checks the cleared-bindings message, and confirms no bindings remain flagged in the state DB.
Spec, audit ledger, and work-log updates
spec/09_SECRETS_AND_ENVIRONMENT.md, spec/12_DATA_MODEL_SQLITE.md, docs/audits/README.md, spec/18_WORK_LOG.md
Updates P6-DATA-02 and related backlog text from an open defect to a shipped fix, refreshes review dates, adjusts the Pass 6 open-findings count, and adds a new work-log entry.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI as devstrap env rotate
  participant Store as Store.ClearRotationForProject
  participant DB as SQLite

  CLI->>Store: ClearRotationForProject(projectID)
  Store->>DB: SELECT env_profile_id FROM namespace_entries WHERE project_id=? AND env_profile_id IS NOT NULL
  Store->>DB: UPDATE secret_bindings SET needs_rotation=false WHERE env_profile_id IN (...)
  DB-->>Store: rows affected
  Store-->>CLI: cleared count
  CLI-->>CLI: print "Cleared N bindings for project"
Loading

Compact metadata:

  • Related issues: None referenced
  • Related PRs: None referenced
  • Suggested labels: bug, tests, documentation
  • Suggested reviewers: None specified
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the main fix: ClearRotationForProject now joins through namespace_entries for P6-DATA-02.
Description check ✅ Passed The description covers the summary and tests, and it mostly matches the template, though the safety checklist is not filled out.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/p6-data-02

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: 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 `@docs/audits/README.md`:
- Around line 50-52: The Pass 6 open-count header is out of sync with the actual
audit rows after moving P6-DATA-02 to Recently shipped. Update the summary text
in the Pass 6 ledger entry so the “open of 43” count matches the current table
totals (and the shipped count implied by the pass index and Recently shipped
section). Verify the arithmetic against the Pass 6 open table and the “Recently
shipped” entries in docs/audits/README.md, then correct the header wording
accordingly.
🪄 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 Plus

Run ID: f8ec1626-37ac-4147-88e2-ebce780d8904

📥 Commits

Reviewing files that changed from the base of the PR and between e00a21b and 2aefb63.

📒 Files selected for processing (7)
  • docs/audits/README.md
  • internal/cli/env_test.go
  • internal/state/store.go
  • internal/state/store_test.go
  • spec/09_SECRETS_AND_ENVIRONMENT.md
  • spec/12_DATA_MODEL_SQLITE.md
  • spec/18_WORK_LOG.md

Comment thread docs/audits/README.md Outdated
…-DATA-02)

The one-arg `devstrap env rotate <path>` (flag-clear-only) filtered its
subquery on env_profiles.namespace_id — a column that does not exist —
so every invocation failed with "no such column: namespace_id"; only
`--all` worked. The project→profile link is namespace_entries.env_profile_id
and the CLI already passes the namespace-entry ID, so the subquery now
joins through namespace_entries (NULL-profile rows excluded).

Pinned by TestClearRotationForProject (per-project isolation: clearing A
leaves B flagged) and TestEnvRotateProjectClearsRotationFlag (one-arg CLI
form exits 0 with the cleared-count message).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Reederey87 Reederey87 enabled auto-merge (squash) July 3, 2026 08:25
…bit)

The header and the open table used different semantics: fully-applied
P6-DOC-02/P6-DOC-03 still sat in the open table while the header count
excluded them. Move the fully-applied doc rows to Recently shipped so
the header (33 open of 43) equals the row count by construction;
P6-DOC-01/P6-DOC-04 stay listed for their open test-hardening residuals.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Reederey87 Reederey87 merged commit 00a85e1 into main Jul 3, 2026
10 of 11 checks passed
@Reederey87 Reederey87 deleted the fix/p6-data-02 branch July 3, 2026 08:30
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