Centralize repository identity resolution by mariusvniekerk · Pull Request #227 · wesm/middleman

mariusvniekerk · 2026-05-01T21:51:54Z

Summary

add a repository identity resolver for host-qualified owner/name lookups and item-number resolution
route repo-bound API handlers through stable repo IDs before loading pull requests or issues
document the repository identity boundary for future handler work

How this makes the code better

removes repeated owner/name/platform_host resolution from handlers and legacy helpers
gives all repo-bound callers the same ambiguity, not-found, and host fallback behavior
prevents future route code from accidentally mixing same owner/name repositories across hosts

roborev-ci · 2026-05-01T21:55:07Z

roborev: Combined Review (`d8e8f29`)

Needs changes before merge.

High

internal/server/huma_routes.go:1129 — The refactor changes repository resolution for user-visible maintainer actions, including create issue, issue comments, starred items, and PR/issue item lookup, but the diff only adds DB/module unit tests. There is no e2e API + SQLite coverage proving the HTTP handlers return the intended 400/404 behavior for ambiguous owner/name rows or correctly honor platform_host when resolving items by repo_id.

Fix: Add e2e tests that exercise the real HTTP API and SQLite database for ambiguous same owner/name repos, explicit platform_host success, and repo_id-scoped item resolution.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

mariusvniekerk · 2026-05-01T22:06:47Z

Why do we support anything other than repoLookupRequireUnambiguousOwnerName at all? feels like we can correctly determin the correct name for things at all times, so no guessing should be needed here.

roborev-ci · 2026-05-01T22:37:23Z

roborev: Combined Review (`ca95898`)

Medium-risk ambiguity handling gaps remain in repository identity resolution.

Medium

Location: internal/server/huma_routes.go:2178
Problem: /items/{number}/resolve only detects ambiguity from local DB rows. If duplicate owner/name repos are tracked but matching rows are not in SQLite yet, the sync fallback can call SyncItemByNumber and choose a host via hostFor instead of rejecting the ambiguous no-host request.
Fix: Check tracked repo ambiguity before sync fallback, or add a syncer-level resolver that requires explicit platform_host for ambiguous tracked repos.
Location: internal/server/helpers.go:69, internal/server/repo_identity.go, context/repository-identity.md:24
Problem: The docs describe separate resolver modes for legacy owner/name lookups versus strict ambiguous lookups, but the implementation appears to use strict ambiguity errors broadly. Legacy helpers/routes that do not accept platform_host may now skip updates or return generic 500s instead of preserving documented fallback behavior or returning a clear 400.
Fix: Implement separate legacy and strict resolver modes, and map errRepoAmbiguous to 400 Bad Request in affected handlers such as those relying on lookupMRID, lookupIssueID, and lookupIssue.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-01T23:16:31Z

roborev: Combined Review (`6fd174a`)

High-level verdict: PR mutation endpoints still bypass cross-host disambiguation and can update or mutate the wrong repository.

High

internal/server/huma_routes.go:1579
internal/server/huma_routes.go:1647

readyForReview and mergePR call lookupRepo(..., "") for local DB updates. When the same owner/name exists on multiple hosts, this returns errRepoAmbiguous; one path silently skips the update and the other ignores the error. More importantly, the mutation can proceed through syncer/GitHub logic without an explicit platform_host, risking mutation of the wrong host’s PR and leaving SQLite stale after successful actions.

Fix: Require or propagate the actual platform_host for PR mutation endpoints, resolve repository identity before calling the syncer, and return HTTP 400 when a request is ambiguous without an explicit host.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-01T23:42:47Z

roborev: Combined Review (`10d8d76`)

Summary verdict: Ambiguous repo handling is incomplete, leaving several PR/issue mutation paths able to target the wrong host or fail silently.

High

Location: internal/server/huma_routes.go:2006
Problem: syncIssue calls SyncIssue before rejecting ambiguous no-host requests. With two tracked repos sharing owner/name across hosts, /issues/{n}/sync can sync and persist data from the wrong host before the later identity lookup returns 400.
Fix: Check platformHost == "" && s.syncer.IsTrackedRepoAmbiguous(owner, name) before any sync call and return 400, or resolve the repo identity first and call SyncIssueOnHost.
Location: internal/server/huma_routes.go in mergePR and readyForReview
Problem: These handlers call lookupRepo with an empty platformHost. If the repo is ambiguous, lookupRepo returns repoidentity.ErrAmbiguous, but the handlers ignore or only conditionally handle the error, causing local DB state updates to be silently skipped.
Fix: Add platform_host to the relevant PR input structs, return 400 for ambiguous repo identity, and use the resolved host for local and GitHub-side mutations.

Medium

