Skip to content

Keep credential withdrawal tests bound to the source they requested - #2774

Merged
freeqaz-openai merged 1 commit into
mainfrom
refactor/credential-withdrawal-scenarios
Oct 17, 2026
Merged

freeqaz-openai merged 1 commit into
mainfrom
refactor/credential-withdrawal-scenarios

Conversation

@freeqaz-openai

Copy link
Copy Markdown
Collaborator

A withdrawal can outlive the person who requested it. These PostgreSQL tests now bind a withdrawal to its agent and credential source once, then name the acting principal at each read or replay. The scenarios still show the work attempts, audit records, and expected outcomes in place. This changes test code only.

All 36 tests in the file passed before and after against disposable PostgreSQL. I also checked that the relevant tests still fail if a withdrawal is attributed to the wrong person or if the test reads the denied source while expecting the allowed one to be revoked.

@freeqaz-openai freeqaz-openai added the agent-created PR opened by an AI agent; does not identify the reviewer or approver. label Oct 17, 2026
@clawsweeper

clawsweeper Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@freeqaz-openai
freeqaz-openai marked this pull request as ready for review October 17, 2026 19:02
@freeqaz-openai
freeqaz-openai requested a review from a team as a code owner October 17, 2026 19:02
@freeqaz-openai

Copy link
Copy Markdown
Collaborator Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 17, 2026
@clawsweeper

clawsweeper Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR binds credential-withdrawal test reads and replays to the Agent and source originally requested.

Example: An operator loses access after requesting a credential withdrawal.

  • Before: Each scenario separately constructs and reuses the withdrawal request for subsequent reads and authorized replays.
  • After: A source-bound handle retains the request target while each read or replay explicitly supplies its acting principal.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and source-consistent; validation rests on reported PostgreSQL runs and an integration job still running at inspection.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored test-only refactor changes no shipped behavior; the reported PostgreSQL runs support scenario preservation but are not live gateway proof.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Repeated withdrawal request plumbing makes source identity and acting-principal changes harder to follow in lifecycle tests.
Reason: The collaborator-authored change modestly simplifies existing scenarios without introducing a platform feature, contract change, or competing test oracle.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: the focused test refactor preserves withdrawal targets, explicit principals, and existing assertions, with no actionable defect found.

Priority: P3
Reviewed head: a5719b2e47951f3f95eb29ff6cc42c7acc5d8d2d

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The supplied validation concerns real PostgreSQL with controller/worker execution and Compute doubles; it does not demonstrate a live gateway withdrawal.
Agent review details

How this fits together

These PostgreSQL integration tests call OCC withdrawal APIs and run the controller worker against persisted work and audit records, using Compute doubles to simulate gateway outcomes.

flowchart TB
  A[Test scenario] --> B[Source-bound withdrawal handle]
  B --> C[OCC controller]
  C --> D[PostgreSQL state and work]
  D --> E[Controller worker]
  E --> F[Compute double]
  E --> G[Persisted outcomes and audit]
Loading

Technical review

Best possible solution:

Retain the source-bound helper and explicit per-call principals with the existing integration assertions.

Do we have a high-confidence way to reproduce the issue?

This PR refactors existing tests rather than repairing a reported product failure; no target code was executed during this read-only review.

Is this the best way to solve the issue?

The helper is appropriately local, preserves controller-owned revision selection, and keeps expected outcomes independent of request construction.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 3bdfcb10fe86.

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit f890b9d adds tests/integration/postgres-worker-agent-revision-credential-sources.test.mjs:1815 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: f890b9d48ddb; files: tests/integration/postgres-worker-agent-revision-credential-sources.test.mjs)

Review metrics

Metric Value Why it matters
Change scope Production +0/-0; tests +121/-142, net -21 lines The local helper replaces repeated request plumbing without expanding production code.

Labels

Label changes:

  • add P3: This is behavior-preserving test maintenance with no demonstrated user-facing regression.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This is behavior-preserving test maintenance with no demonstrated user-facing regression.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 17, 2026, 3:06 PM ET / 19:06 UTC.

@freeqaz-openai
freeqaz-openai merged commit cc14939 into main Oct 17, 2026
27 of 31 checks passed
@freeqaz-openai
freeqaz-openai deleted the refactor/credential-withdrawal-scenarios branch October 17, 2026 19:10

This branch was previously deployed

1 inactive deployment
integration-qa-pr — a5719b2e Deployed Oct 17, 2026 by freeqaz-openai via QA advisory (compose) #1993
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-created PR opened by an AI agent; does not identify the reviewer or approver. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant