Repository navigation
feat(invites): accept repository invitations as the posting identity - #704
Conversation
Add `cr invites list` and `cr invites accept`. A profile's posting identity
can now accept pending repository invitations with the token cr already
holds, so a reviewer bot can be granted write access without anyone
handling its token.
accept requires an explicit --owner/--repo allowlist, supports --dry-run,
reports every pending invitation as accepted, would_accept, skipped or
failed, and exits 1 when any accept failed. The GitHub adapter implements
GET /user/repository_invitations and PATCH /user/repository_invitations/{id}
behind a new gitprovider.RepositoryInvitations seam; GitLab and GitHub App
credentials return config.ErrUnsupported.
Twapp-Session: 59d912f3-982f-402e-8bd8-3e31269a28b0
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 6269af4ecfb5
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| documentation:docs | 1 |
| policies:conventions | 1 |
| structure:repo-health | 1 |
| security:code-auditor | 0 |
| go:implementation | 0 |
| architecture:solid | 3 |
| harness-engineering:repo-health | 0 |
| configuration:data-files | 0 |
documentation:docs (1 finding)
Nits - README.md:1243
The
--ownerrow says "repositories this account owns", but "this account" is ambiguous: the posting identity is the invitee and does not own these repositories. The flag matches the login of the repository owner (the inviting user or org), case-insensitively. Reword to something like "Accept invitations to repositories owned by this login (user or organization). Repeatable."
policies:conventions (1 finding)
Nits - internal/cmd/invitescmd/invitescmd.go:1
docs/init-config-surface.md has a 'Runtime-only Flag Audit' table that lists every command's runtime-only flags (cr me, cr sessions, cr data, and so on). The new
cr invites list|acceptflags (--owner,--repo,--dry-run,--json) are not in it, so the audit no longer covers every command. Add a row such as| cr invites | --owner, --repo, --dry-run, --json | Per-run allowlist/filter and preview controls for account-level invitation acceptance, not profile config. |. That file is outside this PR's changed set, so the note is anchored here.
structure:repo-health (1 finding)
Minor - internal/cmd/invitescmd/invitescmd.go:263
mapRunError copies the credential-error guard list from internal/cmd/mecmd/mecmd.go:176. Both copies carry a 'keep in sync with cmderr.Credential' comment, so there are now three lists to keep aligned by hand. If a new credstore sentinel is added to cmderr.Credential, the commands can disagree on exit codes without any test failing. Fix: export one shared classifier from internal/cmd/cmderr, for example a helper that maps credential, config, or provider errors, or an IsCredential predicate. Have mecmd and invitescmd call it and keep only the command-specific cases (the invitations.Err* cases) locally.
architecture:solid (3 findings)
Minor - internal/architecture/invitation_writes_test.go:52
docs/architecture.md (Git Provider Writes) now says only internal/invitations may call AcceptRepositoryInvitation and that this test enforces it, but the guard is weaker than that claim (U-D2/U-T1 in support of the project rule).
- It only matches a selector that is the direct callee of a CallExpr, so a method value escapes it. I added
accept := p.AcceptRepositoryInvitation; return accept(ctx, "1")to internal/cmd/invitescmd in a temporary copy of the head commit, and the test still passed. That path skips the allowlist and --dry-run.- Line 20 exempts all of internal/gitprovider, including the github and gitlab subpackages, although no production code there calls the method; they only declare it. A method declaration is not a CallExpr, so the exemption is unnecessary: with that entry removed, the test still passed on the head commit. As written, any adapter or future wrapper could accept invitations without the allowlist, which the failure message and the docs both rule out.
This is the only enforcement for the one documented exception to the ledger/outbox write path, so it should match its claim.
Fix: drop
internal/gitproviderfrom allowedDirs and match any *ast.SelectorExpr with Sel.Name == "AcceptRepositoryInvitation" instead of only call expressions. That catches method values as well as calls and method expressions.
Minor - internal/gitprovider/invitations.go:34
U-I1 / U-O1: keeping this interface separate from GitProvider is the right call, and it makes the capability optional. But the PR still adds a stub to GitLab, and unsupported GitLab profiles now have three separate paths:
- invitescmd.prevalidate rejects GitLab before the factory runs.
- gitlab.Client implements RepositoryInvitations only to return config.ErrUnsupported (internal/gitprovider/gitlab/invitations.go).
- newConfiguredProvider has a
client.(Provider)failure branch that also returns ErrUnsupported. It cannot fire today, because gitproviders.New only returns the GitHub or GitLab client and both now satisfy the interface.In the production command path the GitLab stub is unreachable, and its existence is what makes the assertion branch dead. Each new host would repeat this question, and the three messages can drift.
Fix: delete the GitLab stub, its test and its compile-time assertion. Hosts without invitations then simply don't implement the interface, and the type assertion becomes the real capability check. Keep prevalidate so the keychain isn't opened needlessly. If you want the stub as a deliberate direct-caller contract, say so in this interface's doc comment and remove the dead assertion branch by having the provider constructor return the narrower type. Defensible either way on behavior, since all paths return ErrUnsupported; this is about having one owner for the decision.
Minor - internal/invitations/invitations.go:172
U-L2: Outcome carries the same failure twice, and the typed half is never used. Reason is set to err.Error() and Err keeps the typed error, but the only production consumer (view.NewInvitesAccept) reads Reason; Err is read only by invitations_test.go. The provider error class (ErrAuth, ErrPermission, ErrNotFound) is therefore dropped at this boundary.
The new --json output has one free-text
reasonfield covering host error text and the skip sentences at lines 106-107. A script that wants to tell expired from off-allowlist, or a 403 from a vanished invitation, has to match on prose such as "invitation expired; ask the inviter to send a new one".expired: truedoesn't settle it, because the golden file shows an expired, off-allowlist invitation reported as "not on the allowlist".Fix: give Outcome a closed reason code (for example not_allowlisted, expired, or a failure class from errors.Is on the provider sentinels) and add it to the JSON as
reason_code, leavingreasonas the human text. Then derive the human text in the view rather than storing it alongside Err. If scripted branching is out of scope for now, remove Err so the struct doesn't imply a contract nobody honors. Adding the field later is backward compatible, hence minor, but it's cheapest to do before consumers start matching on the strings.
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
go:implementation-tests— complete (broad); scope: 20 files; examples:cmd/cr/main.go,cmd/cr/main_test.go,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go,internal/cmd/invitescmd/invitescmd.go; 15 omitted; body-inspected: 20 files; examples:cmd/cr/main.go,cmd/cr/main_test.go,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go,internal/cmd/invitescmd/invitescmd.go; 15 omitted; skipped: none; context (not coverage): 4 files; examples:internal/cmd/cmderr/cmderr.go,internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/invitations/invitations.go; constraints: Golden and fixture files under testdata/ were not read; golden-output tests were assessed from the Go test code alone. Inspected only the three main implementation files in depth (invitations, github adapter, invitescmd); test files, fakes, view, gitlab, architecture and noleak changes were not read. No tests or linters were executed. Static review only; tests and linters were not run.documentation:docs— complete (broad); scope: 3 files; examples:README.md,docs/architecture.md,docs/development.md; body-inspected: 3 files; examples:README.md,docs/architecture.md,docs/development.md; skipped: none; context (not coverage): 2 files; examples:internal/architecture/invitation_writes_test.go,internal/cmd/invitescmd/testdata/accept.golden.json; constraints: Verified doc claims against file existence and golden JSON fixtures; did not execute the CLI.policies:conventions— complete (broad); scope: 6 files; examples:README.md,cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/cmd/invitescmd/invitescmd.go; 1 omitted; body-inspected: 6 files; examples:README.md,cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/cmd/invitescmd/invitescmd.go; 1 omitted; skipped: none; context (not coverage): 2 files; examples:AGENTS.md,docs/init-config-surface.md; constraints: Only the assigned files were reviewed; tests, internal/invitations and gitprovider changes were out of scope. Shared cli-common docs were not available in context, so no shared-standard drift was assessed beyond repo-local docs.structure:repo-health— complete (broad); scope: 7 files; examples:cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go; 2 omitted; body-inspected: 7 files; examples:cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go; 2 omitted; skipped: none; context (not coverage): 2 files; examples:internal/cmd/cmderr/cmderr.go,internal/cmd/mecmd/mecmd.go; constraints: Did not read internal/gitprovider or view package changes, or the test files; they are outside the assigned scope. Did not run make check or the tests.security:code-auditor— complete (broad); scope: 5 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/cmd/noleak/noleak_test.go,internal/gitprovider/github/invitations.go,internal/gitprovider/gitlab/invitations.go,internal/invitations/invitations.go; body-inspected: 5 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/cmd/noleak/noleak_test.go,internal/gitprovider/github/invitations.go,internal/gitprovider/gitlab/invitations.go,internal/invitations/invitations.go; skipped: none; constraints: Did not execute tests; static review only. Did not inspect doRESTPages/doREST error scrubbing or view rendering outside the assigned files.go:implementation— complete (broad); scope: 4 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; body-inspected: 4 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; skipped: none; context (not coverage): 1 file; examples:internal/gitprovider/github/http.go; constraints: Read doREST and the HTTP status mapping in internal/gitprovider/github/http.go for context only. No tests or linters were run. Test files and the view, noleak, and architecture-test changes were outside the assigned scope and were not reviewed.architecture:solid— complete (broad); scope: 4 files; examples:internal/architecture/invitation_writes_test.go,internal/gitprovider/fake_invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; body-inspected: 4 files; examples:internal/architecture/invitation_writes_test.go,internal/gitprovider/fake_invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; skipped: none; context (not coverage): 9 files; examples:docs/architecture.md,docs/development.md,internal/architecture/model_resolution_test.go,internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go; 4 omitted; constraints: Guard bypass checks ran in a temporary copy of the head commit, which was deleted afterwards; the review checkout was not modified. Ran go test on internal/architecture, internal/invitations, internal/gitprovider and internal/cmd/invitescmd only; make check was not run. internal/gitprovider TestProductionImportsStayInDataLayer failed here: its go list output parsing rejected this checkout's path, which contains spaces. The output points to the environment, not the diff; not re-run from a space-…harness-engineering:repo-health— complete (broad); scope: 3 files; examples:docs/architecture.md,docs/development.md,internal/architecture/invitation_writes_test.go; body-inspected: 3 files; examples:docs/architecture.md,docs/development.md,internal/architecture/invitation_writes_test.go; skipped: none; context (not coverage): 1 file; examples:internal/architecture/model_resolution_test.go; constraints: Did not run the tests; reviewed the diff and grepped the head checkout for AcceptRepositoryInvitation usages.configuration:data-files— complete (broad); scope: 4 files; examples:internal/cmd/invitescmd/testdata/accept.golden.json,internal/cmd/invitescmd/testdata/list.golden.json,internal/gitprovider/github/testdata/repository_invitations_page1.json,internal/gitprovider/github/testdata/repository_invitations_page2.json; body-inspected: 4 files; examples:internal/cmd/invitescmd/testdata/accept.golden.json,internal/cmd/invitescmd/testdata/list.golden.json,internal/gitprovider/github/testdata/repository_invitations_page1.json,internal/gitprovider/github/testdata/repository_invitations_page2.json; skipped: none; constraints: Did not cross-check the golden files against the Go test code that consumes them; review covered only JSON validity, dummy-data hygiene, and internal consistency.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 4m 51s | ~$2.21 (est.) | claude-sonnet-5-5, claude-opus-5-5 | cr 0.10.359
| Field | Value |
|---|---|
| Model | claude-sonnet-5-5, claude-opus-5-5 |
| Reviewers | go:implementation-tests, documentation:docs, policies:conventions, structure:repo-health, security:code-auditor, go:implementation, architecture:solid, harness-engineering:repo-health, configuration:data-files |
| Engine | claude_cli · claude-sonnet-5-5, claude-opus-5-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 4m 51s wall · 7m 52s compute |
| Cost | ~$2.21 (est.) |
| Pricing basis | baseline-2026-10-02/pricing |
| Tokens | 134 in / 35.4k out |
Per-workstream usage
orchestrator-selection— claude-sonnet-5-5- In: 6
- Out: 2.4k
- Cache read: 25.2k
- Cache create: 14.8k
- Cost: ~$0.09 (est.)
- Duration: 26s
go:implementation-tests— claude-sonnet-5-5- In: 22
- Out: 5.0k
- Cache read: 416.1k
- Cache create: 74.9k
- Cost: ~$0.43 (est.)
- Duration: 1m 05s
documentation:docs— claude-sonnet-5-5- In: 12
- Out: 2.0k
- Cache read: 79.5k
- Cache create: 14.6k
- Cost: ~$0.09 (est.)
- Duration: 28s
policies:conventions— claude-sonnet-5-5- In: 16
- Out: 3.0k
- Cache read: 161.2k
- Cache create: 26.3k
- Cost: ~$0.17 (est.)
- Duration: 35s
structure:repo-health— claude-sonnet-5-5- In: 14
- Out: 3.1k
- Cache read: 137.2k
- Cache create: 25.2k
- Cost: ~$0.16 (est.)
- Duration: 42s
security:code-auditor— claude-sonnet-5-5- In: 8
- Out: 1.1k
- Cache read: 54.3k
- Cache create: 23.6k
- Cost: ~$0.12 (est.)
- Duration: 19s
go:implementation— claude-sonnet-5-5- In: 12
- Out: 2.1k
- Cache read: 116.6k
- Cache create: 28.8k
- Cost: ~$0.16 (est.)
- Duration: 34s
architecture:solid— claude-opus-5-5- In: 18
- Out: 12.5k
- Cache read: 272.1k
- Cache create: 58.6k
- Cost: ~$0.77 (est.)
- Duration: 2m 32s
harness-engineering:repo-health— claude-sonnet-5-5- In: 12
- Out: 2.0k
- Cache read: 80.6k
- Cache create: 14.9k
- Cost: ~$0.10 (est.)
- Duration: 25s
configuration:data-files— claude-sonnet-5-5- In: 8
- Out: 1.2k
- Cache read: 42.3k
- Cache create: 12.6k
- Cost: ~$0.07 (est.)
- Duration: 22s
orchestrator-rollup— claude-sonnet-5-5- In: 6
- Out: 1.1k
- Cache read: 61.5k
- Cache create: 7.3k
- Cost: ~$0.05 (est.)
- Duration: 18s
- Add a closed reason_code to accept results; the view derives the text. - Share one error classifier, cmderr.Classify, between cr me and cr invites. - Build the invitation client through gitproviders.NewInvitationClient and drop the dead capability assertion; document the adapter contract. - Catch method values in the AcceptRepositoryInvitation guard and allow only internal/invitations. - Reword the --owner row and list cr invites in the runtime-only flag audit. Twapp-Session: 59d912f3-982f-402e-8bd8-3e31269a28b0
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: d4893fc77c21
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| documentation:docs | 0 |
| policies:conventions | 0 |
| structure:repo-health | 0 |
| security:code-auditor | 0 |
| go:implementation | 0 |
| architecture:solid | 1 |
| harness-engineering:repo-health | 0 |
| configuration:data-files | 0 |
architecture:solid (1 finding)
Minor - internal/invitations/invitations.go:201
U-L2: failureReason reports a host timeout as
canceled, overriding the adapter's ownretryableclassification. The context check runs first, so any error whose chain contains context.DeadlineExceeded returns ReasonCanceled.The GitHub adapter treats only context.Canceled as a cancellation. mapTransportError (internal/gitprovider/github/http.go:170) wraps everything else as ErrRetryable and keeps the cause in the chain, and the default client has a 2-minute http.Client timeout (client.go:159). I reproduced this in a temporary copy: a real http.Client timeout wrapped with WrapError(ErrRetryable, ...) gives errors.Is(err, context.DeadlineExceeded) == true, errors.Is(err, ErrRetryable) == true, and failureReason(err) == "canceled".
The impact is on the closed
reason_codethis PR adds for scripts: a transient host timeout, which is what a retry loop looks for, is reported as if the user had aborted, so a script branching onretryablewon't retry it. The humanreasontext still shows the timeout, and the exit code is 1 either way, so this is minor. It is cheapest to fix now, because correcting it later changes published JSON values.Fix: treat only context.Canceled as ReasonCanceled, matching mapTransportError, and let a deadline fall through to the ErrRetryable case. A bare DeadlineExceeded with no provider wrapper would then be ReasonError, unless you add an explicit case for it after the provider sentinels. Update the
{context.DeadlineExceeded, ReasonCanceled}row in TestFailureReasonClassifiesProviderErrors, and add a row for ErrRetryable wrapping a deadline error.
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
go:implementation-tests— complete (constrained); scope: 25 files; examples:cmd/cr/main.go,cmd/cr/main_test.go,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go,internal/cmd/cmderr/cmderr.go; 20 omitted; body-inspected: 25 files; examples:cmd/cr/main.go,cmd/cr/main_test.go,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go,internal/cmd/cmderr/cmderr.go; 20 omitted; skipped: none; context (not coverage): 2 files; examples:internal/invitations/invitations.go,internal/view/invites.go; constraints: Files not listed as inspected were not opened in this checkout. This includes the fakes, the GitHub and GitLab adapters and their tests, the noleak harness, the command tests and the cmd/cr wiring. They are listed as skipped. Static review only; tests and linters were not run. internal/view/invites.go and internal/invitations/invitations.go were read as context only; they are outside this repair's scope. testdata fixtures and golden files were not read; golden-output tests were judged from the …documentation:docs— complete (constrained); scope: 3 files; examples:README.md,docs/architecture.md,docs/development.md; body-inspected: 3 files; examples:README.md,docs/architecture.md,docs/development.md; skipped: none; context (not coverage): 2 files; examples:docs/init-config-surface.md,internal/architecture/invitation_writes_test.go; constraints: Static read of the diff and referenced files only; CLI not executed. Earlier threads on --owner wording and init-config-surface.md are addressed in the head diff.policies:conventions— complete (constrained); scope: 7 files; examples:README.md,cmd/cr/main.go,docs/architecture.md,docs/development.md,docs/init-config-surface.md; 2 omitted; body-inspected: 7 files; examples:README.md,cmd/cr/main.go,docs/architecture.md,docs/development.md,docs/init-config-surface.md; 2 omitted; skipped: none; constraints: Shared cli-common docs were not available in context; only repo-local conventions were checked. Tests, internal/invitations, cmderr and gitprovider changes were outside the assigned scope and were not reviewed.structure:repo-health— complete (constrained); scope: 7 files; examples:cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go; 2 omitted; body-inspected: 7 files; examples:cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go; 2 omitted; skipped: none; context (not coverage): 1 file; examples:docs/init-config-surface.md; constraints: Did not read the view, gitprovider, or test files, which are outside the assigned scope. Did not run make check or the tests. The README.md --owner wording thread was not checked; README.md is not in scope.security:code-auditor— complete (constrained); scope: 5 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/cmd/noleak/noleak_test.go,internal/gitprovider/github/invitations.go,internal/gitprovider/gitlab/invitations.go,internal/invitations/invitations.go; body-inspected: 5 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/cmd/noleak/noleak_test.go,internal/gitprovider/github/invitations.go,internal/gitprovider/gitlab/invitations.go,internal/invitations/invitations.go; skipped: none; constraints: Static review only; tests not executed. Did not inspect cmderr.Classify, gitproviders.NewInvitationClient, view rendering, or the architecture test (outside assigned scope).go:implementation— complete (constrained); scope: 4 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; body-inspected: 4 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; skipped: none; context (not coverage): 1 file; examples:internal/gitproviders/invitations.go; constraints: Read internal/gitproviders/invitations.go for context only. Test files, the view, cmderr and architecture-test changes, and docs were outside the assigned scope and were not reviewed. No tests or linters were run.architecture:solid— complete (constrained); scope: 4 files; examples:internal/architecture/invitation_writes_test.go,internal/gitprovider/fake_invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; body-inspected: 4 files; examples:internal/architecture/invitation_writes_test.go,internal/gitprovider/fake_invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; skipped: none; context (not coverage): 13 files; examples:docs/architecture.md,internal/cmd/cmderr/cmderr.go,internal/cmd/invitescmd/invitescmd.go,internal/cmd/mecmd/mecmd.go,internal/gitprovider/errors.go; 8 omitted; constraints: Guard and timeout checks ran in temporary copies of the head commit, which were deleted afterwards; the review checkout was not modified. Ran go test on the architecture, invitations, gitprovider (with adapters), gitproviders, invitescmd, cmderr, mecmd and view packages only; make check was not run. The gitprovider, github and gitlab import tests failed in the review checkout because its path contains spaces. All three packages passed in a temporary copy of the head commit at a space-free path,…harness-engineering:repo-health— complete (constrained); scope: 3 files; examples:docs/architecture.md,docs/development.md,internal/architecture/invitation_writes_test.go; body-inspected: 3 files; examples:docs/architecture.md,docs/development.md,internal/architecture/invitation_writes_test.go; skipped: none; constraints: Did not run the tests; reviewed the diff and grepped the head checkout for AcceptRepositoryInvitation references. Scope was limited to docs/architecture.md, docs/development.md and the architecture test. Other discussion threads (README, invitescmd, cmderr, outcome fields) concern files outside this assignment.configuration:data-files— complete (constrained); scope: 4 files; examples:internal/cmd/invitescmd/testdata/accept.golden.json,internal/cmd/invitescmd/testdata/list.golden.json,internal/gitprovider/github/testdata/repository_invitations_page1.json,internal/gitprovider/github/testdata/repository_invitations_page2.json; body-inspected: 4 files; examples:internal/cmd/invitescmd/testdata/accept.golden.json,internal/cmd/invitescmd/testdata/list.golden.json,internal/gitprovider/github/testdata/repository_invitations_page1.json,internal/gitprovider/github/testdata/repository_invitations_page2.json; skipped: none; constraints: Did not cross-check the golden files against the Go tests that consume them; review covered JSON validity, dummy-data hygiene, and internal consistency only.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 3m 33s | ~$1.78 (est.) | claude-sonnet-5-5, claude-opus-5-5 | cr 0.10.359
| Field | Value |
|---|---|
| Model | claude-sonnet-5-5, claude-opus-5-5 |
| Reviewers | go:implementation-tests, documentation:docs, policies:conventions, structure:repo-health, security:code-auditor, go:implementation, architecture:solid, harness-engineering:repo-health, configuration:data-files |
| Engine | claude_cli · claude-sonnet-5-5, claude-opus-5-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 3m 33s wall · 5m 27s compute |
| Cost | ~$1.78 (est.) |
| Pricing basis | baseline-2026-10-02/pricing |
| Tokens | 98 in / 22.0k out |
Per-workstream usage
go:implementation-tests— claude-sonnet-5-5- In: 16
- Out: 4.6k
- Cache read: 793.0k
- Cache create: 62.2k
- Cost: ~$0.45 (est.)
- Duration: 1m 08s
documentation:docs— claude-sonnet-5-5- In: 8
- Out: 1.3k
- Cache read: 99.1k
- Cache create: 12.0k
- Cost: ~$0.08 (est.)
- Duration: 20s
policies:conventions— claude-sonnet-5-5- In: 8
- Out: 1.3k
- Cache read: 144.0k
- Cache create: 10.9k
- Cost: ~$0.09 (est.)
- Duration: 22s
structure:repo-health— claude-sonnet-5-5- In: 8
- Out: 1.5k
- Cache read: 142.7k
- Cache create: 13.5k
- Cost: ~$0.10 (est.)
- Duration: 20s
security:code-auditor— claude-sonnet-5-5- In: 8
- Out: 1.0k
- Cache read: 138.2k
- Cache create: 15.1k
- Cost: ~$0.10 (est.)
- Duration: 17s
go:implementation— claude-sonnet-5-5- In: 8
- Out: 1.3k
- Cache read: 164.8k
- Cache create: 20.8k
- Cost: ~$0.13 (est.)
- Duration: 20s
architecture:solid— claude-opus-5-5- In: 16
- Out: 7.7k
- Cache read: 668.2k
- Cache create: 41.3k
- Cost: ~$0.62 (est.)
- Duration: 1m 41s
harness-engineering:repo-health— claude-sonnet-5-5- In: 12
- Out: 1.5k
- Cache read: 165.3k
- Cache create: 14.0k
- Cost: ~$0.10 (est.)
- Duration: 24s
configuration:data-files— claude-sonnet-5-5- In: 8
- Out: 1.2k
- Cache read: 88.6k
- Cache create: 10.4k
- Cost: ~$0.07 (est.)
- Duration: 19s
orchestrator-rollup— claude-sonnet-5-5- In: 6
- Out: 549
- Cache read: 80.7k
- Cache create: 4.2k
- Cost: ~$0.04 (est.)
- Duration: 13s
Twapp-Session: 59d912f3-982f-402e-8bd8-3e31269a28b0
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 032cccd2f135
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| documentation:docs | 0 |
| policies:conventions | 0 |
| structure:repo-health | 0 |
| security:code-auditor | 0 |
| go:implementation | 0 |
| architecture:solid | 0 |
| harness-engineering:repo-health | 0 |
| configuration:data-files | 0 |
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
go:implementation-tests— complete (constrained); scope: 25 files; examples:cmd/cr/main.go,cmd/cr/main_test.go,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go,internal/cmd/cmderr/cmderr.go; 20 omitted; body-inspected: 25 files; examples:cmd/cr/main.go,cmd/cr/main_test.go,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go,internal/cmd/cmderr/cmderr.go; 20 omitted; skipped: none; context (not coverage): 2 files; examples:internal/gitprovider/github/http.go,internal/invitations/invitations.go; constraints: Focused on internal/invitations (failureReason and its test), the area that changed since the previous head. Other files were not re-opened in this checkout and are listed as skipped. Golden and fixture files under testdata/ were not read; golden-output tests were judged from the Go test code alone. Static review only; tests and linters were not run. failureReason now classifies context.DeadlineExceeded as retryable and only context.Canceled as canceled, with test cases for both. This resolves …documentation:docs— complete (constrained); scope: 3 files; examples:README.md,docs/architecture.md,docs/development.md; body-inspected: 3 files; examples:README.md,docs/architecture.md,docs/development.md; skipped: none; context (not coverage): 1 file; examples:docs/init-config-surface.md; constraints: Static read of the diff only; CLI not executed. The failureReason deadline-vs-canceled thread is a code behavior issue, not a docs inaccuracy, and is left to code reviewers.policies:conventions— complete (constrained); scope: 7 files; examples:README.md,cmd/cr/main.go,docs/architecture.md,docs/development.md,docs/init-config-surface.md; 2 omitted; body-inspected: 7 files; examples:README.md,cmd/cr/main.go,docs/architecture.md,docs/development.md,docs/init-config-surface.md; 2 omitted; skipped: none; constraints: Shared cli-common docs were not available in context; only repo-local conventions were checked. The delta since the previous revision touched only internal/invitations, which is outside the assigned scope and was not reviewed.structure:repo-health— complete (constrained); scope: 7 files; examples:cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go; 2 omitted; body-inspected: 7 files; examples:cmd/cr/main.go,docs/architecture.md,docs/development.md,internal/architecture/command_boundaries_test.go,internal/architecture/invitation_writes_test.go; 2 omitted; skipped: none; constraints: Did not read the view, gitprovider, or test files, which are outside the assigned scope. Did not run make check or the tests. The README.md --owner wording thread was not checked; README.md is not in scope.security:code-auditor— complete (constrained); scope: 5 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/cmd/noleak/noleak_test.go,internal/gitprovider/github/invitations.go,internal/gitprovider/gitlab/invitations.go,internal/invitations/invitations.go; body-inspected: 5 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/cmd/noleak/noleak_test.go,internal/gitprovider/github/invitations.go,internal/gitprovider/gitlab/invitations.go,internal/invitations/invitations.go; skipped: none; constraints: Static review only; tests not executed. Did not inspect cmderr.Classify, gitproviders.NewInvitationClient, view rendering, or the architecture test (outside assigned scope).go:implementation— complete (constrained); scope: 4 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; body-inspected: 4 files; examples:internal/cmd/invitescmd/invitescmd.go,internal/gitprovider/github/invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; skipped: none; context (not coverage): 1 file; examples:internal/gitprovider/github/http.go; constraints: I read the failureReason change in internal/invitations/invitations.go and the github invitations adapter in full. I did not re-read invitescmd.go or gitprovider/invitations.go line by line this round; I carried over my earlier review of them. Test files, view, cmderr, architecture-test and docs changes were outside the assigned scope and were not reviewed. No tests or linters were run. Test files, view, cmderr, gitproviders and docs were outside the assigned scope and were not reviewed. No tes…architecture:solid— complete (constrained); scope: 4 files; examples:internal/architecture/invitation_writes_test.go,internal/gitprovider/fake_invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; body-inspected: 4 files; examples:internal/architecture/invitation_writes_test.go,internal/gitprovider/fake_invitations.go,internal/gitprovider/invitations.go,internal/invitations/invitations.go; skipped: none; context (not coverage): 1 file; examples:internal/invitations/invitations_test.go; constraints: Ran go test on the architecture, invitations, gitproviders, invitescmd, cmderr and view packages at this head, all passing. gitprovider and its adapters were not re-run because they are unchanged; make check was not run. Since the previously reviewed commit d4893fc only internal/invitations/invitations.go and its test changed; the other three assigned files are identical to that review and were not re-examined in depth.harness-engineering:repo-health— complete (constrained); scope: 3 files; examples:docs/architecture.md,docs/development.md,internal/architecture/invitation_writes_test.go; body-inspected: 3 files; examples:docs/architecture.md,docs/development.md,internal/architecture/invitation_writes_test.go; skipped: none; context (not coverage): 1 file; examples:docs/init-config-surface.md; constraints: Did not run the tests; reviewed the diff and the head checkout statically. Scope was limited to docs/architecture.md, docs/development.md and the architecture test. Discussion threads about the README, invitescmd, cmderr and the failureReason classification concern files outside this assignment.configuration:data-files— complete (constrained); scope: 4 files; examples:internal/cmd/invitescmd/testdata/accept.golden.json,internal/cmd/invitescmd/testdata/list.golden.json,internal/gitprovider/github/testdata/repository_invitations_page1.json,internal/gitprovider/github/testdata/repository_invitations_page2.json; body-inspected: 4 files; examples:internal/cmd/invitescmd/testdata/accept.golden.json,internal/cmd/invitescmd/testdata/list.golden.json,internal/gitprovider/github/testdata/repository_invitations_page1.json,internal/gitprovider/github/testdata/repository_invitations_page2.json; skipped: none; constraints: Did not cross-check the golden files against the Go tests that consume them; review covered JSON validity, dummy-data hygiene, and internal consistency only.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 2m 42s | ~$1.53 (est.) | claude-sonnet-5-5, claude-opus-5-5 | cr 0.10.359
| Field | Value |
|---|---|
| Model | claude-sonnet-5-5, claude-opus-5-5 |
| Reviewers | go:implementation-tests, documentation:docs, policies:conventions, structure:repo-health, security:code-auditor, go:implementation, architecture:solid, harness-engineering:repo-health, configuration:data-files |
| Engine | claude_cli · claude-sonnet-5-5, claude-opus-5-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 2m 42s wall · 3m 45s compute |
| Cost | ~$1.53 (est.) |
| Pricing basis | baseline-2026-10-02/pricing |
| Tokens | 90 in / 15.0k out |
Per-workstream usage
go:implementation-tests— claude-sonnet-5-5- In: 20
- Out: 4.6k
- Cache read: 1.3M
- Cache create: 60.8k
- Cost: ~$0.55 (est.)
- Duration: 1m 02s
documentation:docs— claude-sonnet-5-5- In: 6
- Out: 894
- Cache read: 104.7k
- Cache create: 11.0k
- Cost: ~$0.07 (est.)
- Duration: 15s
policies:conventions— claude-sonnet-5-5- In: 8
- Out: 1.2k
- Cache read: 187.0k
- Cache create: 10.1k
- Cost: ~$0.09 (est.)
- Duration: 15s
structure:repo-health— claude-sonnet-5-5- In: 8
- Out: 1.1k
- Cache read: 193.9k
- Cache create: 10.6k
- Cost: ~$0.09 (est.)
- Duration: 15s
security:code-auditor— claude-sonnet-5-5- In: 6
- Out: 870
- Cache read: 140.8k
- Cache create: 12.0k
- Cost: ~$0.08 (est.)
- Duration: 15s
go:implementation— claude-sonnet-5-5- In: 16
- Out: 2.3k
- Cache read: 535.2k
- Cache create: 29.7k
- Cost: ~$0.25 (est.)
- Duration: 40s
architecture:solid— claude-opus-5-5- In: 6
- Out: 1.3k
- Cache read: 327.5k
- Cache create: 14.1k
- Cost: ~$0.20 (est.)
- Duration: 20s
harness-engineering:repo-health— claude-sonnet-5-5- In: 8
- Out: 1.2k
- Cache read: 154.6k
- Cache create: 10.8k
- Cost: ~$0.09 (est.)
- Duration: 16s
configuration:data-files— claude-sonnet-5-5- In: 8
- Out: 1.2k
- Cache read: 129.7k
- Cache create: 10.2k
- Cost: ~$0.08 (est.)
- Duration: 15s
orchestrator-rollup— claude-sonnet-5-5- In: 4
- Out: 375
- Cache read: 59.8k
- Cache create: 3.1k
- Cost: ~$0.03 (est.)
- Duration: 9s
Summary
Adds
cr invites listandcr invites accept, so a profile's posting identity can accept pending repository invitations with the tokencralready holds. On GitHub, an approval counts toward a required review only when the reviewer identity has write access. Before this, granting that access meant someone had to take the reviewer token out of the credential store to accept the invitation.cr meresolves:reviewer_credentialswhen the profile declares them, otherwise the Git credential. Its live login is confirmed withWhoAmIbefore any invitation is read.acceptrequires at least one--owneror--repo. Without one it fails with a usage error before opening the credential store. Matching ignores case.accepted,would_acceptunder--dry-run,skipped, orfailed. Skipped and failed results carry a closedreason_codefor scripts plus a humanreason. Skips usenot_allowlistedorexpired. Failures are classified from the provider error:auth,permission,not_found,conflict,retryable,canceled,error. A failed accept does not stop the rest. The command prints every result, then exits 1 if any accept failed.gitprovider.RepositoryInvitationsis a new interface, separate fromGitProvider, so the review runtime wrappers never carry it.GET /user/repository_invitations(paginated) and a bodilessPATCH /user/repository_invitations/{id}, using the existing auth and transport.config.ErrUnsupported. GitLab adds existing users as members directly, so there is nothing to accept. Every adapter implements the interface and owns its unsupported decision.gitproviders.NewInvitationClientreturns the typed client, so the command needs no capability check.cmderr.Classifymaps config, provider and credential errors from one set of sentinel lists.cr meandcr invitesboth use it, replacing two hand-synced copies of the guard list.docs/architecture.mdrecords this as the one account-level write outside the ledger/outbox path. A new architecture test keeps every reference toAcceptRepositoryInvitation(calls, method values and method expressions) insideinternal/invitations, the package that enforces the allowlist and--dry-run.Tests
internal/invitations: the allowlist is required,--dry-runmakes no writes, accepts continue past a failure, expired and off-allowlist invitations are skipped, results are sorted.internal/gitprovider/github: pagination and field mapping against scrubbed fixtures intestdata/, the bodiless PATCH, status-to-error mapping (an echoed token in the body never reaches the error), ID validation, GitHub App rejection.internal/cmd/invitescmd:--dry-runmakes onlyWhoAmIand list calls;listandaccept;internal/cmd/noleak: the central command-surface audit now coversinvites list(text and JSON),invites accept --dry-run, a successfulaccept --json, and a 403 failure path.make checkpasses locally.Type of Change
Checklist
make tidypassesmake lintpassesmake testpassesmake buildpasses