Location: internal/server/huma_routes.go:37
Problem: PR routes still use repoNumberInput without platform_host, while lookupMRID now requires a unique owner/name. Ambiguous repos make local PR actions such as kanban updates fail with no way to disambiguate, and some PR mutation handlers may call GitHub through first-match ClientForRepo before strict lookup fails.
Fix: Add platform_host to PR route inputs, pass it into repoNumberPathRef, map ambiguity to HTTP 400, and use host-specific GitHub/sync paths for PR mutations.
Location: e2e tests
Problem: The change introduces core API behavior around rejecting ambiguous repository lookups, but there is no end-to-end coverage proving ambiguous requests return 400 or explicit platform_host requests succeed.
Fix: Add e2e tests for ambiguous no-host requests and successful host-qualified requests.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T04:36:11Z

roborev: Combined Review (`ee2ee78`)

Summary verdict: Host disambiguation is incomplete in the SPA, so ambiguous repositories can still fail core maintainer actions.

High

internal/server/huma_routes.go:1496, frontend/
The backend/OpenAPI changes require platform_host for ambiguous owner/name repositories, but the SPA action flow does not appear to pass the selected repo host into ready-for-review, merge, workflow approval, or item resolve requests. Users interacting with ambiguous repositories from the UI will still send no-host requests and receive 400 Bad Request.

Fix: Thread platform_host through PR/issue list and detail state, then include it in affected frontend API calls, including components such as MergeModal.svelte and ReadyForReviewButton.svelte.

Medium

internal/server/huma_routes.go:1609
mergePR and readyForReview select a GitHub client before resolving the local repository identity, then perform the mutation before lookupRepo can reject a stale ambiguous local owner/name. This can mutate GitHub successfully while the API returns an error and skips the local state update.

Fix: Resolve the repo identity, including ambiguity handling, before calling GitHub for PR mutations. Use the resolved host/repo ID for both client selection and local updates.
internal/server/huma_routes.go:1410, frontend/tests/e2e/
The host-specific maintainer workflows lack full-stack e2e coverage. In particular, there should be coverage that ambiguous no-host requests are rejected before approval and that explicit platform_host routes workflow approval, item resolution, and PR mutations to the intended host.

Fix: Add Playwright coverage with ambiguous repositories verifying successful UI actions when the explicit platform host is present.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T05:18:23Z

roborev: Combined Review (`8200086`)

Host-aware repository identity is mostly improved, but several PR detail/navigation paths still drop platform_host and can misroute ambiguous repos.

High

packages/ui/src/components/detail/PullDetail.svelte:783
The Approve action still renders without platformHost, while other PR detail actions preserve it. On host-qualified or ambiguous repos, approving a PR can call the unqualified /approve endpoint and target the wrong host or fail.
Fix: Add platformHost support to ApproveButton, pass detail.platform_host, and update the approve API/backend path to accept and use platform_host.

Medium

internal/server/huma_routes.go:641
getPull still uses the unqualified owner/name lookup when platform_host is omitted, bypassing the strict repository identity resolver. Ambiguous same-owner/name repos can return an arbitrary or wrong PR instead of HTTP 400.
Fix: Resolve the repo through repoIdentity().LookupRepo for both host-qualified and unqualified requests, return 400 for ErrAmbiguous, then load the PR by repo_id + number.
frontend/src/lib/utils/itemRefHandler.ts:57
Resolved PR item references drop platformHost by passing it only when data.item_type === "issue". Clicking #123 from a host-qualified PR to another PR loses host context and can navigate to the wrong repo or an ambiguous route.
Fix: Pass platformHost to buildItemRoute for both PR and issue resolutions.
packages/ui/src/utils/markdown.ts:90
Markdown rendering now depends on platformHost, but the render/Marked cache key does not appear to include it. Identical markdown for the same owner/name can reuse cached HTML without the correct data-platform-host, making item links host-dependent on render order.
Fix: Include platformHost in both markdown HTML and Marked-extension cache keys.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T05:25:44Z

roborev: Combined Review (`15b3390`)

The PR still has host-qualified repository identity gaps that can break ambiguous owner/name pull request flows.

High

[packages/ui/src/components/detail/PullDetail.svelte:798] Approve actions still do not pass platformHost, and the approve endpoint/client does not support platform_host. On host-ambiguous repositories, approving a host-qualified PR can target the unqualified/default repo or fail while other PR actions work.
- Fix: Add platform_host support to the approve route/client, pass detail.platform_host into ApproveButton, and cover it in the host-qualified PR action e2e test.
[internal/server/huma_routes.go:640] getPull bypasses strict repo identity resolution when platform_host is omitted by falling back to s.db.GetMergeRequest. Ambiguous owner/name PR detail requests can return an arbitrary matching repo instead of HTTP 400.
- Fix: Resolve the repository through repoIdentity().LookupRepo or equivalent for all getPull requests, map ambiguity to HTTP 400, then fetch by repo_id + number.

