Make GitHub the sole hosted Git and review surface#17
Conversation
hudsonaikins
changed the title
Add a dry-run-first atomic migration command with source-to-target identity mapping, migrate the live review ledger, and reduce the inherited Fallow baseline without deleting runtime interface methods.
|
@codex review |
|
@codex review |
![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: 99d6a69f70
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const CONTROL_REFS = [ | ||
| "refs/tabellio/reviews", | ||
| "refs/tabellio/validations", | ||
| "refs/heads/entire/checkpoints/v1", |
There was a problem hiding this comment.
With platform/control refs now declaring Entire checkpoints at refs/heads/entire/checkpoints/v1, the Entire context adapter still emits ref: "entire/checkpoints/v1" for each checkpoint in scripts/providers/entire-ledger-provider.mjs, so context/evidence produced after this change points at a ref that the new control-ref transport will never publish or fetch. Consumers following checkpoint refs from context packets will fail to resolve them; align the adapter/tests/examples with the canonical ref.
Useful? React with 👍 / 👎.
| line: firstInteger(comment.line, comment.original_line, comment.position), | ||
| commit: nullableString(firstPresent(comment.commit_id, comment.original_commit_id)), | ||
| author: nullableLogin(comment.user, "reviewComment.user"), | ||
| resolvedBy: null, |
There was a problem hiding this comment.
For GitHub PRs with an already-resolved review thread, this hard-codes every inline comment as unresolved; reviewCommentFeedback maps resolvedBy: null to providerState: "open" and disposition: "pending", so a resolved GitHub conversation still creates a needs_triage blocker on first sync. The provider needs to source thread resolution from GitHub rather than forcing null.
Useful? React with 👍 / 👎.
| dismissed: state === "dismissed", | ||
| stale: false, |
There was a problem hiding this comment.
When GitHub returns a review with state DISMISSED, this leaves it non-stale even though dismissed is set; reviewFeedback only checks stale, then treats non-approved/non-change-request states as pending feedback, so a dismissed review becomes a new triage blocker instead of being ignored as inactive. Mark dismissed reviews stale/resolved or teach reviewFeedback to honor dismissed.
Useful? React with 👍 / 👎.
| title: requiredValue(changeRequest.title, "changeRequest.title"), | ||
| state: changeRequestState(changeRequest), | ||
| draft: changeRequest.draft === true, | ||
| mergeable: nullableBoolean(changeRequest.mergeable), |
There was a problem hiding this comment.
When GitHub returns mergeable: null while its background mergeability job is still running, this preserves the unknown state, but deriveStatus only blocks false and can mark a no-feedback PR with successful checks as ready before GitHub has determined whether it can merge. Treat null as validating/blocked or retry until a non-null value is available.
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| const safeError = new StackOperationExecutionError(intent.operation, error, [ | ||
| this.#env.FORGEJO_TOKEN, | ||
| this.#env.GITHUB_TOKEN, |
There was a problem hiding this comment.
When a stack operation relies on GITHUB_TOKEN already present in the environment instead of --token-file, runExternalCommand still passes that token through process.env, but this sanitizer only redacts this.#env.GITHUB_TOKEN. If git-spice or a credential helper echoes the inherited token in stderr, the failed receipt and CLI error can persist it despite the token-redaction guarantee; include process.env.GITHUB_TOKEN in the secret list or avoid passing it implicitly.
Useful? React with 👍 / 👎.
|
|
||
| async listReviews({ owner, repo, number }) { | ||
| positiveInteger(number, "number"); | ||
| const values = await this.#paginate(`${repoPath(owner, repo)}/pulls/${number}/reviews`); |
There was a problem hiding this comment.
When a reviewer requests changes and later approves the same PR, GitHub keeps both review records in this reviews feed. Sync imports every record, and reviewFeedback treats every changes_requested review as actionable, so the old superseded request can keep the cycle in changes_requested even though GitHub no longer blocks on it. Collapse reviews to the latest effective state or mark superseded reviews stale.
Useful? React with 👍 / 👎.
1 participant
What changed
origin.IntelIP/Tabellio-controlGitHub repository for review, validation, and Entire control refs.GitHubProvideris now the concrete integration.tabellio-review-cycle/v0.2with GitHub provider identity andGITHUB_TOKENauthentication.Tests,Fallow changed-code, andPackage dry-runGitHub checks on protectedmain.Why
The repository still carried self-hosted and provider-neutral surfaces after the product boundary moved to GitHub. This change aligns implementation, package contents, examples, docs, live control state, and branch protection with one supported hosted Git and review service.
Private control state remains separate from the public code repository, but its supported destination is now another private GitHub repository rather than an alternative host.
Developer impact
tabellio-review syncuses--remote-repoplus either--token-fileorGITHUB_TOKEN.--api-urlorGITHUB_API_URL.tabellio-review migratepreviews by default;--apply trueperforms the compare-and-swap ledger move.--numberidentifies the source cycle;--target-numberidentifies the corresponding GitHub PR.--remap-current trueis additionally required to move an already-migrated v0.2 cycle.--legacy-repo-id,--legacy-owner, and--legacy-remote-repomap renamed or moved source repositories to their GitHub target.workflow.controlProvider = "github"andworkflow.controlRemoteName = "control".Validation
npm run check: 74 tests passed plus every bundled contract/example check.audit --base origin/main --gate new-only: pass.npm pack --dry-run --json: 101 entries; no removed-provider file paths.refs/tabellio/reviewsboth resolve to55a7566a72277370868b8285f9fad4cb7dafc122.