Repository navigation
fix(api): restore guarded planned purchase creation - #873
Conversation
|
Independent verdict: no actionable findings in two adversarial passes at The fresh verifier traced the real handler transaction, plan lock and membership-write interlock; validation and unsupported-SP policy; automatic cap/global-switch controls; creator attribution; occupied steps and atomic date metadata; downstream root adoption; and pinned Azure quantity/payment/term conversion. Independent executed evidence:
Author full API race passed in 50.711s; native creator/combined scenarios in 22.408s; build, pinned integration lint and all normal hooks passed. Retained source/provenance reviews also completed two clean passes; the six owned file contents equal the original reviewed B source. Authentication, credentials, email, global state, due clock and the recording-only provider are fixtures. No real cloud purchase, live configuration mutation or manual deployment occurred. Initial sandbox connection/cache failures were excluded; approved retries supplied the behavioral evidence above. Owner-selected independent Sol review applies. The public head/target/labels were verified after creation. Exact-head CI |
b223cc8 to
6a2966f
Compare
|
The refreshed head is The creator fixture now persists an Azure account, while existing pause and run-now tests specialize that fixture with AWS credentials and frozen AWS recommendations. Their credential resolution selects Azure and fails before the recording provider. The failure reproduced on this exact head through the real handler and native PostgreSQL migration119, including an independent reproduction. Earlier creator-focused green results did not cover these consumers. The corrective commit The correction will land through a separate feature-target PR with explicit build/test CI dispatch before this PR receives its final new-head gates. No live purchases or cloud mutations were performed, and this PR is not ready to merge yet. |
There was a problem hiding this comment.
🔇 Additional comments: 7
internal/api/handler_purchases_pause_claim_integration_test.go: 1
105-106: LGTM!Also applies to: 116-116
internal/api/handler_plans_test.go: 1
724-745: LGTM!internal/api/plan_step_no_recs_test.go: 1
38-153: LGTM!internal/api/handler_test.go: 1
1367-1397: LGTM!internal/api/openapi.yaml: 1
449-452: LGTM!internal/api/handler_plans_concurrency_integration_test.go: 1
96-169: LGTM!internal/api/handler_plans.go: 1
544-546: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Compare
start_dateby calendar day, not by the current instant.
parseCreatePurchasesRequestparsesstart_dateas midnight UTC.!startDate.After(time.Now())therefore rejects today's date, which is expected. The check also depends on the server's current time relative to UTC midnight, so there is no time-zone edge for tomorrow: tomorrow at 00:00 UTC is always later than now. This check is correct. The real gap is that the 400 is returned only after the plan lock, account read, and global config read. Move the date check intoparseCreatePurchasesRequestso invalid requests fail before the transaction starts.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
- Review profile: CHILL
- Plan: Essentials
- Run ID:
4b03c4a0-dbbd-4673-810e-ed335547d8e2
📒 Files selected for processing: 7
internal/api/handler_plans.gointernal/api/handler_plans_concurrency_integration_test.gointernal/api/handler_plans_test.gointernal/api/handler_purchases_pause_claim_integration_test.gointernal/api/handler_test.gointernal/api/openapi.yamlinternal/api/plan_step_no_recs_test.go
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.
|
Independent review: PASS at Independent exact-head native race verification passed 33 cases in 35.181s, with 40 fresh databases confirming migration 119. It exercised all shared-fixture consumers, pause/run-now cases, creator scope/clock/rollback guards, nullable caps, SP policy and both Azure modes through recorded provider calls to durable completion without rebuying. Author API race passed in 47.566s, build and pinned integration lint passed, and all full-range normal hooks passed. The previous parent failures and scope/clock deletion controls remain valid through verified source identity. CodeRabbit's review 5526011434 covers this exact head and contains six LGTM notes plus one unverified performance suggestion. We've reviewed that suggestion in two independent passes and kept the future-date check after the plan lock at handler_plans.go:544. A request parsed just before UTC midnight can wait on the lock until its requested date is in the past, so moving the check exclusively into parsing would weaken validation. An extra early check would duplicate the authoritative check without a demonstrated performance problem, so there's no source change for this suggestion. There are no inline findings or unresolved source findings. Required pre-commit run 38653090026 passed on attempt 2 at this head after attempt 1 failed during Trivy installation before hooks started. The failed job was retried normally with the same source and all checks retained. Full CI run 38653090574 remains pending, so this isn't a merge-ready claim yet. The fixture and recording-provider limits in the PR body still apply, and no live purchase or cloud mutation was performed. |
|
Merged normally as The merged whole tree The guarded Add Purchases route is now on main. Its four automatic post-merge workflows, including AWS deployment, have individual background watchers. Deployment and read-only DEV verification are still pending, so this doesn't claim a live plan was changed or a live purchase was made. Original refs, worktrees and proof artifacts remain preserved for the active #631 delivery. Follow-up issues filed: none; the remaining existing #631 UI date-default and planned-purchase projection work is recorded on the parent issue and coordinated separately. |
|
The guarded creator from #873 is now verified in the running deployment. Public GET This confirms the deployed source version. No live plan or purchase was created, no scheduler/configuration state was changed, and no cloud purchase was performed. Authenticated plan listing was not exercised because this verification lane had no existing API session. The real local API/PostgreSQL and recording-provider execution evidence remains recorded above. The quantity/resource display correction continues in #876. The existing UI date-default improvement and permanent combined regression proof remain tracked under #631. |
The authenticated Add Purchases route currently refuses every valid request with409. Restore the existing transactional creator now that plan-step recommendation attachment and execution are available.
Scheduling rechecks full account scope under the plan lock, validates enabled plans/accounts/ramp/veto settings and occupied steps, stamps the creator, and writes roots plus the requested-date clock anchor atomically. Manual plans work with global automatic purchasing OFF; automatic plans require the global switch and a valid cap. Unsupported multi-step Savings Plans remain refused. Requested dates and existing veto rules are preserved.
Refs #631. Seven files, 392 changed lines. UI/graph changes are separate work; this refresh preserves the merged cap-null update behavior and schema migration 119. Corrective PR875 explicitly restores the AWS identity required by existing pause/run-now test fixtures after the shared creator fixture changed to Azure; production credential resolution stays unchanged.
Exact head
e25ae316057f67162b6ff0c3e2164fa22ac20024, tree1cabd2cbd3cb88e514d67d9aa01afeb1439e5a52, basea3bc552aaa85523ffbd96b2f865386bf662a440e:Production router/store/resolver/manager and local PostgreSQL are real. Authentication, credentials, email, global state, clock and recording-only provider are fixtures. No real purchase or live configuration mutation was performed. Root explicitly accepts this proof for code merge; postdeployment DEV verification remains read-only. Existing veto timing still applies; a short requested date is not silently retimed.
Required exact-head CI/protections must pass before normal squash merge. Owner-selected independent Sol review covers this revision; any actual new review finding still requires disposition. Source refs and original worktrees are preserved.