Medium

[frontend/src/lib/utils/itemRefHandler.ts:57] Item-reference navigation drops platformHost when the resolved item is a PR. Clicking a PR reference from a host-qualified or ambiguous repo can navigate to an unqualified /pulls/... route and lose repository identity.
- Fix: Pass platformHost through to buildItemRoute for PRs as well as issues.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T11:57:11Z

roborev: Combined Review (`951533d`)

Medium finding remains: kanban status updates are still not host-qualified.

Medium

packages/ui/src/stores/detail.svelte.ts (updateKanbanState) / internal/server/huma_routes.go (setKanbanStateInput)
Kanban status updates still do not include platform_host. For duplicate owner/name repositories across hosts, changing a host-qualified PR’s status can fail ambiguity resolution or update the wrong local row.

Fix: Add platform_host to the kanban-state API contract and pass the selected PR/detail platform_host from the UI, including board and detail flows.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T12:10:25Z

roborev: Combined Review (`5ff56d0`)

Host-qualified PR support is incomplete; several mutation and rendering paths can still target the wrong host or fail on ambiguous repositories.

High

internal/server/huma_routes.go:901, internal/server/huma_routes.go:1741, internal/server/huma_routes.go PR/issue comment, edit PR description, and PR state handlers
Several maintainer mutation endpoints still resolve GitHub clients and local rows by owner/name only, or perform GitHub side effects before ambiguity is rejected. This affects PR title/body edits, PR and issue comments, and close/reopen actions, allowing duplicate owner/name repos across hosts to mutate the wrong repo or fail ambiguously.
Fix: Add and consistently use platform_host for these inputs, resolve the repo before any GitHub call with host-aware client selection, and update/read rows by repo_id + number.
frontend/src/components/detail/PullDetail.svelte around line 307
The close/reopen PR action sends platform_host in the request body for /repos/{owner}/{name}/pulls/{number}/github-state, while the host-qualified convention in this change expects it in query parameters. The backend may ignore it or reject the request.
Fix: Move platform_host into the query object for that client.POST call.

Medium

internal/server/huma_routes.go:2473
Files/diff endpoints remain host-unaware. getDiff and getFiles fetch diff SHAs by owner/name/number and choose clone host via HostForRepo, so duplicate-host repos can show files from the wrong repository.
Fix: Add platform_host to commits/diff/files APIs and frontend calls, then resolve repo identity and load diff data by repo ID/host.
packages/ui/src/stores/pulls.svelte.ts:278
PR star toggles still send /starred bodies without platform_host. Host-qualified PR list/detail views can fail or be unable to target the intended host.
Fix: Thread platformHost through star toggles and include platform_host in the request body.
packages/ui/src/utils/markdown.ts:77
Markdown renderer caches Marked instances and rendered HTML by owner/name only while the tokenizer closes over repo.platformHost. Rendering the same owner/name on different hosts can reuse cached HTML with the wrong data-platform-host.
Fix: Include platformHost in both markedCache and htmlCache keys.
packages/ui/src/utils/markdown.ts:95
DOMPurify allows user-authored raw HTML to provide internal routing attributes including data-platform-host. A commenter could inject an .item-ref anchor that navigates the maintainer to a different host-qualified repo/item.
Fix: Do not allow raw markdown HTML to supply internal routing attributes, or only trust item-ref anchors generated by the renderer with an unforgeable marker.
frontend/tests/e2e/host-qualified-pr-actions.spec.ts
The e2e coverage does not include commenting, editing descriptions, or close/reopen flows for host-qualified PRs and issues. These are core maintainer actions affected by this change.
Fix: Add e2e cases for those host-qualified actions.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T12:37:44Z

roborev: Combined Review (`15fec76`)

Host identity handling still has two medium-risk gaps that can route maintainer actions to the wrong repository host.

Medium

internal/server/huma_routes.go:1908
setIssueGitHubState ignores the new platform_host query parameter and resolves with input.Body.PlatformHost instead. Generated/OpenAPI clients now send platform_host as a query parameter, so close/reopen requests can be resolved as hostless and mutate the wrong same-owner/name issue when only one local row matches.

Remediation: resolve issue state using strings.TrimSpace(input.PlatformHost), or support a single host source and remove the body fallback after callers are migrated.
internal/repoidentity/repoidentity.go:65
Hostless strict repo resolution only checks SQLite rows via ListReposByOwnerName; it does not consider configured tracked repos. If config tracks the same owner/name on multiple hosts but SQLite currently has one row, hostless mutation handlers can proceed instead of rejecting the request as ambiguous.

