[freeqaz]

Problem

Follow-up to #4098. Two of its repository-set checks had no test that fails when the check is removed, and its end-to-end repository-set journey test was flaky:

• The use check in recordUse compares the request's repository set with the token's grant. Removing that comparison left every test green.
• readRecipientMaterial refuses a family whose grant profile differs from the requested scope. Removing that refusal left every test green.
• The journey's held hook probe ran once after one reconciliation pass. That pass can leave the route "unverified", so the probe answers "not yet" and the test fails (about 1 in 50 runs at 2 CPUs: {"pending":"unverified"} ... undefined !== 'held').

Change

Test-only. No product code changes.

• postgres-protected-git-recipient-custody.test.ts, repository-set retirement cases: after the A+B request is allowed, the same request that also names an unselected repository C is denied.
• Same file, repository-to-installation scope case: the selected-repository sender cannot read the installation-wide family's material ({ absent: "unprepared" }).
• postgres-repository-set-journey.ts: the held hook probe now runs in waitFor (20 s). Before each later read it runs one more reconciliation pass, as production enrollment retries on the worker's next pass. The last read happens at the deadline, there are no fixed sleeps, and a blocked route still fails at once.

Both new assertions use the fixtures that #4098 added. No new fixture copies.

Verification

• Mutants (on a remote test VM). Removing the repository-set comparison in recordUse makes the five repository-set retirement cases fail at the new assertion ('allow' !== 'denied'). Removing the profile/scope refusal in readRecipientMaterial makes the scope-change case fail at the new assertion (the installation material is returned). Both new assertions pass on this head.
• postgres-platform, narrowed to the two touched files: all 10 tests pass. The selected-repository unit and wire files also pass.
• Pins at 2 CPUs (postgres-platform lane, 10 runs x 5): postgres-protected-git-recipient-custody.test.ts passes 50/50 (all 8 tests). postgres-openshell-standing-git-transfer.test.ts passes 50/50 (both tests, including the repository-set journey).
• Full CI gate on a remote test VM at 92b435c47c: 22/22 jobs pass, including First Agent Smoke.
• Typecheck: pnpm run typecheck passes.

Risks

Low. The changes are test-only. The journey retry can add up to 20 s (plus one reconciliation pass) to a failing run, which stays inside the test's 90 s timeout.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs changes before merge.

What this changes

This PR adds repository-set denial assertions and retries the journey’s held hook probe after further reconciliation.

Example: A token covers repositories 73 and 74, but a use request also names repository 75.

• Before: The integration suite did not directly assert rejection of that expanded request.
• After: The suite requires denial and retains the existing assertion that only one authorized use was recorded.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Useful integration coverage with successful exact-head CI; proof remains harness-based and the outer deadline limits the retry.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored test-only PR changes no shipped behavior; its body reports mutation checks and PostgreSQL runs, and exact-head CI succeeds. Inspected screenshots linked from the earlier feature show that feature’s Agent workflow, not this test change.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Credential-boundary regressions could escape coverage, and transient route readiness could fail the repository-set journey.
Reason: The collaborator-authored direction strengthens existing contracts without changing platform behavior or adding test-only production seams.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep open: this PR adds useful credential-boundary coverage, with one test timeout mismatch to repair.

Priority: P3
Reviewed head: 92b435c47ce4e2c7aa11fe6ad216002410465f3b

Before merge

• Extend the outer worker wait to cover the new probe budget (P3) - The journey still calls f.work(work, "succeeded") without a timeout. That helper uses waitFor’s 10-second default, while preparation can now spend 20 seconds in this probe loop plus reconciliation time. If the route becomes ready after 10 seconds but within the new probe budget, the outer wait fails and starts teardown before preparation can succeed. Pass a sufficiently larger timeout at the journey’s f.work call so the added retry window can actually complete.

Findings

• [P3] Extend the outer worker wait to cover the new probe budget — tests/helpers/postgres-repository-set-journey.ts:390

Agent review details

How this fits together

OCC records repository sessions and credential custody in PostgreSQL; the credential owner checks requested scope before returning material or authorizing an OpenShell request.

