Guarantee repository identity before first persistence - #36
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3259174a26
ℹ️ 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 remoteRepository: string; | ||
| try { | ||
| remoteRepository = resolveGitHubRepository(local.remoteUrl, enterpriseHost).repository; | ||
| } catch { |
There was a problem hiding this comment.
Validate the effective push repository before persisting
When remote.origin.url names repository A but remote.origin.pushurl names repository B, inspectCurrentGitState returns both endpoints, yet this preflight validates only local.remoteUrl and persists A as the canonical identity. The task is therefore durably created despite a conflicting effective publication destination; the later GitHub adapter rejects this mismatch, leaving the new task stranded rather than failing before its first write. Compare the existing push-URL handling in src/adapters/git.ts:287-300 and validate that local.pushUrl resolves to the same repository before adding the manifest entry.
Useful? React with 👍 / 👎.
| let local; | ||
| try { | ||
| local = await inspectCurrentGitState(input.workspacePath, "origin"); | ||
| } catch { |
There was a problem hiding this comment.
Inspect origin independently of branch state
In a valid GitHub-backed checkout with a detached HEAD, this call rejects even though the Git root and origin identity are available, because inspectCurrentGitState first runs git symbolic-ref --quiet --short HEAD; git symbolic-ref -h describes --quiet as suppressing the error for “non-symbolic (detached) refs.” An unborn repository similarly fails at rev-parse HEAD. Consequently configured Codex returns unassigned before persistence for branch-state problems unrelated to the repository-identity guarantee; read and validate the remote configuration without requiring an attached branch or existing HEAD.
Useful? React with 👍 / 👎.
| ## Current blockers | ||
|
|
||
| - Publication authority is required before commit, push, or PR creation; absent authority blocks at the Level 2 publication boundary while permitted Level 1 local work may still return a verified result. | ||
| - Publication authority is required before commit, push, or PR creation; this candidate remains local and uncommitted. The full unit suite has one unrelated Windows line-ending byte assertion failure; targeted runtime/typecheck and all integration suites pass. |
There was a problem hiding this comment.
Update the checkpoint after committing the candidate
This checkpoint is itself included in commit efb23905, so the statement that the candidate “remains local and uncommitted” is already false, as is the next-action instruction to keep the commit unperformed. Because STATUS.md is the canonical continuation checkpoint, subsequent sessions will apply the wrong publication state and next action; record the candidate as committed while retaining any still-true push/PR/merge restrictions.
AGENTS.md reference: AGENTS.md:L93-L99
Useful? React with 👍 / 👎.
Invariant
For a configured Codex new task, Git root, origin, and canonical GitHub repository identity are inspected before the first durable persistence. Any failure returns unassigned/bootstrap/BLOCKED with zero durable task writes. Existing durable-task semantics remain unchanged.
Scope
Exactly four files are changed:
Evidence
The local full non-integration suite reports 750 passed, 9 skipped, and one unrelated Windows CRLF byte assertion failure at tests/memory/memory-git-transfer.test.ts:42. That file and the Memory subsystem are outside this PR; no Memory fix was made.
No merge is authorized by this PR task.