Remediation: include configured tracked repos in strict resolution, or have hostless single-repo actions reject when syncer.IsTrackedRepoAmbiguous(owner, name) is true before DB-only resolution.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T12:43:20Z

roborev: Combined Review (`3ce2488`)

Summary verdict: Changes need fixes before merge due to host ambiguity and issue state routing gaps.

High

internal/server/huma_routes.go:1481
PR mutation handlers resolve ambiguity only from SQLite rows via lookupRepo. If config tracks the same owner/name across multiple hosts but only one host has a local repo row, no-host actions can still approve, comment, close, or merge the wrong host.
Fix: For no-host PR actions, reject when syncer.IsTrackedRepoAmbiguous(owner, name) is true before selecting a repo, or make repository identity resolution include configured repos.

Medium

internal/server/huma_routes.go:1902
setIssueGitHubState ignores the new platform_host query parameter and still reads input.Body.PlatformHost, while OpenAPI/generated clients now send it as query.
Fix: Use strings.TrimSpace(input.PlatformHost) for issue state resolution, optionally falling back to the body field for compatibility.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T12:53:18Z

roborev: Combined Review (`8a25059`)

Host-qualified issue state updates still drop platform_host on one mutation path.

Medium

internal/server/huma_routes.go:1908 - setIssueGitHubState still resolves repository identity from input.Body.PlatformHost, even though the route/OpenAPI now exposes platform_host as a query parameter. A generated client request like ...?platform_host=ghe.example.com is treated as unqualified, which can return 400 for ambiguous repos or target the wrong local repo when only one matching row is present.

Fix: Resolve issues from strings.TrimSpace(input.PlatformHost), optionally falling back to input.Body.PlatformHost for compatibility. Add a full-stack issue close/reopen test using the query parameter.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-05-02T13:07:22Z

roborev: Combined Review (`9a996ba`)

Host identity handling has two High severity issues that can route maintainer actions to the wrong repository host.

High

internal/server/huma_routes.go:1908 / internal/server/huma_routes.go:1909
setIssueGitHubState declares platform_host as a query parameter, and generated clients send it there, but the handler resolves using input.Body.PlatformHost. Host-qualified issue close/reopen requests can ignore the requested host and mutate the wrong locally matching issue.
Fix: Resolve with strings.TrimSpace(input.PlatformHost), optionally falling back to input.Body.PlatformHost for backward compatibility. Add coverage for issue state mutation with query platform_host.
internal/repoidentity/repoidentity.go:66
LookupRepo treats no-host owner/name as unambiguous when the DB has only one matching row. Repo-scoped PR mutation handlers relying on this resolver can mutate the wrong host when config tracks duplicate owner/name repos across hosts but SQLite has only one synced row.
Fix: Include configured tracked repos in strict identity resolution, or precheck syncer.IsTrackedRepoAmbiguous(owner, name) for repo-scoped mutation/detail actions when platform_host is omitted. Add tests for ambiguous configured repos with only one matching local DB row.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Repository host, owner/name fallback, ambiguity, not-found, and item-number lookup rules were spread across route helpers and handlers, which made each caller decide a slightly different slice of repository identity semantics. This deepens the backend module boundary so handlers resolve a repo once, then use stable repo IDs for pull request and issue item lookups. The result keeps same owner/name rows across hosts from leaking through item resolution and gives mutations one place to enforce platform_host when owner/name is ambiguous, improving calling uniformity for repo-bound handlers. Validation: go test ./internal/db -shuffle=on; go test ./internal/server -run 'TestRepositoryIdentity|TestAPICreateIssue|TestAPIResolveItem|TestAPISetStarred|TestAPIUnsetStarred|TestAPIGetIssue|TestAPIPostIssueComment|TestAPIEditIssueComment|TestAPIListPullsAcceptsHostQualifiedRepoFilter' -shuffle=on; git diff --check. Full go test ./internal/server -shuffle=on was attempted and failed in unrelated tmux process-leak stress coverage under host process pressure. Co-authored-by: OpenAI Codex <noreply@openai.com>

Owner/name repo lookup now has one rule: it succeeds only when the tuple is unambiguous, otherwise callers must provide platform_host. This removes the permissive lookup mode and keeps host selection explicit across item resolution and generated clients. Validation: go test ./internal/server -run 'TestResolveItem_(PR|Issue|AmbiguousOwnerNameRequiresPlatformHost|ExplicitPlatformHostResolvesRepoScopedItem|UntrackedRepo|NotFoundOnGitHub|GitHubServerError)$|TestRepositoryIdentity' -shuffle=on; go test ./internal/github -shuffle=on.

