fix(configuration): provision the GitHub origin source binding - #2221
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46d69b55d2
ℹ️ 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".
| let operation = if github.credential.is_anonymous() { | ||
| GitHubReviewReadOperationV1::RestListPullRequestReviewComments | ||
| } else { |
There was a problem hiding this comment.
When no credential is available, this routes production ingestion through the REST review-comments endpoint, whose payload has no thread-resolution field; rest_lifecycle therefore never emits Resolved. Consequently, comments from resolved threads remain non-resolved, and delivery.rs filters only Resolved at line 1124, presenting those comments as unresolved review attention. Anonymous reads need a truthful lifecycle/coverage state rather than treating unavailable resolution data as unresolved.
AGENTS.md reference: AGENTS.md:L7-L12
Useful? React with 👍 / 👎.
| if !credential.is_anonymous() { | ||
| Self::Bound | ||
| } else if discovery == Some(&GitHubExactCommitDiscoveryOutcomeV1::Denied) { | ||
| Self::DeniedNoCredential |
There was a problem hiding this comment.
bound
If GH_TOKEN, gh auth token, or the credential helper returns an expired token or one without access to this repository, discovery returns Denied, but this branch reports Bound solely because the credential is non-anonymous and supplies no remedy. Thus tracedecay status claims that a credential authorizes reads precisely when GitHub has rejected it; the denied credential needs its own truthful state/remediation.
AGENTS.md reference: AGENTS.md:L7-L12
Useful? React with 👍 / 👎.
| let Some((owner, repository)) = | ||
| tracedecay_runtime_core::git::git_remote_url(&target.project_root) | ||
| .as_deref() | ||
| .and_then(github_repository_from_remote_v1) | ||
| else { | ||
| return Ok(current); |
There was a problem hiding this comment.
When a checkout previously bound to GitHub has origin removed or changed to a non-GitHub host, this early return leaves binding.tracedecay-daemon.github-origin durably authorizing the old repository. That contradicts the documented promise in docs/USER-GUIDE.md:885-890 that the binding follows origin and leaves a stale source authority indefinitely; this path should unbind the daemon-owned binding while preserving operator-owned bindings.
AGENTS.md reference: AGENTS.md:L169-L171
Useful? React with 👍 / 👎.
| pub fn record_github_source_status_v1(project_root: &Path, status: GitHubSourceStatusV1) { | ||
| github_source_status_registry_v1() | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner) | ||
| .insert(project_root.to_path_buf(), status); |
There was a problem hiding this comment.
This process-global registry only inserts or replaces entries and exposes no removal path. If a checkout records a GitHub source and is later reopened after removing or repointing origin to another host, GitHub discovery no longer records anything, so tracedecay_status continues returning the old repository, credential state, and PR indefinitely instead of not_observed. Reconcile or remove this entry on every project reopen/owner teardown.
AGENTS.md reference: AGENTS.md:L7-L12
Useful? React with 👍 / 👎.
46d69b5 to
7e84af6
Compare
1 participant
Fixes #2159
Root cause
Pull-request discovery never ran for a project unless an operator did two things by hand. Four gaps sat behind that, and a fifth bug stopped review ingest once discovery worked.
binding.tracedecay-daemon.project-open.ConfiguredGitHubSourceAccessAuthorityV1found no GitHub binding, so discovery loggedoutcome="not_attempted" source_access=Denied.[[github_review_sources]]entry answeredProfileGitHubReadOnlyCredentialMountOutcomeV1::NotConfigured. That became theGitHubCredentialNotConfiguredDelivery gate, before any read.403withx-ratelimit-limit: 0and no usable checkpoint. The code mapped that toDenied, so a public repository was hard-denied without a token. The review-threads read is GraphQL as well, so review ingest had the same problem.BindSourceappended the new binding (andUpsertAccessRuleappended the new rule), leaving the list out of canonical order.ConfigurationValueV1::validaterequires canonical order, and only apply built the resulting snapshot. So the dry run returned a plan that apply then refused withsource binding order is not canonical.(repository, commit, path, lines)alone, so every comment on the same lines shares it.resolve_stored_seedstill compared the stored seed'scomment_id. The second comment on a line (any reply, here4069777906answering theNit:) resolved toNone.resolve_manythen failed the batch, and the whole PR's review read becameUnavailable.What changed
Provisioning. Project open (both
tracedecay initand every later open) binds theoriginremote's GitHub repository asbinding.tracedecay-daemon.github-origin. It goes through a compare-and-swap daemon write,publish_daemon_source_binding, which also replacedrebind_daemon_project_source_binding.originwhen the remote changes.github_repository_from_remote_v1, and the daemon's copy was deleted.Credential. The local-login token source tries
GH_TOKEN, thengh auth token, thengit credential fillforhttps://github.com. The git probe runs with terminal prompts disabled and sends only the protocol/host request. A repository the profile does not name is read through that credential, anonymously when none exists. TheGitHubCredentialNotConfiguredgate lost its only producer and was deleted.Anonymous discovery. Without a token, discovery uses the REST issue search
repo:{owner}/{repo} is:pr head:{branch}. It then reads each candidateGET /repos/{owner}/{repo}/pulls/{n}to pin the exact head commit and head repository, which also finds fork heads. REST401/404/422map toDenied. The GraphQL route is unchanged for a credential. Review ingest for an anonymous source usesRestListPullRequestReviewComments, and the pull-request identity read follows either review read.Typed state.
GitHubSourceStatusV1carriesstate(bound | unauthenticated_public | denied_no_credential),remedy, the discovery outcome, the PR number, and the head repository. It is recorded at advisory mount, served asgithub_sourcebytracedecay_status, and printed bytracedecay status. It is not in doctor; status is the only surface.One validator for preview and apply.
protected_change_snapshot_v1(global-db) derives the candidate snapshot for the dry run, for protected apply, and for daemon binding writes.BindSourceandUpsertAccessRulenow insert in canonical order.Anchors. The stored code anchor matches on location only. Per-comment author, body and URL anchors come from each comment's own seed.
Fixtures. Everything captured was cached; no test reaches GitHub. In
fork_head_pull_request.json(Add Error::new_with_backtrace dtolnay/anyhow#463), all captured without a credential:422search refusal for an unseen repository.Also added:
rust_lang_log_741_review_comments.rest.json, which is anonymous REST.Hermetic harness tests. The two harness journeys that used a
github.comremote now use a non-GitHub host, so no test reaches GitHub. Docs:USER-GUIDE.md(GitHub source and pull-request discovery) andSECURITY.md(outbound access and credentials).Fail before / pass after
Each "before" run is this branch with only the named fix reverted.
discovery::tests::anonymous_discovery_finds_a_fork_headed_pull_request_by_rest_head_ref_search, which runs anonymously against the cached anyhow#463 answers. Before, with the scan routed to GraphQL as on master:ok, withGitHubSourceStatusV1 { state: UnauthenticatedPublic, pull_request_discovery: Found, pull_request: Some(463), head_repository: Some("sb123sb123/anyhow"), .. }. No GraphQL request is sent.config::tests::runtime_configuration_cutover::fresh_open_binds_the_github_origin_as_the_project_github_source, a fresh open of a checkout whose origin ishttps://github.com/dtolnay/anyhow.git. Before:ok. A reopen keeps the revision, and a remote moved togit@github.com:rust-lang/log.gitrebinds.configuration::operations::tests::protected_dry_run_refuses_what_apply_refuses_with_the_same_reason: unbinding an absent binding. Before:left: Ok(ProtectedChangePlan { … }),right: Err(PlanStale). After:ok, and the preview equalsprotected_change_snapshot_v1's refusal.domain_suite::configuration_contract::bind_source_inserts_in_canonical_binding_order, which bindsbinding.github.rust-lang-logbeforebinding.tracedecay-daemon.project-open. Before:ok.runtime_acceptance_suite::advisory_runtime_acceptance::retained_review_body_expansion_rechecks_exact_scope_and_source_access, using the realProjectGitHubAnchorAuthorityV1with a reply seed on the same lines. Before:panicked at …advisory_runtime_acceptance.rs:661:10: canonical body anchors. After:ok.ok):credentialed_discovery_reads_the_head_ref_graphql_query;anonymous_discovery_of_a_repository_github_will_not_show_is_denied_no_credential;delivery::tests::anonymous_rest_review_comments_publish_the_pull_request_lane;protected_bind_source_preview_and_apply_share_one_outcome: preview, then apply →effect.protected_preview_redacts_the_change_and_refuses_stale_or_invalid_input, previewing an absent-binding unbind is now refused withconfiguration.stale, which is what apply answers.Runtime journey
Debug
tracedecay-cli --no-default-features --features production. One daemon undersystemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G. IsolatedHOME/XDG_*, withGH_TOKENunset,gh auth statusreportingYou are not logged into any GitHub hosts, and no profileconfig.toml.In the dashboard, Delivery → Journey for the admitted PR serves 56 episodes over 3 lanes. The CI / review lane holds the three review comments on
.github/workflows/main.yml:135/113/113, thereview comments read · completeobservation, and Next actionREVIEW · Unresolved review/NEW REV.Before this branch, the same journey logged
github_pull_request_discovery outcome="not_attempted" source_access=Denied. The #2173 journey needed a hand-added binding and a profile[[github_review_sources]]entry, and/api/delivery/overviewreadpull_requests: not_published.Checks
cargo test --lib, after rebase:tracedecay-application: 476 passed.-configuration: 42.-dashboard-api: 172.-domain: 222.-global-db: 374.-mcp: 392.-project: 37.application_suiteanddomain_suiteran before the rebase; the other two after):application_suite: 64 passed.domain_suite: 171.runtime_acceptance_suite -- advisory_runtime_acceptance: 5.tracedecay --lib -- production_harness::{configuration_protected_preview_journey_test, delivery_read_gate_journey_test, advisory_cycle_language_journey_test}: 5.cargo clippy -p tracedecay-domain -p tracedecay-global-db -p tracedecay-configuration -p tracedecay-application -p tracedecay-project -p tracedecay-mcp -p tracedecay-cli -p tracedecay-dashboard-api -p tracedecay --all-targets --features tracedecay/test-helpers -- -D warnings: clean.cargo fmt --all -- --check: clean.Left open
git_hub_reviewfailedwhenever a review read ingests items during the cycle.AdvisoryFindingValidityWindowV1::validate_forrefuses items withobserved_at > valid_at, and a same-cycle refresh always observes aftervalid_at. The Delivery lane is unaffected.