Skip to content

feat(invites): accept repository invitations as the posting identity - #704

Merged
piekstra merged 4 commits into
mainfrom
feat/invites-accept
Nov 2, 2026
Merged

piekstra merged 4 commits into
mainfrom
feat/invites-accept

Conversation

@piekstra

@piekstra piekstra commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Summary

Adds cr invites list and cr invites accept, so a profile's posting identity can accept pending repository invitations with the token cr already 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 invites list   --profile <name> [--owner <login>]... [--repo <owner/name>]... [--json]
cr invites accept --profile <name> (--owner <login> | --repo <owner/name>)... [--dry-run] [--json]
  • Identity. The posting identity is the same one cr me resolves: reviewer_credentials when the profile declares them, otherwise the Git credential. Its live login is confirmed with WhoAmI before any invitation is read.
  • Allowlist. accept requires at least one --owner or --repo. Without one it fails with a usage error before opening the credential store. Matching ignores case.
  • Results. Every pending invitation gets one result: accepted, would_accept under --dry-run, skipped, or failed. Skipped and failed results carry a closed reason_code for scripts plus a human reason. Skips use not_allowlisted or expired. 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.
  • Provider seam. gitprovider.RepositoryInvitations is a new interface, separate from GitProvider, so the review runtime wrappers never carry it.
    • GitHub: GET /user/repository_invitations (paginated) and a bodiless PATCH /user/repository_invitations/{id}, using the existing auth and transport.
    • GitLab: returns 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.NewInvitationClient returns the typed client, so the command needs no capability check.
    • GitHub App credentials are rejected as unsupported, both in the command (before any keychain read) and in the adapter.
  • Exit codes. cmderr.Classify maps config, provider and credential errors from one set of sentinel lists. cr me and cr invites both use it, replacing two hand-synced copies of the guard list.
  • Write boundary. docs/architecture.md records this as the one account-level write outside the ledger/outbox path. A new architecture test keeps every reference to AcceptRepositoryInvitation (calls, method values and method expressions) inside internal/invitations, the package that enforces the allowlist and --dry-run.

Tests

  • internal/invitations: the allowlist is required, --dry-run makes 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 in testdata/, 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:
    • usage errors happen before the factory is called;
    • --dry-run makes only WhoAmI and list calls;
    • a partial failure exits 1 and still prints the full JSON;
    • golden JSON for list and accept;
    • the reviewer-credential source is used when declared;
    • GitHub App and GitLab profiles fail with exit 3 without opening credentials;
    • a real file credential store plus the real GitHub client never prints the token, even when the host echoes it back.
  • internal/cmd/noleak: the central command-surface audit now covers invites list (text and JSON), invites accept --dry-run, a successful accept --json, and a 403 failure path.

make check passes locally.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Repo scaffolding / maintenance

Checklist

  • I have read the contributing guide
  • My change follows the project structure and conventions
  • make tidy passes
  • make lint passes
  • make test passes
  • make build passes

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
piekstra-dev previously approved these changes Nov 2, 2026

@piekstra-dev piekstra-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 --owner row 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|accept flags (--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).

  1. 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.
  2. 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/gitprovider from 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 reason field 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: true doesn'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, leaving reason as 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

Comment thread internal/cmd/invitescmd/invitescmd.go Resolved
Comment thread README.md Resolved
Comment thread internal/cmd/invitescmd/invitescmd.go Resolved
Comment thread internal/invitations/invitations.go Resolved
Comment thread internal/gitprovider/invitations.go Resolved
- 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
piekstra-dev previously approved these changes Nov 2, 2026

@piekstra-dev piekstra-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 own retryable classification. 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_code this 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 on retryable won't retry it. The human reason text 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

Comment thread internal/invitations/invitations.go Resolved
Twapp-Session: 59d912f3-982f-402e-8bd8-3e31269a28b0

@piekstra-dev piekstra-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@piekstra
piekstra merged commit ddb49b0 into main Nov 2, 2026
12 checks passed
@piekstra
piekstra deleted the feat/invites-accept branch November 2, 2026 23:39
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.

2 participants