Resolve item now checks configured repo ambiguity before falling through to no-host sync, so duplicate owner/name repos cannot be silently resolved via hostFor. Issue routes also map ambiguous repo input to 400 and the context doc records the strict identity rule.

Repository identity resolution now lives in internal/repoidentity with a small persistence interface and package-level tests. The server keeps only route-shape adapters, so ambiguity, not-found handling, and repo-id item lookup have one shared implementation.

Prevent ready-for-review, workflow approval, merge, and issue sync paths from choosing the first tracked host when owner/name is ambiguous. Host-qualified PR actions now use host-specific syncer helpers and the API spec exposes platform_host on the affected mutation routes.\n\nVerified with:\n- GOCACHE=/tmp/middleman-gocache go test ./internal/server -run 'Test(API(SyncIssueRejectsAmbiguousTrackedRepoBeforeSync|MergePRRejectsAmbiguousRepoBeforeMutation|MergePRUsesPlatformHostQuery|ReadyForReviewRejectsAmbiguousRepoBeforeMutation)|ResolveItem_(AmbiguousOwnerNameRequiresPlatformHost|AmbiguousTrackedRepoRequiresPlatformHostBeforeSync|ExplicitPlatformHostResolvesRepoScopedItem))' -shuffle=on\n- GOCACHE=/tmp/middleman-gocache go test ./internal/github -shuffle=on\n- git diff --check\n\nFull internal/server shuffle was attempted but hit unrelated resource-sensitive tmux/workspace failures in the test environment.

Thread platform_host through PR route state, detail loads, action requests, and markdown item references so ambiguous owner/name repos keep their selected host across mutations. Resolve PR repo identity before GitHub mutations and add Playwright coverage for host-qualified PR ready, workflow approval, merge, and item-reference resolution flows. Verified with: bun run typecheck (packages/ui); bun run typecheck (frontend); bun run test src/ReadyForReviewButton.test.ts src/ApproveWorkflowsButton.test.ts; bun run test:e2e:mock tests/e2e/host-qualified-pr-actions.spec.ts; go test ./internal/server -run TestWorkspacePRDetailPlatformHost|TestAPI... -shuffle=on; go test ./internal/github -shuffle=on; git diff --check

Preserve platform_host for PR selections in the Activity drawer URL state so duplicate detail loads dedupe under the same host-qualified identity. Update ready-for-review e2e assertions to expect the host-qualified mutation URL now emitted by the PR action controls. Verified with: bun run typecheck (frontend); bun run test src/lib/utils/activitySelection.test.ts src/ReadyForReviewButton.test.ts src/ApproveWorkflowsButton.test.ts; bun run test:e2e tests/e2e-full/activity-drawer.spec.ts --project=chromium -g 'duplicate load stalls'; bun run test:e2e tests/e2e-full/detail-action-buttons.spec.ts --project=chromium -g 'ready for review updates API state|self-contained actions close'; git diff --check

Require ambiguous PR detail and approval requests to resolve repository identity before mutation, and thread platform_host through the approve UI and PR item-reference navigation. Verified with: - go test ./internal/server -run 'TestAPI(GetPullAmbiguousOwnerNameRequiresPlatformHost|GetPullUsesPlatformHostQuery|ApprovePRRejectsAmbiguousRepoBeforeMutation|ApprovePRUsesPlatformHostQuery)$' -shuffle=on - go test ./internal/server -run 'Test(API(GetPullAmbiguousOwnerNameRequiresPlatformHost|GetPullUsesPlatformHostQuery|ApprovePRRejectsAmbiguousRepoBeforeMutation|ApprovePRUsesPlatformHostQuery)|WorkspacePRDetailPlatformHost)$' -shuffle=on - bun run test src/ApproveButton.test.ts src/lib/utils/itemRefHandler.test.ts - bun run test:e2e:mock tests/e2e/host-qualified-pr-actions.spec.ts --project=chromium - bun run typecheck - bun run build - git diff --check Note: go test ./internal/server -shuffle=on also hit an unrelated tmux stress/process-limit failure in TestWorkspaceListTmuxActivityStressDoesNotLeakProcesses. 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: OpenAI Codex <noreply@openai.com>

Add platform_host to the kanban state endpoint and preserve host identity through PR detail, drawer, and board kanban update flows so duplicate owner/name repositories update the intended row. Verified with: - go test ./internal/server -run 'TestAPISetKanbanState|TestAPISetKanbanStateRejectsInvalidStatus|TestAPISetKanbanStateRejectsAmbiguousRepo|TestAPISetKanbanStateUsesPlatformHostQuery' -shuffle=on - bun run test src/lib/stores/detail-comment.svelte.test.ts - bun run typecheck - bun run test:e2e:mock tests/e2e/host-qualified-pr-actions.spec.ts --project=chromium - bun run build - git diff --check 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: OpenAI Codex <noreply@openai.com>

