Skip to content

Add GitHub collaborator wire values - #17

Open
coenttb wants to merge 4 commits into
mainfrom
issue-16-github-collaborators-wire-values
Open

Add GitHub collaborator wire values#17
coenttb wants to merge 4 commits into
mainfrom
issue-16-github-collaborators-wire-values

Conversation

@coenttb

@coenttb coenttb commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Implements #16.

Adds transport-neutral compiled GitHub Standard values for collaborator roles, identities, list/check/add/permission requests and responses, and repository invitations. Existing owner, repository, user, pagination, timestamp, and URI values are composed rather than duplicated.

Verification: a fresh capped workspace package test --fresh --jobs 4 passed during implementation; the final exact head passed the same capped test command without --fresh after the invitation-ID refinement. No client, router, HTTP adapter, manifest, or legacy-tree changes are included.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

BLOCKED at exact head 68fb4f6. GitHub.Collaborators.Invitations.Update.Request exposes permission, but GitHub Update a repository invitation requires the body key permissions (plural). This L2 value is the later coding contract, so a client composing this request would send the wrong field and fail to update invitation access. Smallest fix: rename the stored property and initializer label to permissions, then add a focused assertion of that wire spelling. Reviewed the complete collaborator and invitation value surface and focused tests; the existing client/router trees remain untouched. Current CI is non-green and still running: formatting and SwiftLint failures annotate pre-existing paths outside this diff; the Swift-main nightly failure has no changed-path annotation. The source defect above is independently sufficient to block.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

BLOCKED at exact head 9c0fd3910ada1bd57bdb597028a6a9f9aa6ad534 (base de450191cafb3462519a9fdc82c5dc91f7f269ab).

The previous permissionpermissions correction is present, and a fresh capped workspace package test --fresh --jobs 4 passed (21 tests). However, GitHub.Collaborators.Invitations.Update.Request still relies on synthesized Codable. Its encoded body therefore includes owner, repository, and invitationID as well as permissions. Those are route values; the Update a repository invitation request body must contain only permissions. The focused test proves presence of permissions, but does not prove the routing fields are absent.

Smallest fix: give this request an explicit body-only encoding (or an equivalently narrow CodingKeys/encoder implementation) that emits only permissions, then extend the focused encoding test to assert that owner, repository, and invitationID are absent. The prior exact-head review blocker is otherwise resolved.

This source blocker is sufficient; no CI dispatch or landing action was taken. Next owner: Task #16's assignee.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewed at exact head fd72e97f0ff2c1319575b9146d85d25388ed40ca (base de450191cafb3462519a9fdc82c5dc91f7f269ab).

The prior route-field leakage is resolved: GitHub.Collaborators.Invitations.Update.Request explicitly encodes only permissions, retains its declared Codable decoding keys, and the focused test asserts the exact sole key plus absence of owner, repository, and invitationID. I found no remaining changed-surface source defect.

Fresh capped workspace package test --fresh --jobs 4 passed: 21 tests across 22 suites. The exact-head full-tier CI run remains the required off-machine landing gate; no duplicate dispatch was made.

@coenttb coenttb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewed at exact head 62162e7bb364f5489be8bf3d0e1699f926c254e2 (base de450191cafb3462519a9fdc82c5dc91f7f269ab).

This replacement is formatting-only in the collaborator test fixture. The route-body encoding repair and its exact-key/route-coordinate-absence test remain intact; I found no changed-surface source defect. Fresh capped workspace package test --fresh --jobs 4 passed: 21 tests across 22 suites.

The exact-head push CI run cleared the prior swift-format failure. Its remaining SwiftLint findings are in the pre-existing legacy client/type surface, outside this PR’s changed collaborator source; the nightly job is a compiler crash. The run is a nonterminal pull-request tier and is not closure evidence; no duplicate CI dispatch was made.

@coenttb

coenttb commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Diagnosis lane of coordinator session 09363bfe: outcome for PR #17 (head 62162e7bb364f5489be8bf3d0e1699f926c254e2).

Windows (Swift 6.3, debug) — already green (run 30744204332, 14m41s). No action needed; consistent with the #15 finding that the Windows leg's earlier red was infra flake, not a real defect.

SwiftLint — real finding, NOT covered by the #219 exemption. Same 10 unique violation sites as reported on #15/#14 (no_any_protocol_existential + typed_throws_required in Sources/GitHub Repositories Types/GitHub.Repositories.Client.swift and Sources/GitHub Traffic Types/GitHub.Traffic.Client.swift, on @Witness client closures typed throws(any Swift.Error)). Verified swift-institute/.github#219's exemption is scoped to stdlib Encodable/Decodable witnesses and the SwiftSyntaxMacros class only — it does not reach this repo's @Witness DI-macro pattern. git diff origin/main -- <those two files> is empty: pre-existing main drift, not introduced by #17's diff. A correct fix needs a concrete per-client error type replacing any Swift.Error across ~6 @Witness client files, which is API-breaking for out-of-repo Live conformers this lane cannot see — left unfixed as a blocker, matching the coordinator's L3 client-modularization / leaf-error-rulings scope rather than freelancing it here.

Ubuntu (Swift main nightly, release) — real, but a toolchain defect. Job 91486920717 crashes identically to #15's job 91483507008: swift-frontend: .../SILOptimizer/Utils/SILInliner.cpp:167: ... Assertion 'mdi.isNonEscaping()' failed, Swift 6.5-dev, while compiling the swift-binary-parser-primitives dependency. Not our code, not fixable from this repo. Distinct crash site from the tracked swift-institute/Issues#94 but the same nightly-compiler-crash class. Reporting as blocker.

Net: #17 is green on every leg except SwiftLint and Ubuntu main-nightly, both pre-existing main drift/toolchain issues shared identically with #15.

@coenttb

coenttb commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Triage: substantive PR (GitHub collaborator wire values). CI at head 62162e7 has real failures — ci / matrix / ci-ok, ci / ci-ok, ci / matrix / SwiftLint, and Ubuntu (Swift main nightly, release) all FAILURE — alongside many SUCCESS jobs. Not trivial (adds typed wire-value surface), so outside merge authority regardless of CI. Not vestigial or superseded. Flagging as NEEDS-DECISION for coordinator/principal (and CI triage) — no merge, no close.

— peer task bookkeeping-pr-triage (coordinator session e04b89ef)

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.

1 participant