flowchart LR
  Selection[Agent repository selection] --> Sessions[OCC repository sessions]
  Sessions --> Credentials[Prepared credential owner]
  Credentials --> Sender[OpenShell sender]
  Sender --> Check[Current scope check]
  Check --> Provider[GitHub operations]

Loading

Technical review

Best possible solution:

Keep the focused assertions and give the journey’s worker-result wait enough time for the probe and reconciliation budgets.

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

Source inspection establishes the nested 20-second probe and 10-second worker-result deadlines; no target code was executed during this read-only review.

Is this the best way to solve the issue?

Extending existing PostgreSQL cases and retrying through the existing runtime loop is appropriately scoped, but the enclosing deadline must accommodate the new wait.

Full review comments:

• [P3] Extend the outer worker wait to cover the new probe budget — tests/helpers/postgres-repository-set-journey.ts:390
  The journey still calls f.work(work, "succeeded") without a timeout. That helper uses waitFor’s 10-second default, while preparation can now spend 20 seconds in this probe loop plus reconciliation time. If the route becomes ready after 10 seconds but within the new probe budget, the outer wait fails and starts teardown before preparation can succeed. Pass a sufficiently larger timeout at the journey’s f.work call so the added retry window can actually complete.
  Confidence: 0.98

Overall correctness: patch is correct
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Held repository-set probe keeps the original intent (b08f900: The journey reconciles through the runtime loop to preserve observation fences and requires a held challenge before activation.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

• node --test tests/integration/postgres-protected-git-recipient-custody.test.ts
• node --test --test-name-pattern='one deployed OpenShell Agent pushes A and B' tests/integration/postgres-openshell-standing-git-transfer.test.ts
• pnpm lint
• pnpm typecheck
• git diff --check

What I checked:

• Introduced change: The pinned introduction changes only two test files: 36 additions and 5 deletions, with no production or persisted-format changes. (92b435c47ce4)
• Independent credential-boundary assertions: The added assertions exercise the real prepared owner against PostgreSQL: installation-wide material is unavailable through the selected-repository port, and adding repository 75 to an allowed 73+74 request is denied. These correspond to separate production checks in readRecipientMaterial and recordUse. (tests/integration/postgres-protected-git-recipient-custody.test.ts:329, 92b435c47ce4)
• Outer deadline undercuts the retry: The new probe waiter allows 20 seconds, but the journey calls f.work without its timeout argument; that helper delegates to waitFor, whose default is 10 seconds. (tests/helpers/postgres-repository-set-journey.ts:390, 92b435c47ce4)
• Exact-head CI: CI Required and PostgreSQL Platform succeeded for the reviewed head in https://github.com/openclaw/openclaw-enterprise/actions/runs/41124839872. Leaf-log retrieval redirected to a blocked Azure storage host, so detailed test output was not independently inspected. (92b435c47ce4)
• Feature history and discussion: Enable Git and GitHub API access to an Agent’s assigned repositories on OpenShell #4098 introduced the selected-set journey and retirement cases; Credential Proxy Implementation #2487 established the custody suite. Both commits map to freeqaz-openai. Path history and scoped content searches support that ownership; follow-history inspection reached a missing creation object. (tests/helpers/postgres-repository-set-journey.ts:369, b08f9006869a)
• Repository review policy: Read the root policy and checked for governing nested instructions; none were found under the changed paths. Applied test-integrity requirements and the implementation-status boundary. No maintainer-notes directory or pr-simplify skill was present; the autoreview helper was not run under the explicit single-reviewer instruction. (AGENTS.md, 92b435c47ce4)

Likely related people:

• Free Wortley: Raw commit b08f900 adds tests/helpers/postgres-repository-set-journey.ts:369 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: b08f9006869a; files: tests/helpers/postgres-repository-set-journey.ts)

Review metrics

Metric	Value	Why it matters
Change scope	Production +0/-0; tests +36/-5	The assertions reuse existing fixtures and the retry reuses the existing waiter.

Labels

Label changes:

• add P3: This is focused test hardening with a low-impact test deadline defect and no changed product behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P3: This is focused test hardening with a low-impact test deadline defect and no changed product behavior.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

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 30, 2026, 4:52 PM ET / 20:52 UTC.