Reject ambiguous PR routes before mutating GitHub, carry platform_host through comments, state changes, content edits, starring, and diff reads, and keep generated markdown item refs distinct from sanitized raw HTML. Verified with: - go test ./internal/server -shuffle=on - bun run typecheck - bun run test src/lib/stores/detail-comment.svelte.test.ts src/lib/utils/itemRefHandler.test.ts src/lib/stores/diff.svelte.test.ts src/lib/stores/diff-scope.test.ts - bun run test:e2e:mock tests/e2e/host-qualified-pr-actions.spec.ts --project=chromium - bun run build - git diff --check Note: npx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/detail/PullDetail.svelte --svelte-version 5 hung and was stopped. 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: OpenAI Codex <noreply@openai.com>

Update CommentBox persistence expectations for the platform_host argument now passed through PR comment submission. Verified with: - bun run test src/lib/components/detail-comment-persistence.test.ts - bun run typecheck - git diff --check 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: OpenAI Codex <noreply@openai.com>

Allow e2e state-change route mocks to match platform_host query strings so failure-path tests do not fall through and mutate the shared backend state. Verified with: - bun run test:e2e tests/e2e-full/detail-action-buttons.spec.ts tests/e2e-full/activity-drawer.spec.ts --project=chromium - git diff --check 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: OpenAI Codex <noreply@openai.com>

