feat(github): support per-workspace GitHub App registrations by carlosflorencio · Pull Request #1810 · kdlbs/kandev

carlosflorencio · 2026-07-19T21:38:54Z

Summary

move GitHub App configuration from system settings into each workspace's GitHub connection flow
support multiple encrypted GitHub App registrations that workspaces can create, import, reuse, and install independently
route installation tokens, OAuth callbacks, webhooks, cache entries, and broker leases by registration ID
replace the unpublished singleton schema and remove KANDEV_GITHUB_APP_* environment configuration
clarify PAT, named GitHub CLI, GitHub App, and personal identity ownership in responsive desktop/mobile UX
document private/public visibility, shared registrations, work/personal isolation, and exact create/import setup

Security

bind manifest/import/callback state to workspace, user, registration, public origin, and expiry
verify webhook HMAC before delivery claims or mutations
preserve monotonic personal credential generations during compensated transitions
make App credential deletion fail closed with durable rollback checks
serialize create/import/delete/runtime publication per registration

Verification

make fmt
make typecheck
make test
make lint
GitHub backend race suite: 923 passed
web unit suite: 5,738 passed, 4 skipped
CLI suite: 280 passed
desktop/mobile GitHub authentication E2E: 6 passed
desktop/mobile GitHub App registration E2E: 6 passed

Real GitHub manifest conversion, installation, webhook delivery, and OAuth callbacks remain staging checks because they require a public HTTPS Kandev origin and GitHub-owned credentials.

Preview Environment


URL	https://kandev-pr-1810-bwo7.sprites.app
Commit	`f75cdc1`
Agent	Mock agent

Updates automatically on each push. Destroyed when the PR is closed.

greptile-apps · 2026-07-19T21:38:58Z