Install the PR files route before opening the detail view so the diff summary test intercepts the host-qualified files request instead of racing the real endpoint.\n\nVerified with:\n- bun run test:e2e tests/e2e-full/pr-detail-branches.spec.ts --project=chromium\n- git diff --check\n\n🤖 Generated with [OpenAI Codex](https://openai.com/codex)\nCo-authored-by: OpenAI Codex <noreply@openai.com>

Resolve issue state changes from the query platform_host used by generated clients, while preserving the legacy body fallback. Reject unqualified repo lookups when configured tracked repositories are ambiguous even if SQLite currently has only one matching row, preventing maintainer mutations from guessing a host.\n\nVerified with:\n- go test ./internal/server -run 'TestAPISetIssueStateUsesPlatformHostBody|TestAPISetIssueStateUsesPlatformHostQuery|TestAPIApprovePRRejectsAmbiguousRepoBeforeMutation|TestAPIApprovePRRejectsAmbiguousTrackedRepoBeforeMutation' -shuffle=on\n- go test ./internal/server -shuffle=on\n- git diff --check\n\n🤖 Generated with [OpenAI Codex](https://openai.com/codex)\nCo-authored-by: OpenAI Codex <noreply@openai.com>

roborev-ci · 2026-05-02T15:44:07Z

roborev: Combined Review (`202cade`)

High-risk ambiguity bypass remains in host-qualified mutations; one additional medium trust-boundary issue exists in markdown item refs.

High

internal/server/huma_routes.go:1159 and internal/server/huma_routes.go:2447
createIssue and lookupStarredRepoID call repoIdentity().LookupRepo directly, bypassing the configured-repo ambiguity guard in lookupRepo / requireTrackedRepoUnambiguous. If two configured repos share owner/name across hosts but only one local SQLite row exists, a request without platform_host can mutate the wrong GitHub host instead of returning 400.

Fix: Route these paths through lookupRepo, or call requireTrackedRepoUnambiguous before direct repo identity lookups. Add API e2e coverage for ambiguous configured repos where only one DB row exists, asserting no create/star mutation occurs.

Medium

packages/ui/src/utils/markdown.ts:92 and frontend/src/lib/utils/itemRefHandler.ts:10
Rendered markdown allows data-middleman-item-ref="true", and the click handler treats that marker as trusted. A PR author can include raw HTML with attacker-controlled data-owner, data-name, data-number, and data-platform-host; when clicked, the app resolves and navigates to the attacker-selected tracked item.

Fix: Strip data-middleman-item-ref from sanitized markdown and add it only during trusted post-processing, disable raw HTML, or render generated item refs as structured trusted components.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Remove a one-use helper and keep the issue state platform_host fallback visible at the handler call site. Behavior is unchanged: query platform_host still wins, with the body value retained as compatibility fallback.\n\nVerified with:\n- go test ./internal/server -run 'TestAPISetIssueStateUsesPlatformHostBody|TestAPISetIssueStateUsesPlatformHostQuery|TestAPIApprovePRRejectsAmbiguousRepoBeforeMutation|TestAPIApprovePRRejectsAmbiguousTrackedRepoBeforeMutation' -shuffle=on\n- git diff --check\n\n🤖 Generated with [OpenAI Codex](https://openai.com/codex)\nCo-authored-by: OpenAI Codex <noreply@openai.com>

roborev-ci · 2026-05-02T15:55:50Z

roborev: Combined Review (`3b1d057`)

High severity issue remains: repo-scoped mutations can resolve the wrong host when configured repos are ambiguous.

High

internal/server/huma_routes.go:1157, internal/server/huma_routes.go:2457
createIssue and lookupStarredRepoID call repoIdentity() directly, which only checks SQLite rows. If config tracks both github.com/acme/widget and ghe.example.com/acme/widget but SQLite currently has only one row, a no-host request can still resolve and mutate/star the wrong host instead of returning 400.

Fix: Run requireTrackedRepoUnambiguous first, or route these through the same lookupRepo/host-aware helper path used by the other repo-scoped actions. Add e2e coverage for configured-but-not-yet-synced ambiguity.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Move platform_host off JSON mutation query parameters and into required request body fields so repository identity is carried with the payload. Regenerate API clients and update host-qualified UI actions to send the new shape. Verified with: - go test ./internal/server -run 'TestAPI(SetKanbanState|SetIssueState|ApprovePR|MergePR|EditPR|EditPrComment|EditIssueComment|PostPrComment)' -shuffle=on - bun run typecheck - bun run test src/ApproveButton.test.ts src/ReadyForReviewButton.test.ts src/ApproveWorkflowsButton.test.ts src/lib/stores/detail-comment.svelte.test.ts - bun run test:e2e:mock tests/e2e/host-qualified-pr-actions.spec.ts --project=chromium - git diff --check Note: go test ./internal/server -shuffle=on still fails in TestWorkspaceListTmuxActivityStressDoesNotLeakProcesses due to a tmux/process stress violation unrelated to this API contract change. 🤖 Generated with [OpenAI Codex](https://openai.com/codex) Co-authored-by: OpenAI Codex <noreply@openai.com>

Centralize repeated lookup and scanning paths that grew while adding host-qualified repository identity. This keeps the behavior anchored in one place so future host-aware actions are less likely to drift in status mapping, tracked-repo checks, or merge-request row scans.\n\nValidation:\n- go test ./internal/db ./internal/github ./internal/server -shuffle=on\n- git diff --check Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>

Host-qualified repository identity needs to hold beyond API calls. Duplicate repos with the same owner/name/number could be shown as the same selected activity item or collapsed into one commit run when only the platform host distinguished them.\n\nThe tests now use duplicate-host examples that fail if PR selection, commit-run collapse, or item-ref navigation drops platform_host.\n\nValidation:\n- bun run --cwd frontend typecheck\n- bun run --cwd frontend test ../packages/ui/src/components/ActivityFeed.test.ts ../packages/ui/src/components/activityRows.test.ts src/lib/utils/itemRefHandler.test.ts\n- git diff --check Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>

Repository identity is platform_host plus owner/name, so the repoidentity module should never perform hostless owner/name guessing. Server handlers now derive a concrete host from unambiguous tracked configuration before calling the module and reject ambiguous hostless mutations before side effects.\n\nThis closes the configured-but-not-yet-synced ambiguity path for create-issue and starred actions, where a single local SQLite row could previously hide multiple tracked repos sharing owner/name.\n\nValidation:\n- go test ./internal/repoidentity ./internal/server -run 'TestLookupRepoRequiresPlatformHost|TestRepositoryIdentityRequiresPlatformHost|TestAPICreateIssueRejectsAmbiguousTrackedRepoBeforeMutation|TestAPISetStarredRejectsAmbiguousTrackedRepo|TestAPICreateIssueUsesPlatformHost|TestAPISetStarred$|TestAPIUnsetStarred|TestResolveItem' -shuffle=on\n- go test ./internal/repoidentity ./internal/server -run 'TestRepositoryIdentity|TestLookup|TestResolveLocalItem|TestGetIssue|TestAPI(GetPull|SetKanbanState|ReadyForReview|ApprovePR|MergePR|SetPRGitHubState|SetIssueGitHubState|CreateIssue|SetStarred|UnsetStarred|PostPrComment|EditPrComment|EditPRContent|ResolveItem)' -shuffle=on\n- go test ./internal/repoidentity ./internal/server -short -shuffle=on\n- git diff --check\n\nNote: full go test ./internal/server -shuffle=on currently fails in TestWorkspaceDeleteDirtyKeepsRuntimeSessionsE2E, an existing workspace runtime test unrelated to repository identity. Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>

roborev-ci · 2026-05-03T04:35:19Z

roborev: Combined Review (`eaa53ef`)

Medium issue found: one e2e assertion still expects the old request shape.

Medium

frontend/tests/e2e-full/detail-action-buttons.spec.ts:558
The ready-for-review e2e assertions now wait for platform_host in the URL query, but the updated UI/API sends platform_host in the JSON request body for this endpoint. The wait predicate will never match the real response.

Suggested fix: assert against response.request().postDataJSON().platform_host, or change the client/API contract to use the query consistently.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Ready-for-review now carries platform_host in the mutation body. The full-stack action-menu tests were still waiting for a query parameter, so CI could miss the actual host-preservation contract and time out on the response matcher.\n\nValidation:\n- bun run --cwd frontend test:e2e tests/e2e-full/detail-action-buttons.spec.ts --project=chromium -g 'self-contained actions close|ready for review updates API state'\n- git diff --check Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>

roborev-ci · 2026-05-03T04:43:20Z

roborev: Combined Review (`4c70acf`)

Medium-risk issue found: sanitized raw HTML can forge trusted item-reference navigation attributes.

Medium

packages/ui/src/utils/markdown.ts:96
The new trusted data-middleman-item-ref="true" marker is allowed through DOMPurify, so raw PR/comment HTML can forge the marker along with data-owner, data-name, and data-number, then trigger item-resolution navigation as if it came from the generated markdown renderer.
Fix: Strip data-middleman-item-ref during sanitization and add it only after sanitization to anchors produced by the item-ref renderer, or use a rendering path where raw HTML cannot create trusted item-ref attributes.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Raw markdown HTML should not be able to opt into Middleman item-reference navigation by forging data-middleman-item-ref. Generated item refs now carry a per-render internal token through sanitization, and only those anchors receive the trusted marker after raw HTML has been scrubbed.\n\nValidation:\n- bun run --cwd frontend typecheck\n- bun run --cwd frontend test ../packages/ui/src/utils/markdown.test.ts src/lib/utils/itemRefHandler.test.ts\n- git diff --check Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>

roborev-ci · 2026-05-03T04:53:47Z

roborev: Combined Review (`09a165e`)

No Medium, High, or Critical issues were reported.

All review outputs are clean at the requested severity threshold.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

mariusvniekerk marked this pull request as draft May 1, 2026 23:51

mariusvniekerk force-pushed the architecture-repo-identity branch from 10d8d76 to ee2ee78 Compare May 2, 2026 04:33

mariusvniekerk and others added 14 commits May 2, 2026 11:43

mariusvniekerk force-pushed the architecture-repo-identity branch from 202cade to 99a04cd Compare May 2, 2026 15:47

mariusvniekerk and others added 4 commits May 3, 2026 00:30

Conversation

mariusvniekerk commented May 1, 2026

Summary

How this makes the code better

Uh oh!

roborev-ci Bot commented May 1, 2026

roborev: Combined Review (d8e8f29)

High

Uh oh!

mariusvniekerk commented May 1, 2026

Uh oh!

roborev-ci Bot commented May 1, 2026

roborev: Combined Review (ca95898)

Medium

Uh oh!

roborev-ci Bot commented May 1, 2026

roborev: Combined Review (6fd174a)

High

Uh oh!

roborev-ci Bot commented May 1, 2026

roborev: Combined Review (10d8d76)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (ee2ee78)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (8200086)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (15b3390)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (951533d)

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (5ff56d0)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (15fec76)

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (3ce2488)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (8a25059)

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (9a996ba)

High

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (202cade)

High

Medium

Uh oh!

roborev-ci Bot commented May 2, 2026

roborev: Combined Review (3b1d057)

High

Uh oh!

roborev-ci Bot commented May 3, 2026

roborev: Combined Review (eaa53ef)

Medium

Uh oh!

roborev-ci Bot commented May 3, 2026

roborev: Combined Review (4c70acf)

roborev: Combined Review (`d8e8f29`)

roborev: Combined Review (`ca95898`)

roborev: Combined Review (`6fd174a`)

roborev: Combined Review (`10d8d76`)

roborev: Combined Review (`ee2ee78`)

roborev: Combined Review (`8200086`)

roborev: Combined Review (`15b3390`)

roborev: Combined Review (`951533d`)

roborev: Combined Review (`5ff56d0`)

roborev: Combined Review (`15fec76`)

roborev: Combined Review (`3ce2488`)

roborev: Combined Review (`8a25059`)

roborev: Combined Review (`9a996ba`)

roborev: Combined Review (`202cade`)

roborev: Combined Review (`3b1d057`)

roborev: Combined Review (`eaa53ef`)

roborev: Combined Review (`4c70acf`)

roborev: Combined Review (`09a165e`)