Too many files changed for review. (343 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

coderabbitai · 2026-07-19T21:39:03Z

Important

Review skipped

Too many files!

This PR contains 343 files, which is 243 over the limit of 100.

To get a review, narrow the scope:
• coderabbit review --committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

Upgrade to a paid plan to raise the limit.

Usage-priced reviews support at most 300 files.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c96b2bcc-3630-405d-9b3b-f1c3737f735e

📥 Commits

Reviewing files that changed from the base of the PR and between bc218df and a4a4321.

📒 Files selected for processing (343)

apps/backend/AGENTS.md
apps/backend/cmd/agentctl/github_cli_shim.go
apps/backend/cmd/agentctl/github_cli_shim_test.go
apps/backend/cmd/agentctl/github_credential.go
apps/backend/cmd/agentctl/github_credential_test.go
apps/backend/cmd/agentctl/main.go
apps/backend/internal/agent/remoteauth/catalog.go
apps/backend/internal/agent/remoteauth/catalog_test.go
apps/backend/internal/agent/runtime/lifecycle/broker_reachability.go
apps/backend/internal/agent/runtime/lifecycle/broker_reachability_test.go
apps/backend/internal/agent/runtime/lifecycle/container.go
apps/backend/internal/agent/runtime/lifecycle/container_config_test.go
apps/backend/internal/agent/runtime/lifecycle/credential_uploader.go
apps/backend/internal/agent/runtime/lifecycle/executor_docker.go
apps/backend/internal/agent/runtime/lifecycle/executor_docker_test.go
apps/backend/internal/agent/runtime/lifecycle/executor_sprites.go
apps/backend/internal/agent/runtime/lifecycle/executor_sprites_clone_auth_test.go
apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go
apps/backend/internal/agent/runtime/lifecycle/executor_sprites_e2e_test.go
apps/backend/internal/agent/runtime/lifecycle/executor_sprites_operations.go
apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go
apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go
apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go
apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go
apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go
apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go
apps/backend/internal/agent/runtime/lifecycle/remote_github_env_test.go
apps/backend/internal/agentctl/server/config/config.go
apps/backend/internal/agentctl/server/config/config_test.go
apps/backend/internal/automation/evaluator.go
apps/backend/internal/automation/provider.go
apps/backend/internal/backendapp/adapters_test.go
apps/backend/internal/backendapp/e2e_reset.go
apps/backend/internal/backendapp/e2e_reset_test.go
apps/backend/internal/backendapp/gateway.go
apps/backend/internal/backendapp/gateway_git_association_test.go
apps/backend/internal/backendapp/github_deployment_app_test.go
apps/backend/internal/backendapp/helpers.go
apps/backend/internal/backendapp/main.go
apps/backend/internal/backendapp/orchestrator.go
apps/backend/internal/backendapp/services.go
apps/backend/internal/backendapp/services_github_broker_test.go
apps/backend/internal/common/config/config.go
apps/backend/internal/common/config/config_test.go
apps/backend/internal/github/action_presets_service.go
apps/backend/internal/github/app_client.go
apps/backend/internal/github/app_client_test.go
apps/backend/internal/github/app_credential_provider.go
apps/backend/internal/github/app_credential_provider_test.go
apps/backend/internal/github/app_installation_service.go
apps/backend/internal/github/app_installation_service_test.go
apps/backend/internal/github/app_registration_helpers.go
apps/backend/internal/github/app_registration_import.go
apps/backend/internal/github/app_registration_import_test.go
apps/backend/internal/github/app_registration_lifecycle_service.go
apps/backend/internal/github/app_registration_store_test.go
apps/backend/internal/github/app_registration_test_helpers_test.go
apps/backend/internal/github/app_token_cache.go
apps/backend/internal/github/app_token_cache_test.go
apps/backend/internal/github/auth_principal.go
apps/backend/internal/github/auth_resolver.go
apps/backend/internal/github/auth_resolver_test.go
apps/backend/internal/github/branch_protection.go
apps/backend/internal/github/controller.go
apps/backend/internal/github/controller_app_registration.go
apps/backend/internal/github/controller_app_registration_test.go
apps/backend/internal/github/controller_auth.go
apps/backend/internal/github/controller_auth_identity_test.go
apps/backend/internal/github/controller_test.go
apps/backend/internal/github/controller_watch_reset.go
apps/backend/internal/github/controller_webhook_test.go
apps/backend/internal/github/copy.go
apps/backend/internal/github/copy_test.go
apps/backend/internal/github/credential_broker.go
apps/backend/internal/github/credential_broker_test.go
apps/backend/internal/github/deployment_app_config.go
apps/backend/internal/github/deployment_app_config_test.go
apps/backend/internal/github/deployment_app_conversion_client.go
apps/backend/internal/github/deployment_app_conversion_client_test.go
apps/backend/internal/github/deployment_app_manifest.go
apps/backend/internal/github/deployment_app_manifest_test.go
apps/backend/internal/github/deployment_app_store.go
apps/backend/internal/github/gh_accounts.go
apps/backend/internal/github/gh_accounts_test.go
apps/backend/internal/github/gh_client.go
apps/backend/internal/github/handlers.go
apps/backend/internal/github/handlers_authorization_test.go
apps/backend/internal/github/mock_auth.go
apps/backend/internal/github/mock_client.go
apps/backend/internal/github/mock_controller.go
apps/backend/internal/github/mock_controller_test.go
apps/backend/internal/github/models.go
apps/backend/internal/github/noop_client.go
apps/backend/internal/github/oauth_flow.go
apps/backend/internal/github/oauth_flow_test.go
apps/backend/internal/github/pat_client.go
apps/backend/internal/github/pat_client_test.go
apps/backend/internal/github/personal_auth_service.go
apps/backend/internal/github/personal_auth_service_test.go
apps/backend/internal/github/personal_connection_repository.go
apps/backend/internal/github/personal_connection_repository_test.go
apps/backend/internal/github/poller.go
apps/backend/internal/github/poller_test.go
apps/backend/internal/github/service.go
apps/backend/internal/github/service_accessible_repos.go
apps/backend/internal/github/service_app_auth.go
apps/backend/internal/github/service_app_auth_connection_store.go
apps/backend/internal/github/service_app_auth_test.go
apps/backend/internal/github/service_auth_test.go
apps/backend/internal/github/service_automation_auth.go
apps/backend/internal/github/service_cleanup.go
apps/backend/internal/github/service_cleanup_policy_test.go
apps/backend/internal/github/service_connections.go
apps/backend/internal/github/service_connections_test.go
apps/backend/internal/github/service_git_transport.go
apps/backend/internal/github/service_git_transport_test.go
apps/backend/internal/github/service_issues.go
apps/backend/internal/github/service_mentions_test.go
apps/backend/internal/github/service_pr.go
apps/backend/internal/github/service_pr_sync_test.go
apps/backend/internal/github/service_pr_watch.go
apps/backend/internal/github/service_pr_watch_batched.go
apps/backend/internal/github/service_pr_watch_multi_branch_test.go
apps/backend/internal/github/service_repo_contents.go
apps/backend/internal/github/service_repo_negative_cache_test.go
apps/backend/internal/github/service_request_review_test.go
apps/backend/internal/github/service_reviews.go
apps/backend/internal/github/service_reviews_auth_test.go
apps/backend/internal/github/service_routing.go
apps/backend/internal/github/service_routing_test.go
apps/backend/internal/github/service_task_events.go
apps/backend/internal/github/service_task_events_test.go
apps/backend/internal/github/service_task_issue.go
apps/backend/internal/github/service_tokens.go
apps/backend/internal/github/service_watch_reset.go
apps/backend/internal/github/service_workspace_authorization_test.go
apps/backend/internal/github/store.go
apps/backend/internal/github/store_connections.go
apps/backend/internal/github/store_connections_test.go
apps/backend/internal/github/token_client.go
apps/backend/internal/github/token_client_test.go
apps/backend/internal/github/webhook_service.go
apps/backend/internal/github/webhook_service_test.go
apps/backend/internal/github/workspace_settings_service.go
apps/backend/internal/github/workspace_settings_test.go
apps/backend/internal/githubauth/environment.go
apps/backend/internal/health/checks.go
apps/backend/internal/health/checks_test.go
apps/backend/internal/health/service_test.go
apps/backend/internal/improvekandev/handler.go
apps/backend/internal/improvekandev/handler_test.go
apps/backend/internal/orchestrator/event_handlers_github.go
apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go
apps/backend/internal/orchestrator/event_handlers_github_test.go
apps/backend/internal/orchestrator/event_handlers_test.go
apps/backend/internal/orchestrator/executor/executor.go
apps/backend/internal/orchestrator/executor/executor_credentials.go
apps/backend/internal/orchestrator/executor/executor_credentials_test.go
apps/backend/internal/orchestrator/executor/executor_execute.go
apps/backend/internal/orchestrator/executor/executor_resume.go
apps/backend/internal/orchestrator/executor/executor_resume_clone_default_branch_test.go
apps/backend/internal/orchestrator/executor/executor_resume_test.go
apps/backend/internal/orchestrator/executor/executor_test.go
apps/backend/internal/orchestrator/service.go
apps/backend/internal/repoclone/clone.go
apps/backend/internal/repoclone/clone_auth_test.go
apps/backend/internal/repoclone/clone_test.go
apps/backend/internal/secrets/user_visible_store.go
apps/backend/internal/secrets/user_visible_store_test.go
apps/backend/internal/task/handlers/repository_handlers_test.go
apps/backend/internal/task/models/models.go
apps/backend/internal/task/repository/sqlite/session_branch.go
apps/backend/internal/task/service/repository_discovery.go
apps/backend/internal/task/service/repository_discovery_test.go
apps/backend/internal/task/service/service.go
apps/backend/internal/task/share/backend.go
apps/backend/internal/task/share/gist_backend.go
apps/backend/internal/task/share/gist_backend_test.go
apps/backend/internal/task/share/http.go
apps/backend/internal/task/share/http_test.go
apps/backend/internal/task/share/provider.go
apps/backend/internal/task/share/service.go
apps/backend/internal/task/share/service_test.go
apps/backend/internal/workflowsync/service.go
apps/backend/internal/workflowsync/service_test.go
apps/web/app/github/github-page-client.tsx
apps/web/app/settings/executors/new/[type]/page.tsx
apps/web/app/settings/integrations/github/page.tsx
apps/web/components/app-sidebar/sections/settings/workspaces-group.tsx
apps/web/components/github/github-app-connection-panel.tsx
apps/web/components/github/github-app-create-form.tsx
apps/web/components/github/github-app-import-fields.tsx
apps/web/components/github/github-app-import-form.test.tsx
apps/web/components/github/github-app-import-form.tsx
apps/web/components/github/github-app-import-guide.test.tsx
apps/web/components/github/github-app-import-guide.tsx
apps/web/components/github/github-app-onboarding-model.test.ts
apps/web/components/github/github-app-onboarding-model.ts
apps/web/components/github/github-app-policy-dialog.tsx
apps/web/components/github/github-app-registration-list.tsx
apps/web/components/github/github-app-visibility-field.tsx
apps/web/components/github/github-auth-method-list.tsx
apps/web/components/github/github-callback-notice.tsx
apps/web/components/github/github-cli-form.tsx
apps/web/components/github/github-connection-dialog.test.tsx
apps/web/components/github/github-connection-dialog.tsx
apps/web/components/github/github-pat-form.tsx
apps/web/components/github/github-permissions-dialog.tsx
apps/web/components/github/github-settings.tsx
apps/web/components/github/github-status.test.tsx
apps/web/components/github/github-status.tsx
apps/web/components/github/multi-pr-ci-popover.tsx
apps/web/components/github/my-github/pr-list.tsx
apps/web/components/github/my-github/quick-task-launcher.tsx
apps/web/components/github/my-github/use-github-search.ts
apps/web/components/github/my-github/use-pr-statuses.test.ts
apps/web/components/github/my-github/use-pr-statuses.ts
apps/web/components/github/pr-ci-popover.automation.test.tsx
apps/web/components/github/pr-ci-popover.tsx
apps/web/components/github/pr-detail-panel.test.ts
apps/web/components/github/pr-detail-panel.tsx
apps/web/components/github/pr-merge-button.tsx
apps/web/components/github/pr-status-chip.tsx
apps/web/components/github/repo-filter-selector.tsx
apps/web/components/github/use-pr-scoped-review-request.ts
apps/web/components/integrations/integrations-menu.tsx
apps/web/components/kanban-card.tsx
apps/web/components/settings/profile-edit/remote-credentials-card.tsx
apps/web/components/settings/settings-layout-client.test.tsx
apps/web/components/task-create-dialog-remote-repo-chips.test.tsx
apps/web/components/task-create-dialog-remote-repo-chips.tsx
apps/web/components/task-create-dialog-repo-chips.tsx
apps/web/components/task-create-dialog-state.ts
apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.tsx
apps/web/components/task/new-subtask-dialog.tsx
apps/web/components/task/new-subtask-form-state.test.ts
apps/web/components/task/new-subtask-form-state.ts
apps/web/components/task/task-github-pr-dialog.tsx
apps/web/components/task/task-session-sidebar-dialogs.tsx
apps/web/e2e/fixtures/backend.ts
apps/web/e2e/helpers/api-client.ts
apps/web/e2e/helpers/layout-assertions.ts
apps/web/e2e/helpers/multi-pr-review.ts
apps/web/e2e/pages/github-auth-settings-page.ts
apps/web/e2e/tests/integrations/github-authentication.spec.ts
apps/web/e2e/tests/integrations/mobile-github-auth-settings.spec.ts
apps/web/e2e/tests/office/mobile-tasks.spec.ts
apps/web/e2e/tests/pr/mobile-pr-rerequest-review.spec.ts
apps/web/e2e/tests/pr/pr-topbar-popover.spec.ts
apps/web/e2e/tests/review/external-vcs-file-link.spec.ts
apps/web/e2e/tests/review/review-file-status.spec.ts
apps/web/e2e/tests/settings/config-chat-popover.spec.ts
apps/web/e2e/tests/settings/github-app-registration.spec.ts
apps/web/e2e/tests/settings/mobile-github-app-registration.spec.ts
apps/web/e2e/tests/task/mobile-create-task-remote-repo.spec.ts
apps/web/e2e/tests/task/mobile-external-vcs-file-link.spec.ts
apps/web/hooks/domains/github/use-accessible-repos.test.ts
apps/web/hooks/domains/github/use-accessible-repos.ts
apps/web/hooks/domains/github/use-active-task-pr-files.ts
apps/web/hooks/domains/github/use-branches-by-url.test.ts
apps/web/hooks/domains/github/use-branches-by-url.ts
apps/web/hooks/domains/github/use-github-app-registrations.test.tsx
apps/web/hooks/domains/github/use-github-app-registrations.ts
apps/web/hooks/domains/github/use-github-status.test.tsx
apps/web/hooks/domains/github/use-github-status.ts
apps/web/hooks/domains/github/use-issue-watches.test.ts
apps/web/hooks/domains/github/use-issue-watches.ts
apps/web/hooks/domains/github/use-pr-ci-popover.ts
apps/web/hooks/domains/github/use-pr-commits.ts
apps/web/hooks/domains/github/use-pr-diff.test.ts
apps/web/hooks/domains/github/use-pr-diff.ts
apps/web/hooks/domains/github/use-pr-feedback.ts
apps/web/hooks/domains/github/use-pr-info-by-url.test.ts
apps/web/hooks/domains/github/use-pr-info-by-url.ts
apps/web/hooks/domains/github/use-pr-watch.test.ts
apps/web/hooks/domains/github/use-pr-watch.ts
apps/web/hooks/domains/github/use-pr-workspace-scope.test.ts
apps/web/hooks/domains/github/use-repo-merge-methods.test.ts
apps/web/hooks/domains/github/use-repo-merge-methods.ts
apps/web/hooks/domains/github/use-review-watches.test.ts
apps/web/hooks/domains/github/use-review-watches.ts
apps/web/hooks/domains/integrations/use-remote-repositories.test.tsx
apps/web/hooks/domains/integrations/use-remote-repositories.ts
apps/web/lib/api/domains/github-api.test.ts
apps/web/lib/api/domains/github-api.ts
apps/web/lib/api/domains/github-auth-api.ts
apps/web/lib/api/domains/github-pr-api.ts
apps/web/lib/api/domains/github-review-api.test.ts
apps/web/lib/api/domains/github-review-api.ts
apps/web/lib/api/domains/settings-api.ts
apps/web/lib/github-auth.test.ts
apps/web/lib/github-auth.ts
apps/web/lib/state/default-state.ts
apps/web/lib/state/hydration/hydrator.ts
apps/web/lib/state/slices/github/github-slice.test.ts
apps/web/lib/state/slices/github/github-slice.ts
apps/web/lib/state/slices/github/types.ts
apps/web/lib/state/slices/index.ts
apps/web/lib/state/store-overrides.ts
apps/web/lib/state/store-reexports.ts
apps/web/lib/state/store.ts
apps/web/lib/types/github-app.ts
apps/web/lib/types/github.ts
apps/web/lib/ws/handlers/github.test.ts
docs/ARCHITECTURE.md
docs/decisions/0030-workspace-scoped-integration-settings.md
docs/decisions/0047-github-authentication-ownership.md
docs/decisions/2026-07-20-managed-github-app-registration.md
docs/decisions/2026-07-21-workspace-selectable-github-app-registrations.md
docs/decisions/INDEX.md
docs/docker.md
docs/k8s.md
docs/plans/github-authentication/plan.md
docs/plans/github-authentication/task-01-persistence-migration.md
docs/plans/github-authentication/task-02-app-token-primitives.md
docs/plans/github-authentication/task-03-workspace-credential-resolver.md
docs/plans/github-authentication/task-04-app-oauth-webhooks.md
docs/plans/github-authentication/task-05-service-routing.md
docs/plans/github-authentication/task-06-executor-credentials.md
docs/plans/github-authentication/task-07-http-health-mocks.md
docs/plans/github-authentication/task-08-frontend-settings.md
docs/plans/github-authentication/task-09-e2e-docs.md
docs/plans/github-authentication/task-10-qa-security.md
docs/plans/github-authentication/task-11-reconcile-main.md
docs/plans/github-authentication/task-12-registration-persistence.md
docs/plans/github-authentication/task-13-registration-onboarding-protocol.md
docs/plans/github-authentication/task-14-runtime-registration-resolution.md
docs/plans/github-authentication/task-15-registration-webhook-routing.md
docs/plans/github-authentication/task-16-workspace-app-lifecycle-api.md
docs/plans/github-authentication/task-17-frontend-registration-client.md
docs/plans/github-authentication/task-18-workspace-authentication-ux.md
docs/plans/github-authentication/task-19-registration-e2e.md
docs/plans/github-authentication/task-20-registration-documentation.md
docs/plans/github-authentication/task-21-security-qa.md
docs/public/configuration.md
docs/public/coverage.json
docs/public/executors.md
docs/public/feature-status.md
docs/public/integrations.md
docs/public/use-kandev.md
docs/public/workflow-sync.md
docs/specs/INDEX.md
docs/specs/integrations/github-authentication.md

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feature/github-token-secrets-u9d

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

claude · 2026-07-19T21:39:11Z

Claude finished @carlosflorencio's task in 7m 27s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context and reading skill file
Reviewing core auth architecture (auth_resolver.go, auth_principal.go)
Reviewing credential broker and token cache
Reviewing personal auth service and OAuth flow
Reviewing executor credential changes
Reviewing tests
Posting findings

Review complete. Full findings posted below. 3 blockers, 2 suggestions.

Blockers:

Unbounded spinloop in CredentialResolver.Resolve — CPU starvation under concurrent epoch invalidation
Missing tests for CreateReviewWatchForUser — new auth-routing logic with no coverage
CredentialBroker in-memory lease map has no size cap — gradual memory growth on long-running servers

Suggestions:
4. KANDEV_GITHUB_CREDENTIAL_* constants duplicated across 3 packages — silent rename risk
5. Isolated gh config dirs not cleaned up on local executors

The architecture itself is well-designed: PKCE/state-digest OAuth flow, generation-keyed cache invalidation, scope-binding in the credential broker, and safe ConsumeBound for public callbacks all look correct.

github-actions · 2026-07-19T21:41:04Z

Cloudflare Pages docs preview

Open the docs preview

Built from docs commit f75cdc1.

Stable PR alias: https://docs-pr-1810.landing-87j.pages.dev/docs

claude · 2026-07-19T21:46:40Z

Findings

Blocker (must fix before merge)

Spinloop without bound in CredentialResolver.Resolve — apps/backend/internal/github/auth_resolver.go:124-137
- Issue: The for {} loop re-runs if the workspace epoch changes mid-resolution. No backoff or maximum iteration count is enforced. The only termination guard is ctx.Err(), checked only when the epoch is detected as stale.
- Why: Under sustained concurrent InvalidateWorkspace calls, every resolution goroutine in-flight spins on CPU until its context is cancelled. This can starve the goroutine scheduler under load.
- Fix: Add runtime.Gosched() before continue so the scheduler can yield, or check ctx.Err() unconditionally at the top of the loop.
Missing tests for CreateReviewWatchForUser — apps/backend/internal/github/service_reviews.go:29-42
- Issue: New logic introduced in this PR with no *_test.go coverage. It resolves a personal auth credential, validates the resulting login, and feeds targetLogin into the watch record.
- Why: Per project convention, new non-UI logic without tests is a blocker. Untested auth-routing decisions are especially important — a bug that sets an empty targetLogin would silently create an App-auth watch that CheckReviewWatch would disable on its first poll.
- Fix: Cover at minimum: empty WorkspaceID, resolvePersonalReadClient failure, empty Login after resolution, and the happy path. Use /tdd.
CredentialBroker in-memory lease map grows without bound — apps/backend/internal/github/credential_broker.go:148
- Issue: sweepExpiredLocked is called lazily inside Issue and loadLease only. A caller that issues many leases within a 12h window adds entries indefinitely. No per-workspace cap, no background sweeper.
- Why: For a long-running server, accumulated unexpired leases are never evicted until the next Issue or Resolve call for that workspace. This is a gradual memory growth risk.
- Fix: Add a per-workspace lease cap (e.g., 100 entries) and return an error when exceeded, or start a background sweeper goroutine.

Suggestion (recommended, doesn't block)

Duplicated KANDEV_GITHUB_CREDENTIAL_* constants in three packages — executor_credentials.go:22-30, remote_github_env.go:6-15, cmd/agentctl/github_credential.go:18-26
- The same nine string values are copy-pasted across three packages with slightly different Go variable names. A future rename silently breaks two packages at runtime with no compile error. Extract to a shared package.
github_cli_shim.go:68-78: isolated gh config directories accumulate on local executors
- isolatedGitHubCLIConfigDir creates dirs in os.TempDir() with no cleanup registration. Fine for cloud executors (ephemeral), but accumulates on local_pc and local_docker. Register cleanup or document the caller responsibility.

What looks good

PKCE and state digest pattern — storing only the SHA-256 hash of state and PKCE verifier in DB, never the raw bearer values, is correct and prevents state replay from a DB read.
Generation-keyed credential cache — storeCached evicts old generations before inserting; clone-on-read prevents aliased mutation of cached entries.
singleflight in InstallationTokenCache.Get — generation check inside the lock before committing a newly minted token correctly handles the race between an in-flight mint and a concurrent Invalidate.
CredentialBroker scope binding — lease records bind task/session/repo at issuance, and authorizeLease re-verifies ownership AND credential generation on every redemption, so a workspace credential rotation immediately invalidates all outstanding leases.
ConsumeBound for public OAuth callbacks — workspace and user IDs are recovered from the server-side flow record, never from untrusted callback query parameters.
Compensation in StorePersonalConnectionRepository — best-effort compensate-on-failure for the secrets-then-metadata write sequence is sound.

Summary

Severity	Count
Blocker	3
Suggestion	2

Verdict: Blocked — fix blockers first.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc4183b482

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

claude · 2026-07-19T22:02:36Z

Claude finished @carlosflorencio's task in 5m 32s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Reading code-review skill and gathering context
Reviewing auth resolver and credential cache
Reviewing credential broker and OAuth flow
Reviewing executor credential plumbing
Reviewing PR watch / poller changes
Reviewing gh shim and credential helper
Posting inline findings

Findings

Blocker (must fix before merge)

associatePRFromPushScoped stores PR watches without workspace_id — apps/backend/internal/orchestrator/event_handlers_github.go:1089-1095
- CreatePRWatch (not CreatePRWatchForWorkspace) is called, writing workspace_id = "" to the DB. The poller's tryBatchedPRWatchCheck (line 199) logs a warning and returns false for the entire batch when any watch has an empty WorkspaceID. The fallback checkSinglePRWatch then calls CheckPRWatchForWorkspace, which returns ErrGitHubWorkspaceRequired on an empty workspace — logged at DEBUG and silently dropped.
- Net effect: one push-detected PR with a missing workspace puts all PR monitoring onto the slow per-watch path, and that watch is never polled for CI or review state.
- This PR added EnsurePRWatchForWorkspace and updated ensureSessionPRWatch / CheckSessionPR to be workspace-aware, but associatePRFromPushScoped was left behind.
- Fix: add workspaceID string to associatePRFromPushScoped, thread it from detectPushAndAssociatePR (which already resolves workspaceID at line 553), and call CreatePRWatchForWorkspace. Also add CreatePRWatchForWorkspace to the GitHubService interface.
Missing tests for CreateReviewWatchForUser — apps/backend/internal/github/service_reviews.go:28-42 (inline comment posted)
- New function with no *_test.go coverage. It resolves a personal credential, validates the login, and sets targetLogin on the watch record — all paths with observable failure modes.
- A bug returning Login = "" silently creates a watch with targetLogin = "", making CheckReviewWatch behave as if no personal target was configured.
- Fix: add tests for empty WorkspaceID, resolver failure, empty Login, and the happy path. Use /tdd.
gh shim uses the primary repo lease for all repos — apps/backend/cmd/agentctl/github_cli_shim.go:43-47 (inline comment posted)
- newGitHubCredentialBrokerClient reads only the primary env-var lease, ignoring KANDEV_GITHUB_CREDENTIAL_SCOPES. The git credential helper already correctly iterates scopes to find the matching repo.
- Under GitHub App auth, installation tokens are repo-scoped. A gh command run in a secondary repo's working directory (e.g., gh pr create) presents the primary repo's token, which GitHub rejects with 403.
- Fix: resolve the target repo from args or GH_REPO/git remote and select the matching scope from KANDEV_GITHUB_CREDENTIAL_SCOPES, mirroring newGitHubCredentialBrokerClientForInput.

Suggestion (recommended, doesn't block)

Duplicate KANDEV_GITHUB_CREDENTIAL_* constants in three packages — executor_credentials.go:22-30, remote_github_env.go:6-14, cmd/agentctl/github_credential.go:16-25 (inline comment posted)
- Nine identical string values defined independently. A rename compiles without error in all three packages while breaking two at runtime.
- Fix: extract to a shared package and import from all three.
Isolated gh config dirs accumulate on local executors — apps/backend/cmd/agentctl/github_cli_shim.go:68-78 (inline comment posted)
- isolatedGitHubCLIConfigDir creates dirs in os.TempDir() with no cleanup. Cloud/container executors are ephemeral, but local_pc and local_docker accumulate these indefinitely.
- Fix: defer os.RemoveAll(configDir) in runGitHubCLIShim.

What looks good

PKCE + state digest in OAuthFlowManager — only the SHA-256 hash of state and PKCE verifier are stored; raw bearer values never hit the DB. ConsumeBound recovers workspace/user from the server-side row, never from untrusted callback params.
CredentialBroker scope binding — authorizeLease re-verifies task/session/repo ownership AND credential generation on every redemption, so a workspace credential rotation immediately invalidates all outstanding leases.
singleflight in InstallationTokenCache — generation check inside the write lock before committing prevents the race between an in-flight mint and a concurrent Invalidate.
storeCached evicts old generations before insert — prevents stale credentials from surviving a rotation.
runtime.Gosched() in CredentialResolver.Resolve — the spinloop correctly yields and checks ctx.Err() on each retry; not a starvation risk.
EnsurePRWatchForWorkspace migration — ensureSessionPRWatch and CheckSessionPR are correctly updated to the scoped variant.

Summary

Severity	Count
Blocker	3
Suggestion	2

Verdict: Blocked — fix the three blockers. The architecture and security properties of the new auth layer are solid; the issues are all in the operational plumbing wired around it.

claude · 2026-07-19T22:30:40Z

Claude finished @carlosflorencio's task in 5m 12s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context (current head 9ffbd25, prior findings, two fix commits)
Verifying fixes for prior blockers (spinloop, broker memory cap, review-watch tests)
Reviewing new commits for new issues (gh shim scope, push-detected PR watches, constants)
Posting inline findings

Findings

All 6 prior blockers are resolved. One earlier suggestion remains open.

Prior blockers — verified fixed

Spinloop (auth_resolver.go:129-133) — runtime.Gosched() is called and ctx.Err() is checked before each retry. No CPU starvation under concurrent epoch invalidation. ✅
CredentialBroker lease cap (credential_broker.go:152-154, credential_broker_test.go:168-185) — maxCredentialLeasesPerWorkspace = 10_000 enforces a per-workspace cap; sweepExpiredLocked runs before the cap check so expired leases don't count. Test TestCredentialBrokerLimitsActiveLeasesPerWorkspace confirms rejection at the 10,001st lease. ✅
CreateReviewWatchForUser tests (service_reviews_auth_test.go) — four cases cover nil/empty workspace → ErrGitHubWorkspaceRequired, resolver failure without persistence, empty resolved login → ErrGitHubPersonalRequired, and happy path persisting the resolved targetLogin. ✅
gh shim multi-repo scope (github_cli_shim.go:77-102) — newGitHubCLIShimCredentialBrokerClient now checks CredentialScopesEnv; resolves target repo from --repo arg → GH_REPO env → git remote get-url origin, then selects the matching scope from KANDEV_GITHUB_CREDENTIAL_SCOPES. Falls back to primary lease only when no target repo can be determined. TestGitHubCLIShimSelectsRepositoryLease verifies the GH_REPO path selects the correct lease. ✅
Push-detected PRs missing workspace (event_handlers_github.go:1090-1116) — associatePRFromPushScoped now takes workspaceID and calls CreatePRWatchForWorkspace/AssociatePRWithTaskForWorkspace; searchPRForExistingWatch also takes workspaceID and uses the workspace-scoped variants. Tests assert ghSvc.lastCreateWatchWorkspaceID == "ws1" and ghSvc.lastAssociateWorkspaceID == "ws1". ✅
Duplicated constants — consolidated into internal/githubauth/environment.go; executor_credentials.go, remote_github_env.go, and cmd/agentctl/github_credential.go all import from there. ✅

Suggestion (still open)

Isolated gh config dirs accumulate on non-ephemeral executors — github_cli_shim.go:59
- isolatedGitHubCLIConfigDir creates a per-session dir under os.TempDir() with no defer os.RemoveAll. Cloud and container executors are ephemeral, but local_pc and local_docker accumulate these across sessions.
- installGitHubCLIShim returns a cleanup func() for its symlink dir; the same pattern is missing here.
- Fix (one line): defer os.RemoveAll(configDir) immediately after the isolatedGitHubCLIConfigDir call in runGitHubCLIShim.

Summary

Severity	Count
Blocker	0
Suggestion	1

Verdict: Ready to merge — all blockers resolved. The config-dir cleanup suggestion is low-risk for cloud/container deployments; address it whenever convenient for local-executor users.

claude · 2026-07-19T22:49:06Z

Claude finished @carlosflorencio's task in 1m 17s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Verifying fix for remaining suggestion (isolated gh config dir cleanup — 4066e54)
Scanning new commit for any regressions
Posting final findings

Summary of `4066e54 fix: clean up isolated GitHub CLI state`

Three files changed, all targeted and correct.

github_cli_shim.go

defer func() { _ = os.RemoveAll(configDir) }() added immediately after the config dir is created — exactly the fix from the suggestion.
isolatedGitHubCLIConfigDir now uses os.MkdirTemp instead of os.MkdirAll with a deterministic path. This is a bonus improvement: concurrent gh invocations within the same task/session now each get a unique directory with a random suffix, eliminating a subtle race where two simultaneous invocations could stomp each other's gh auth state. Cleanup on chmod failure also calls os.RemoveAll.

github_cli_shim_test.go

The assertion was correctly inverted from configDirs[0] != configDirs[1] (stable, same dir) to configDirs[0] == configDirs[1] (unique dirs expected), reflecting the MkdirTemp change.
Two new assertions verify: the config dir exists during the invocation and is gone after it returns. This is tight, correct coverage.

main.go

os.Getenv(envGitHubCredentialBrokerURL) → os.Getenv(githubauth.CredentialBrokerURLEnv), consistent with the centralized constants from 9ffbd25.

No regressions detected. The change is minimal and scoped.

Final status

Round	Blockers	Suggestions
Initial review	3	2
After `9e0589b`	0	2
After `9ffbd25`	0	1
After `4066e54`	0	0

Verdict: ready to merge. All blockers and suggestions are resolved.

claude · 2026-07-20T09:52:24Z

Claude finished @carlosflorencio's task in 2m 45s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill
Checking new commit since last review (531c8a8 / was 4066e54 in prior round)
Reviewing new changes — 5e22fa4 fix: scope GitHub PR data to active workspace
Posting findings

Findings — `5e22fa4 fix: scope GitHub PR data to active workspace`

All prior blockers and suggestions remain resolved. This round covers the new commit only.

What changed

Seven files across three areas:

Hooks (use-active-task-pr-files, use-pr-commits, use-pr-diff) — Each hook now reads workspaceId from the active workspace store slice and:

Prefixes in-flight / fetched tracking keys with the workspace ID so a workspace switch immediately invalidates the tracking refs
Passes workspace_id in every WS request
Wraps state in a WorkspacePRFiles / WorkspacePRCommitsState / WorkspacePRDiffState envelope; returns EMPTY_FILES / INITIAL_STATE for the render-phase window between a workspace change and the effect that clears cached data
Uses workspaceIdRef.current !== workspaceId to drop stale in-flight responses after a workspace switch

E2E helpers (api-client.ts) — mockGitHubSetUser now also seeds a workspace connection and personal connection for the active workspace. mockGitHubAddPRs, mockGitHubAddIssues, and mockGitHubSeedPRFeedback each call a new private seedMockGitHubRepositoryAccess helper that auto-registers any referenced repos in the mock, de-duplicated by owner/name.

E2E test (pr-topbar-popover.spec.ts) — one assertion updated from the global mockGitHubSetAuthHealth({ authenticated: false }) to the workspace-scoped mockGitHubSetWorkspaceConnectionStatus(seedData.workspaceId, "invalid"), consistent with the scoped auth model.

Test file (use-pr-workspace-scope.test.ts) — two new tests:

workspace_id is included in usePRDiff and usePRCommits requests
A stale response for ws-1 is discarded and never lands in filesByPRKey after switching to ws-2, while the fresh ws-2 response does land

Verdict on this commit

No blockers, no suggestions. The workspace-scoping is applied consistently across all three hooks using an identical structural pattern. The staleness guard (workspaceIdRef.current !== workspaceId checked inside the .then() / .catch() closures, with the ref updated synchronously on each render) correctly prevents out-of-order responses from contaminating the new workspace's data. The pruneByKeySet / EMPTY_FILES render-phase guard in the return path handles the brief window before effects run.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0

Verdict: ready to merge.

claude · 2026-07-20T10:10:18Z

Claude finished @carlosflorencio's task in 2m 35s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill
Reviewing new commit 210a6bc — test: verify GitHub auth controls do not overlap
Posting findings

Findings — `210a6bc test: verify GitHub auth controls do not overlap`

All prior blockers and suggestions remain resolved. This round covers the new commit only (one file changed: apps/web/e2e/tests/integrations/github-authentication.spec.ts).

What changed

The test that checks the floating save button doesn't visually overlap with the Configuration Chat button was updated:

Test ID fix — github-scope-save (which no longer exists in the component tree) replaced with settings-floating-save, the correct floating save element.
Trigger interaction added — github-scope-mode is now clicked and "Organizations" selected to put settings into a dirty state before checking for the floating save, which only appears when there are unsaved changes. The old version would have been finding a null bounding box once github-scope-save was removed.
Improved overlap check — the original saveBox!.x + saveBox!.width <= configChatBox!.x was a strict "save must be entirely left of chat" assertion. The new check computes the full 2D intersection rectangle and passes if either dimension is non-positive — correct, and handles cases where the two elements are not horizontally adjacent.

Suggestion (doesn't block)

Inline overlap logic duplicates an equivalent expectElementsNotToIntersect local helper in config-chat-popover.spec.ts:200-213. Both could be consolidated into apps/web/e2e/helpers/layout-assertions.ts, which already exists for shared layout assertions. Inline comment posted.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1

Verdict: ready to merge. No blockers. The test fix is correct; the overlap-helper duplication is minor cleanup for whenever convenient.

claude · 2026-07-20T10:15:47Z

Claude finished @carlosflorencio's task in 1m 53s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill
Reviewing new commit e46c711 — test: share E2E overlap assertion
Posting findings

Findings — `e46c711 test: share E2E overlap assertion`

All prior blockers and suggestions remain resolved. This round covers the new commit only (3 files: layout-assertions.ts, github-authentication.spec.ts, config-chat-popover.spec.ts).

What changed

The inline expectElementsNotToIntersect function that existed only in config-chat-popover.spec.ts is now extracted to apps/web/e2e/helpers/layout-assertions.ts and imported by both specs.

layout-assertions.ts — New export added at line 130:

export async function expectElementsNotToIntersect(first: Locator, second: Locator): Promise<void>

Uses the imported Locator type and expect already in scope. AABB intersection test is correct: checks all four separation conditions (x < r + w && x + w > r && y < r + h && y + h > r) and asserts the conjunction is false.

Semantic equivalence confirmed — the old formula in github-authentication.spec.ts used Math.min/Math.max to compute signed overlap width and height, passing when overlapWidth <= 0 || overlapHeight <= 0. The new formula is the dual: it asserts NOT (overlapX AND overlapY). Both treat touching edges (overlap = 0) as non-intersecting. Logically identical.

config-chat-popover.spec.ts — Local function definition (lines 200–213) removed in full, import added. No orphaned code left.

github-authentication.spec.ts — Inline 9-line bounding-box calculation replaced with a single await expectElementsNotToIntersect(floatingSave, ...) call. Import added.

No regressions, no dead code, no type widening.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0

Verdict: ready to merge. All blockers and suggestions are resolved. The final commit is a correct mechanical extraction with no behavioral change.

claude · 2026-07-20T10:37:37Z

Claude finished @carlosflorencio's task in 2m 40s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill
Reviewing new commits since last review (e46c711)
Posting findings

Findings — `b073ca1` + `b4f4519` (test: synchronize / limit concurrent launch regression)

All prior blockers and suggestions remain resolved. This round covers the two new commits, both of which modify a single file: apps/backend/internal/orchestrator/executor/executor_test.go.

What changed

TestLaunchPreparedSession_SerialisesConcurrentLaunches gained a startProcessGate channel that holds both startAgentProcessAsync background goroutines until the two synchronous LaunchPreparedSession calls have finished.

The problem they fix: The mock repository holds shared session pointers (unlike the real SQLite store, which returns independent copies). Two concurrent LaunchPreparedSession calls can mutate the same session struct. Before the fix, wg.Wait() returned when the two synchronous launches were done, but both startAgentProcessAsync goroutines could still be running concurrently, racing to read the now-mutated session snapshot. This is a flaky-test risk under the race detector.

The fix is correct:

startProcessGate blocks both startAgentProcessAsync goroutines inside startAgentProcessFunc until releaseStartProcessGate() is called after wg.Wait().
sync.Once prevents double-close of the gate channel whether the cleanup fires from an early t.Fatalf or from the explicit call at the bottom of the test.
t.Cleanup(releaseStartProcessGate) is registered before any t.Fatal path, so goroutines are always unblocked on early failure — no goroutine leak.
Two waitForUpdateTaskStateIfNotArchivedCall calls correctly drain both goroutines: the main launch path (LaunchAgent → startAgentProcessAsync) and the fast path (startAgentOnExistingWorkspace → startAgentProcessAsync), which is reached by the second goroutine because GetExecutionIDForSession and IsAgentCommandConfigured both return truthy values in the mock.

No production code was changed. No new edge cases opened.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0

Verdict: ready to merge. The two new commits fix a race condition in the concurrent-launch test and introduce no production changes. All prior blockers and suggestions remain resolved.

claude · 2026-07-20T11:07:12Z

Claude finished @carlosflorencio's task in 1m 43s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill
Reviewing new commit since last review (e132d34 — test: authenticate GitHub branch fixtures)
Posting findings

Findings — `e132d34 test: authenticate GitHub branch fixtures`

All prior blockers and suggestions remain resolved. This round covers the new commit only (one file changed: apps/web/e2e/helpers/api-client.ts).

What changed

mockGitHubAddBranches now seeds a legacy_shared/active workspace connection for the active workspace immediately after posting branch fixture data to the mock server.

Why this was needed: The many E2E tests that use mockGitHubAddBranches predate workspace-scoped authentication. Under the old shared-auth model these tests passed without any workspace connection being present; under the new model a workspace connection is required for the GitHub provider to resolve credentials during branch lookups (e.g., task-creation dialog branch selector). The fix unblocks all affected branch-selector tests without requiring each caller to be individually updated.

Pattern consistency: The approach is identical to what mockGitHubSetUser already does for the same compatibility reason. Both helpers call mockGitHubSetWorkspaceConnection(workspaceId, { source: "legacy_shared", status: "active" }) after their primary operation, and both guard with if (!workspaceId) return when no active workspace exists. The mockGitHubSetUser helper additionally seeds a personal connection — correctly omitted here since branch lookups go through the workspace automation credential, not the personal one.

The if (!workspaceId) return guard is sound: it silently no-ops for tests that call mockGitHubAddBranches before a workspace is set up. In practice all callers that need GitHub branch resolution already have a workspace by the time they call this method.

No regressions, no dead code, no new patterns introduced.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0

Verdict: ready to merge. The commit is a minimal, correct compatibility shim that makes legacy branch-fixture callers work under the new workspace-scoped auth model. All prior blockers and suggestions remain resolved.

claude · 2026-07-20T11:58:40Z

Claude finished @carlosflorencio's task in 1s —— View job

I'll analyze this and get back to you.

claude · 2026-07-20T18:48:49Z

Claude finished @carlosflorencio's task in 4m 14s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill file
Reviewing new commits since last review (e132d34)
Posting findings

Findings — `b13f584` + `03ec46d`

All prior blockers and suggestions remain resolved. This round covers the two new commits.

`b13f584 test: authenticate GitHub PR commit fixtures`

mockGitHubAddPRCommits in api-client.ts now seeds both a workspace connection (legacy_shared/active) and a personal connection (e2e-test) for the active workspace after posting PR commit fixture data.

This follows the same backward-compatibility pattern established in e132d34 for mockGitHubAddBranches (workspace connection only) and in mockGitHubSetUser (both workspace and personal). The difference — adding a personal connection here that mockGitHubAddBranches omits — is correct: PR commit views can trigger personal-auth lookups (e.g., checking the personal actor for review context), whereas branch lookups only require workspace automation credentials. The current guard and if (!workspaceId) return short-circuit are consistent with the helper pattern. ✓

`03ec46d fix: improve GitHub authentication settings UX`

Six files changed. This is a UX refactoring that moves the inline PAT/CLI/App forms into a modal dialog (GitHubConnectionDialog) and moves the capability badges into a separate modal (GitHubPermissionsDialog).

github-connection-dialog.tsx (new, 299 lines)

CLIForm correctly cancels in-flight fetchGitHubCLIAccounts with the current flag. ✓
handleOpenChange(true) calls setMethod(methodForStatus(status)) to reset the method selector to the current connection source on re-open. ✓
fetchGitHubCLIAccounts import from @/lib/api/domains/github-api resolves via the export * from "./github-auth-api" re-export. ✓
All three form sub-components are correctly gated by the method state: PAT and CLI forms call onSaved() (which closes the dialog), App form calls onAppInstall and stays open pending the external OAuth redirect. ✓

github-permissions-dialog.tsx (new, 88 lines)

permissionStates merges capabilities (map of name → bool) with missing_capabilities and missing_permissions (arrays), setting the missing entries to false. Handles duplicates correctly (a capability present in both capabilities and a missing list ends up as false either way). ✓
Both early-return guards (source !== "github_app_installation", permissions.length === 0) are correct. ✓

github-status.tsx (refactored, 365 lines)

All six @tabler/icons-react imports remain used after the code movement. ✓
AutomationStatusSummary correctly shows the human-identity note only for non-App connections where effective_personal_actor?.kind === "human" — the exact case where the automation credential also powers My GitHub. ✓

GitHubPersonalSettings — conditional render on load

During the loading window (!loaded || loading || !status) the SettingsSection is always rendered regardless of the eventual automation source. For PAT and gh CLI connections the section returns null once status resolves, so there is a brief flash-then-disappear on the first page load. This is a minor layout shift that wasn't present before (previously the section was always shown, with a description that adjusted per source). In practice the two useGitHubStatus hooks for automation and personal settings likely resolve simultaneously so the window is short, but it's observable on slower connections.

Suggestion (doesn't block)

Brief loading flash in GitHubPersonalSettings — apps/web/components/github/github-status.tsx:297-305

The loading state always renders the SettingsSection header; PAT/gh CLI connections then return null after status resolves. This causes a one-tick layout shift. One option: keep the loading guard as if (!loaded || loading || !status) return null; (skip the loading UI entirely, since the parallel GitHubAutomationSettings already shows a loading state). The personal section appearing instantaneously once loaded is fine UX.

E2E tests (github-authentication.spec.ts, mobile-github-auth-settings.spec.ts)

Updated correctly for the new dialog-based UI: "Change connection" button opens the method selector dialog, "View permissions" opens the capabilities dialog, capabilities asserted inside the dialog before closing. Both desktop and mobile test cases updated. ✓

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1

Verdict: ready to merge. No blockers. The UX refactoring is clean — the dialog extraction removes visual complexity from the settings page without regressions in behavior or coverage. The loading flash suggestion is low-risk to fix or defer.

claude · 2026-07-20T19:17:54Z

Claude finished @carlosflorencio's task in 1m 48s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews
Reading code-review skill and checking new commits
Reviewing new changes since last review (03ec46d → a88630d)
Posting findings

Findings — `a88630d` (Merge remote-tracking branch 'origin/main')

All prior blockers and suggestions remain resolved. This round covers the merge commit only.

What changed

This is a merge of origin/main into the feature branch, with a single stated conflict in docs/decisions/INDEX.md. The merge brings in ~199 files of main-branch changes (layout profiles, workspace git status, repository trust, etc.) that are entirely orthogonal to the GitHub auth work.

Conflict resolution — docs/decisions/INDEX.md ✅

The file correctly includes both entries in the table:

PR's ADR: | 0047 | Separate GitHub deployment, workspace automation, and personal identities | accepted | ... | 2026-07-19 |
Main's new ADR: | 2026-07-20-explicit-local-repository-trust | Explicit Local Repository Trust | accepted | ... | 2026-07-20 |

No entries were dropped or duplicated.

Semantic intersection check — four files were modified by both the PR and main and are worth confirming:

apps/backend/cmd/agentctl/main.go — main added ListenHost() binding; PR added GitHub credential env injection. The merge correctly includes both. ✅
apps/backend/internal/agentctl/server/config/config.go — main added ListenHost() and ConsumeNonce; PR added GitHub auth config fields. No overlap. ✅
apps/backend/internal/orchestrator/executor/executor_test.go — main added TestMockRepositoryGetTaskSessionReturnsDetachedMutableFields; PR added the concurrent-launch synchronization gate. Both additions are present and independent. ✅
apps/backend/internal/task/service/repository_discovery.go — main's fix(repositories): allow explicit paths outside discovery roots (#1825) rewrote ValidateLocalRepositoryPath to use resolveExplicitLocalRepositoryPath. PR had already modified this file for workspace-scoped GitHub context. No overlap in the changed lines. ✅

No semantic conflicts, no regression introduced.

Open from Round 10

Brief loading flash in GitHubPersonalSettings — apps/web/components/github/github-status.tsx:297-305: during the loading window the section header renders then disappears when status resolves for PAT/gh-CLI connections. Addressable in a follow-up.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0

Verdict: ready to merge. The merge commit is clean — conflict resolved correctly, no semantic regressions, all prior blockers resolved.

claude · 2026-07-20T23:09:22Z

Claude finished @carlosflorencio's task in 1s —— View job

I'll analyze this and get back to you.

claude · 2026-07-20T23:25:41Z

Claude finished @carlosflorencio's task in 0s —— View job

I'll analyze this and get back to you.

claude · 2026-07-20T23:57:00Z

Claude finished @carlosflorencio's task in 6m 27s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews (11 rounds, last head a88630d, verdict: ready to merge)
Reading code-review skill file
Reviewing new commits since last review (b7afcc4, 7f1028b, 1e3e058)
Posting findings

Findings — `b7afcc4` + `7f1028b` + `1e3e058`

All prior blockers and suggestions remain resolved. This round covers three new commits.

`b7afcc4 feat: add guided GitHub App setup`

Large feature commit adding the GitHub App Manifest flow for deployment App self-registration. Key components: DeploymentAppRegistrationService, DeploymentAppRepository, deployment_app_manifest.go, ManifestConversionClient, controller_app_registration.go, and the System Settings frontend page.

What looks good:

State hash security — Only the SHA-256 of the raw state is persisted at Start time; the raw state is returned to the caller and never stored. consumeRegistrationFlow hashes the incoming state and matches against the stored digest. Prevents DB-read attacks on the registration flow. ✓
Single-use consumption — ConsumeDeploymentAppRegistrationFlow uses UPDATE ... WHERE consumed_at IS NULL AND state_hash = ? AND EXISTS (head), so only one goroutine can consume a given flow and a stale replay attempt returns ErrDeploymentAppFlowUnavailable. ✓
Staleness under concurrent starts — CreateDeploymentAppRegistrationFlow marks all active flows consumed in a transaction before inserting the new head. If Complete is racing with a second Start, IsLatestDeploymentAppRegistrationFlow catches the stale attempt. ✓
Policy validation after conversion — matchesDeploymentAppPolicy re-derives the canonical manifest to compare permissions/events, preventing a GitHub App that granted fewer permissions than required from being activated. ✓
Generation monotonicity — SaveManagedRegistration verifies CredentialGeneration == previous + 1 and that the initial generation is 1, preventing credential sequence tampering. ✓
Lock ordering — deploymentAppMutationMu → s.mu with no reverse path; swapDeploymentAppRuntime only acquires s.mu and is always called by code that already holds deploymentAppMutationMu. No deadlock risk. ✓
Authenticates before Handle — Both operate on the same []byte payload; no body double-read issue. ✓
Input bounds on callback — State validated by validGitHubCallbackState (exact entropy length + base64), code bounded at 1024 bytes, providerError at 256 bytes. ✓
Referrer-Policy on callback redirect — redirectDeploymentAppCallback sets Referrer-Policy: no-referrer, preventing the conversion code from leaking in Referer headers. ✓
Frontend submitManifestToGitHub — POST form with the manifest as a JSON field; registration_url is always a hardcoded GitHub path from manifestRegistrationURL, not user-controllable. ✓

Suggestion (doesn't block)

HandleAppWebhook joins the DB health-update error into the return value — service_app_auth.go

return result, errors.Join(err, healthErr)

If the webhook is processed successfully (err == nil) but the DB health update fails transiently (healthErr != nil), errors.Join(nil, healthErr) is non-nil, httpGitHubAppWebhook writes a 500 response, and GitHub marks the delivery failed and retries. The in-memory health update (s.updateDeploymentAppWebhookHealth) always runs regardless of healthErr, so the DB persistence is secondary. Logging healthErr instead of joining it into the return removes the spurious retry:

if healthErr != nil && s.logger != nil {
    s.logger.Warn("deployment App webhook health persist failed", zap.Error(healthErr))
}
return result, err

`7f1028b fix: satisfy GitHub backend lint`

Two targeted fixes:

mock_controller.go:133 — req.InstallationAccountType default changed from "Organization" (bare string) to string(DeploymentAppOwnerOrganization) (typed constant). ✓
store.go — Added deploymentAppPersistenceMu sync.Mutex field and reformatted struct alignment. ✓

No issues.

`1e3e058 test: update mobile GitHub App setup flow`

mobile-github-auth-settings.spec.ts — Updated assertion to reflect the new App-unavailable UX: instead of checking for the old tooltip "GitHub App requires deployment configuration.", the test now selects "GitHub App" from the combobox, then verifies the inline message "This Kandev deployment does not have a GitHub App yet..." and the "Set up deployment App" link are visible. This matches the github-connection-dialog.tsx UX changes in b7afcc4. ✓

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1

Verdict: ready to merge. No blockers. The guided App setup feature is well-implemented — state hash security, single-use consumption, policy validation, and lock ordering are all correct. The health-update error joining is a low-risk suggestion to address whenever convenient.

claude · 2026-07-21T00:16:12Z

Claude finished @carlosflorencio's task in 1m 35s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews (12 rounds, last head 1e3e058, verdict: ready to merge)
Reading code-review skill file
Reviewing new commit b6a17cc — fix: avoid retrying processed GitHub webhooks
Posting findings

Findings — `b6a17cc fix: avoid retrying processed GitHub webhooks`

All prior blockers and suggestions remain resolved. This round covers the single new commit (3 files: service_app_auth.go, webhook_service.go, deployment_app_registration_service_test.go).

What changed

This commit implements the suggestion from Round 12 (health-update error causing spurious GitHub webhook retries).

HandleAppWebhook moved from service_app_auth.go → webhook_service.go

The function is now in the more semantically appropriate file. The logic change is the exact fix suggested:

Before:

return result, errors.Join(err, healthErr)

After:

if healthErr != nil && s.logger != nil {
    s.logger.Warn("deployment App webhook health persistence failed", zap.Error(healthErr))
}
return result, err

When err == nil (webhook processed successfully) and healthErr != nil (transient DB failure), the old code returned a non-nil error causing GitHub to mark the delivery failed and retry. The new code logs the DB failure as a warning and returns only the actual webhook processing result. The in-memory health update runs unconditionally regardless of DB persistence, so the DB failure has no observable impact on the running service state.

nil check consolidation — the two-step nil guard was simplified to runtime == nil || runtime.webhookAuth == nil, which is equivalent and cleaner.

TestDeploymentAppWebhookHealthPersistenceFailureDoesNotRetryProcessedDelivery — covers the exact scenario: drops github_app_registration to force DB persistence failure, sends a valid signed webhook, and asserts:

HandleAppWebhook returns nil error (not the DB failure)
In-memory health is DeploymentAppWebhookVerified with a non-nil lastWebhookAt

This is tight, targeted coverage for the precise failure mode.

No regressions, no dead code. The errors import in service_app_auth.go remains needed (still used at lines 680, 687 and elsewhere); the zap import addition to webhook_service.go is correct.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1
Round 13	`b6a17cc`	0	0

Verdict: ready to merge. All blockers and suggestions are resolved. The final commit is a minimal, correct fix with targeted regression coverage.

claude · 2026-07-21T08:52:49Z

Claude finished @carlosflorencio's task in 3m 59s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews (13 rounds, last head b6a17cc, verdict: ready to merge)
Reading code-review skill file
Reviewing new commits since last review (8ebcd2d — Merge remote-tracking branch 'origin/main')
Posting findings

Findings — `8ebcd2d` (Merge remote-tracking branch 'origin/main')

All prior blockers and suggestions remain resolved. This round covers the merge commit only.

What changed

This merge brings in c2a678b feat: add Azure DevOps integration (#1778) from main, which was independently reviewed and merged. The overlap between the two features spans nine files; all intersections are clean.

apps/backend/internal/repoclone/clone.go

Two semantic changes from the Azure DevOps PR compose correctly with the GitHub auth work:

WorkspaceRepoPath: owner is removed from the strict validatePathSegment list and handled instead by a new validateOwnerPath that permits slash-separated segments (org/project for Azure DevOps). GitHub owners (no slash) pass through unchanged. ✓
workspaceCloneAuth: the provider != githubProvider early-return was moved before the CloneURL(provider, ...) call, so Azure DevOps URLs are returned as-is instead of being fed through a GitHub-only URL constructor. GitHub credential resolution is unchanged. ✓
New EnsureWorkspaceClonedWithBasicAuth + gitCmdWithHTTPHeader: Basic Auth header is injected via GIT_CONFIG_KEY/VALUE child environment variables, not command-line args — confirmed by TestGitCmdWithHTTPHeaderKeepsCredentialOutOfArguments. Header is scoped to the specific repo URL (http.<authURL>.extraHeader) preventing cross-host leakage. ✓

apps/backend/internal/backendapp/services.go / main.go

Azure DevOps service wired alongside GitHub/GitLab using the same Provide → lifecycle → poller pattern. No interaction with the GitHub auth credential-resolver or broker. ✓

apps/web/hooks/domains/github/use-github-status.ts

useEffect(() => subscribeIntegrationAvailability(doFetch), [doFetch]) added. When any integration's availability changes (including Azure DevOps connecting), the GitHub status is re-fetched. This causes one extra roundtrip on first Azure DevOps connect but is intentional — integration health events are a shared cross-provider signal. Cleanup is correct (the subscribe return value is the unsubscribe function). ✓

apps/web/hooks/domains/integrations/use-remote-repositories.ts (new)

Replaces useAccessibleRepos (GitHub-only) with a unified multi-provider hook. GitHub fetch uses fetchAccessibleRepos({ workspaceId, ... }) — preserving the workspace-scoped auth model from this PR. workspaceId change correctly clears all caches via the useEffect([workspaceId]) cleanup. ✓

apps/web/hooks/domains/github/use-branches-by-url.ts

branchRequestForURL extended for GitLab and Azure DevOps URLs. GitHub and Azure DevOps branches remain gated on workspaceId; GitLab does not require one. The workspaceId-change effect clears all cached entries, so stale no-workspace "empty branches" records are evicted when workspace subsequently appears. ✓

apps/web/components/task-create-dialog-remote-repo-chips.tsx

Switch from useAccessibleRepos to useRemoteRepositories preserves the workspace-scoped GitHub auth; makeURLChange now also sets providerRepoId/providerOwner/providerName from the RemoteRepository shape. ✓

docs/decisions/INDEX.md

New Azure DevOps ADR entry added correctly; all three existing entries from the GitHub auth PR are present. ✓

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1
Round 13	`b6a17cc`	0	0 — ready to merge
Round 14	`8ebcd2d`	0	0

Verdict: ready to merge. The merge commit is clean — Azure DevOps integration composes correctly with the GitHub workspace-scoped auth model, all nine overlapping files were resolved correctly, and all prior blockers and suggestions remain resolved.

claude · 2026-07-21T09:41:19Z

Claude finished @carlosflorencio's task in 2m 43s —— View job

PR Review: feat: add workspace-scoped GitHub authentication

Gathering context from prior reviews (14 rounds, last head 8ebcd2d, verdict: ready to merge)
Reading code-review skill file
Reviewing new commit 23c0e22 — fix(web): isolate GitHub status by workspace
Posting findings

Findings — `23c0e22 fix(web): isolate GitHub status by workspace`

All prior blockers and suggestions remain resolved. This round covers the new commit only (13 files changed).

What changed

This commit migrates the GitHub status state from a single-slot model ({ workspaceId, status, loaded, loading }) to a per-workspace map ({ byWorkspaceId: Record<string, GitHubStatusEntry> }), enabling true multi-workspace GitHub status isolation.

types.ts / github-slice.ts

GitHubStatusState.byWorkspaceId replaces the single workspaceId/status/loaded/loading fields. The new GitHubStatusEntry type holds the per-workspace state.
setGitHubStatus / setGitHubStatusLoading now look up the entry by key and no-op when the key hasn't been initialized — the safety valve for late responses before resetGitHubStatus is called.
resetGitHubStatus creates a fresh null entry under the given workspace key without touching other workspace entries. The | null overload is removed (the hook short-circuits at if (!workspaceId) return; before reaching the action).
applyGitHubRateLimitUpdate iterates all workspace entries but only applies the update to legacy_shared connections. WS rate-limit events are global (not per-workspace), so workspace-scoped connections (PAT, App) correctly derive their rate limits from HTTP fetches instead. The new github.test.ts test asserts this filter works — legacy connections get the update, others don't. ✓

use-github-status.ts

The hook now selects only the specific workspace's entry (state.githubStatus.byWorkspaceId[workspaceId]) rather than the whole slice. The initialization guard changes from statusState.workspaceId !== workspaceId (single-slot identity check) to !statusState (entry doesn't exist yet), which is correct and simpler. The dependency array uses optional chaining (statusState?.loaded, statusState?.loading) to handle the brief undefined window before initialization. ✓

workspaces-group.tsx

WorkspaceIntegrationItems previously called useGitHubStatus() (active workspace), so all sidebar workspace items showed the same workspace's GitHub status. The fix passes the per-item workspaceId. With the new map, each sidebar item tracks its own workspace's GitHub status independently. ✓

pr-ci-popover.tsx / pr-merge-button.tsx

Both previously read state.githubStatus.status directly from the store (a raw single-slot read) and PRCIPopover had an extra side-effect useGitHubStatus() call to ensure the load was triggered. Both now use useGitHubStatus(workspaceId) — the hook handles initialization and they no longer read the old single-slot field. The redundant side-effect call is removed. ✓

pr-ci-popover.automation.test.tsx

The mock changed from vi.fn() (which returns undefined by default, crashing when destructuring { status }) to a constant () => ({ status: null }). This was likely always broken or required per-test setup that was never added. The new constant mock is correct for automation tests where auth state isn't the focus. ✓

page.tsx (SSR hydration)

Updated to write into byWorkspaceId[statusWorkspaceId] instead of the flat top-level fields, preserving the SSR hydration optimization for the GitHub settings page. ✓

Tests

github.test.ts (new) — verifies the legacy_shared filter: rate-limit events land in the legacy workspace but not in a PAT workspace. ✓
github-slice.test.ts — updated tests now verify workspace isolation (ws-2's status is unaffected by ws-1's rate-limit update), and the stale-response test correctly asserts that a late response is stored under its original workspace key rather than discarded. ✓
use-github-status.test.tsx — new test asserts the hook does not call fetchGitHubStatus when the workspace's entry is already loaded, even when another unscoped useGitHubStatus() renders simultaneously. ✓
github-authentication.spec.ts — the test no longer uses a setTimeout(350) sleep. It now uses a gate Promise + SPA navigation (pushState + kandev:navigation event) instead of a full-page goto(). The gate is released after the loading state is confirmed, making the test deterministic. ✓
mobile-create-task-remote-repo.spec.ts — adds workspace connection mock needed under the new workspace-scoped auth model. ✓

No blockers, no suggestions. The migration is complete, well-tested, and eliminates the last class of cross-workspace GitHub status bleed.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1
Round 13	`b6a17cc`	0	0 — ready to merge
Round 14	`8ebcd2d`	0	0 — ready to merge
Round 15	`23c0e22`	0	0

Verdict: ready to merge. The state migration from single-slot to per-workspace map is correct, covers all consumers, and includes thorough test coverage including a notably improved deterministic E2E test. All blockers and suggestions from prior rounds remain resolved.

claude · 2026-07-21T22:18:49Z

Claude finished @carlosflorencio's task in 0s —— View job

I'll analyze this and get back to you.

claude · 2026-07-21T23:49:59Z

Claude finished @carlosflorencio's task in 5m 35s —— View job

PR Review: feat: support per-workspace GitHub App registrations

Gathering context from prior reviews (15 rounds, last head 23c0e22, verdict: ready to merge)
Reading code-review skill file
Reviewing new commits since last review (23c0e22)
Posting findings

Findings — `4b6d603` + `5499ead` + `1b609bc` + `fc48a52`

All prior blockers and suggestions remain resolved. This round covers four new commits.

`4b6d603 feat(github): explain repository scope (#1842)`

Five files changed. Adds a RepositoryScopeHelp info button to the Repository Scope settings section with adaptive display: tooltip on desktop, a Drawer on mobile via useTouchDrawer.

The DrawerTrigger asChild wrapping is correct — the same button element is used for both the tooltip trigger and the drawer trigger, so accessibility props (aria-haspopup, aria-expanded, aria-label) correctly describe the dialog that opens. Section order also changed (Review Watch before Repo Scope), which has no behavioral impact. No issues.

`5499ead fix(github): keep draft pull requests out of merge-ready state (#1851)`

Eight files, 190 additions. Fixes the bug where GitHub reports mergeable_state=CLEAN for a draft PR, which would previously cause Kandev to treat the PR as ready to merge.

Backend — service_pr_watch.go

prepareTaskPRSyncState extracts the sync logic from SyncTaskPR and inserts:

if status.PR.Draft {
    nextMergeableState = "draft"
}

This writes the synthetic "draft" value to the DB, overriding whatever GitHub sent. All downstream consumers that read MergeableState correctly handle this value because they require == "clean":

event_handlers_github_ci_automation.go:523: pr.MergeableState != "clean" blocks auto-merge for drafts ✓
Frontend isPRReadyToMerge: mergeable_state !== "clean" returns false → merge button hidden ✓

Frontend — pr-task-icon.tsx

isPRDraft is a new exported helper:

export function isPRDraft(pr: TaskPR): boolean {
  return pr.state === "open" && pr.mergeable_state === "draft";
}

Correct — the state === "open" guard prevents a closed draft from triggering draft styling. This function is used in getPRStatusColor (before the openMergeBlockerColor/isPRReadyToMerge cascade), getPRTooltip (correctly placed before "Ready to merge"), PRStatusIcon, and pr-topbar-button.tsx.

Frontend — pr-status-chip.tsx

chipStatus returns "draft" after the dirty/behind checks. The rank draft: 0.5 is intentional: for multi-PR tasks where one PR is passed (rank 1) and one is draft (rank 0.5), the passed PR's status surfaces — correct, since the draft doesn't need attention.

Tests

Backend: TestSyncTaskPR_DraftOverridesCleanMergeableState — seeds a TaskPR with mergeable_state=clean, syncs a draft status, asserts stored state is "draft" and an event is published ✓
Frontend: pr-task-icon-draft.test.ts — draft PR with passing CI stays muted, tooltip contains "Draft" but not "Ready to merge" ✓
Frontend: pr-status-chip.test.tsx — chip status is "draft" and data-pr-ready-to-merge is "false" ✓
Spec updated to document draft exclusion from auto-merge ✓

No issues.

`1b609bc fix(github): harden managed clone command arguments`

Two files, resolves CodeQL alert #254.

Both clone() and cloneWithHTTPHeader() now place "--" before the user-controlled cloneURL, preventing a URL starting with - from being misinterpreted as a git option. The // lgtm[go/command-injection] annotation suppresses the alert at the right site, explaining that no shell interprets the args. Test verifies --\nhttps://... order:

if !strings.Contains(args, "--\nhttps://github.com/acme/private.git") {
    t.Fatalf("git args do not terminate clone options before the URL: %s", args)
}

No issues.

`fc48a52 chore: merge main and resolve credential conflicts`

Merge of 1b609bc (the clone harden commit) with c4a856d (CORS fix already in main) into the PR branch, resolving conflicts between the GitHub auth feature and the Azure DevOps integration (c2a678b) that was merged into main since the prior merge.

Conflict resolution — clone.go

The Azure DevOps integration added new clone paths (cloneWithHTTPHeader, ProviderRepoPath, WorkspaceProviderRepoPath). The merge correctly carries the -- terminator into all clone command constructions — both clone paths now have -- before the URL. This also resolves CodeQL alert #257 (which was on the Azure DevOps-added path). ✓

Conflict resolution — executor_credentials.go

GitLab constants (envGitLabToken, envGitLabHost, envKandevGitLabHost) added by the Azure DevOps merge are correctly integrated alongside the GitHub broker credential types from this PR. The GitHubCredentialLeaseIssuer interface and all broker env constants remain in internal/githubauth/ (centralized from Round 3's fix). ✓

service_pr_watch.go

The Azure DevOps main-branch fetchRequiredReviewsForTaskPR rename is preserved alongside the 5499ead draft state fix. Both changes compose correctly. ✓

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1
Round 13	`b6a17cc`	0	0 — ready to merge
Round 14	`8ebcd2d`	0	0 — ready to merge
Round 15	`23c0e22`	0	0 — ready to merge
Round 16	`fc48a52`	0	0

Verdict: ready to merge. The two security fixes (CodeQL clone-argument injection, CSRF via CORS state changes) are complete and correct. The draft PR fix is thorough end-to-end: backend normalizes GitHub's misleading CLEAN mergeability for draft PRs, and the frontend correctly hides the merge button, blocks auto-merge, and surfaces draft styling. All merge conflicts with main are cleanly resolved.

claude · 2026-07-22T00:48:12Z

Claude finished @carlosflorencio's task in 2m 42s —— View job

PR Review: feat: support per-workspace GitHub App registrations

Gathering context from prior reviews (16 rounds, last head fc48a52, verdict: ready to merge)
Reading code-review skill file
Reviewing new commit 310c74a — fix(github): address workspace credential CI regressions
Posting findings

Findings — `310c74a fix(github): address workspace credential CI regressions`

All prior blockers and suggestions remain resolved. This round covers the single new commit (8 files changed).

What changed

auth_resolver.go — Extracts automationCacheKey and applyAppCredentialMetadata as named helpers from the inline logic in resolveAutomation. Pure refactoring; logic is identical. Reduces resolveAutomation to a readable call sequence. ✅

credential_broker.go — Extracts issueConnection from the opening block of Issue. The connection load, nil guard, status check, and App registration generation lookup are unchanged; they're just in a named method. ✅

service_app_auth.go — Splits GetWorkspaceAuthStatus into four methods:

workspacePersonalConnection — nil guard around GetUserConnection when userID is empty
populateAppRegistrationStatus — early-return guard for non-App connections, then clone + runtime snapshot
populateEffectivePersonalActors — best-effort void fill (_, _ error discards for resolver calls, unchanged behavior)
GetWorkspaceAuthStatus — now a clean, flat sequence of the above

The return status, nil at the end of GetWorkspaceAuthStatus is correct — populateEffectivePersonalActors is intentionally void (best-effort display data, same as before). ✅

service_connections.go — Removes restorePATAfterFailedTransition. Confirmed no remaining references — dead code removal. ✅

store.go — Splits the 120-line initSchema into five named sub-functions that run in the same order:

initSchemaFoundations — table/column creation migrations
applyIdempotentSchemaColumns — ALTER TABLE ADD COLUMN calls (all _, _ discards, unchanged)
initSchemaUpgrades — watcher self-heal and App registration columns
initSchemaData — backfills and legacy-upgrade data migrations
applyIdempotentSchemaIndexes — CREATE INDEX IF NOT EXISTS (all _, _ discards)

Execution order is preserved; the ensureWorkspaceOwnershipIndexes call (which propagates errors) is still last. ✅

improvekandev/handler.go — Adds canonicalWorkspaceID that calls uuid.Parse and returns id.String(), producing canonical lowercase hyphenated UUID form. The httpBootstrap handler calls this immediately after the workspace_id presence check, rejecting with HTTP 400 before the value reaches resolveOrCloneRepo or ensureWorkflow. This resolves CodeQL alert #258. Test covers uppercase-to-lowercase normalization and traversal-shaped rejection. ✅

api-client.ts — Removes personal connection seeding from mockGitHubSetUser. The function now only seeds legacy_shared/active workspace connection, not a personal connection. The personal connection seeding remains in mockGitHubAddPRCommits (which still needs it because PR commit views can trigger personal-auth lookups). The removal fixes tests that were asserting "not authenticated personally" — those tests were unexpectedly getting a personal connection seeded by a background mockGitHubSetUser call. Tests that need a personal connection must seed it explicitly. ✅

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1
Round 13	`b6a17cc`	0	0 — ready to merge
Round 14	`8ebcd2d`	0	0 — ready to merge
Round 15	`23c0e22`	0	0 — ready to merge
Round 16	`fc48a52`	0	0 — ready to merge
Round 17	`310c74a`	0	0

Verdict: ready to merge. The commit is a correct cleanup pass: complexity-reducing refactors across four Go files (all logic-preserving), removal of confirmed dead code, a targeted UUID-validation fix for the remaining CodeQL alert, and a precise E2E mock correction that restores expected auth state in tests that don't intend to set up a personal connection.

claude · 2026-07-22T01:25:25Z

Claude finished @carlosflorencio's task in 2m 52s —— View job

PR Review: feat: support per-workspace GitHub App registrations

Gathering context from prior reviews (17 rounds, last head 310c74a, verdict: ready to merge)
Reading code-review skill file
Checking new commits since last review (583dd2f)
Reviewing new changes
Posting findings

Findings — `583dd2f test(github): align PR commit fixture with workspace auth`

All prior blockers and suggestions remain resolved. This round covers the single new commit (1 file changed: apps/web/e2e/helpers/api-client.ts).

What changed

mockGitHubAddPRCommits no longer seeds a personal connection for the active workspace. It now seeds only a legacy_shared/active workspace connection — the same pattern as mockGitHubAddBranches.

The personal connection seeding was added in Round 13 (b13f584) under the reasoning that PR commit views "can trigger personal-auth lookups." That reasoning was overly conservative. GetPRCommitsForWorkspace calls resolvePersonalReadClient → resolveServiceClient(CredentialPurposePersonalRead) → resolver.Resolve, which invokes resolvePersonal. When no personal connection exists and the workspace uses legacy_shared, resolvePersonal returns nil, nil (line 191 of auth_resolver.go) — because ConnectionSourceLegacyShared is a human-PAT-like identity that can supply automation semantics. Resolve then falls through to resolveAutomation, which uses the legacy shared client. A personal connection is not required when the mock workspace is legacy_shared.

The removal is correct for two reasons:

legacy_shared workspace connections already provide authenticated-viewer semantics for commit lookups — no personal connection needed.
Tests that check "not authenticated personally" (e.g., personal auth status assertions) were at risk of unexpected pollution when mockGitHubAddPRCommits ran in their setup path and deposited a personal connection they didn't request. This mirrors the same fix applied to mockGitHubSetUser in 310c74a.

The commit is a correct, minimal correctness fix with no behavior change for any test that doesn't explicitly check personal auth state.

Summary — all rounds

Round	Head	Blockers	Suggestions
Round 1	`666a2d3`	3	2
Round 2	`da46586`	0	2
Round 3	`b367a33`	0	1
Round 4	`531c8a8`	0	0 — ready to merge
Round 5	`5e22fa4`	0	0
Round 6	`210a6bc`	0	1
Round 7	`e46c711`	0	0
Round 8	`b4f4519`	0	0
Round 9	`e132d34`	0	0
Round 10	`03ec46d`	0	1
Round 11	`a88630d`	0	0 — ready to merge
Round 12	`1e3e058`	0	1
Round 13	`b6a17cc`	0	0 — ready to merge
Round 14	`8ebcd2d`	0	0 — ready to merge
Round 15	`23c0e22`	0	0 — ready to merge
Round 16	`fc48a52`	0	0 — ready to merge
Round 17	`310c74a`	0	0 — ready to merge
Round 18	`583dd2f`	0	0

Verdict: ready to merge. The new commit correctly removes an unnecessary personal connection fixture that was seeded by mockGitHubAddPRCommits. For legacy_shared workspace connections, PR commit lookups fall back to automation credentials when no personal connection exists — the credential resolver's resolvePersonal returns nil, nil for ConnectionSourceLegacyShared, triggering the automation fallback. All prior blockers and suggestions remain resolved.

claude · 2026-07-24T19:26:14Z

Claude finished @carlosflorencio's task in 0s —— View job

I'll analyze this and get back to you.

claude · 2026-07-24T22:27:21Z

Claude finished @carlosflorencio's task in 0s —— View job

I'll analyze this and get back to you.

claude · 2026-07-25T18:45:19Z

Claude finished @carlosflorencio's task in 1s —— View job

I'll analyze this and get back to you.