feat(sync): add verifiable global-sync foundation#1985
Conversation
Investigation summary
|
Verification evidenceFresh local results on rebased head
The full global certificate is intentionally still red pending protected signer/release workflow deployment, transactional staging, profile/evidence migration, and real nightly history. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: 8cd3e257ed
ℹ️ 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".
| cli = importlib.import_module("pdd.cli").cli | ||
|
|
||
| cli.main( | ||
| args=["sync", "certify", *sys.argv[1:]], |
There was a problem hiding this comment.
When the protected pdd-sync-checker entry point is used, this dispatches to pdd sync with certify as the optional basename. In this commit certify is registered as a top-level command, while sync is a plain command rather than a group, so global checker invocations with options such as --repos or --merge-group are rejected before certification runs. Invoke the registered command path or register a real sync certify subgroup; otherwise the released checker cannot produce global certificates.
Useful? React with 👍 / 👎.
Implementer CI triageAcknowledged findings:
I will follow the strict TDD runbook: mine sibling/history evidence, add and push a red focused-test commit, then implement and verify with targeted tests, lint, regression-public, and a built-wheel smoke. xhigh review remains pending due to service quota. |
Implementer investigation evidencePre-fix sibling/history hunt completed before tests.
Scope decision: fix the sole routing function, sole full-report JSON sink, and canonical parser only; no unrelated |
Red TDD evidencePushed test-only commit Command run: Actual result:
Proceeding to the production fixes. |
Green CI regression evidencePushed:
Passed verification: I also ran direct pylint on Final branch is clean and pushed ( |
Implementer response to remaining CodeQL alertsP1 #1: clear-text logging of sensitive data —
|
Red TDD evidence — remaining CodeQL alertsPushed Investigation: the only remaining regexes in |
Implementer evidence — remaining CodeQL alertsPushed
A full pylint invocation over the legacy |
CodeQL follow-upThe new CodeQL run completed successfully: No remaining CodeQL annotations were reported for the two prior alerts. Branch remains clean and fully pushed at |
Foundation CI fix round 3 evidenceRed commit: The full unit lane exposed 37 failures:
Fix commit: The fix restores the boundary:
Verification:
|
Codex xhigh adversarial review — PR #1985 @ 63d3100Verdict: DON'T MERGE P1 (blocking)
P2 (non-blocking)
NotesReviewed against actual base |
Implementer response to Codex xhigh reviewP1 #1: Remote signer capabilities leak into untrusted candidate/test subprocessesStatus: FIXING in this PR P1 #2: Pytest support closure misses
|
|
To use Codex here, create an environment for this repo. |
Verification evidence — review findings resolvedFinal pushed head: TDD and implementation commits
Required outcomes
Verification
Worktree is clean; local HEAD and remote branch both equal the SHA above. |
Exhaustive code-quality reviewReviewed SHA: P1
P2
Verification and scope
|
Implementer response to xhigh review\n\nRe: https://github.com/promptdriven/pdd/pull/1985#issuecomment-4949554786\n\n### P1 #1: Candidate code can forge both checker-owned pytest result channels\nStatus: FIXING\nPlan: move authoritative collection/execution result derivation behind a protected process/namespace boundary and add an adversarial module that discovers both worker paths, forges both payloads, and exits zero.\n\n### P1 #2: Transitive product dependencies are absent from the signed input closure\nStatus: FIXING\nPlan: bind the statically resolvable transitive local import closure of product files and fail closed for unresolved dynamic loading, with stale-evidence coverage for an indirectly imported helper change.\n\n### P1 #3: Deleting the mutable working-tree policy disables protected mutation preflights\nStatus: FIXING\nPlan: resolve the Git root and inspect the committed protected policy independently of working-tree policy existence, with real CLI zero-write coverage for deleted and renamed policy files.\n\n### P1 #4: Post-install races can produce false COMMITTED or overwrite concurrent edits\nStatus: FIXING\nPlan: add a final desired-state CAS sweep before COMMITTED and conditional rollback CAS behavior that preserves third-party bytes and records a recoverable conflict.\n\n### P1 #5: The seven-night gate can be downgraded to one night\nStatus: FIXING\nPlan: enforce an independent protected minimum of seven in certificate construction and verification, including rejection of a validly re-signed one-night certificate.\n\n### P2 #1: Durable evidence identity depends on an ephemeral interpreter path\nStatus: FIXING\nPlan: replace absolute interpreter paths in runner identity payloads with stable measured runtime identity and checker artifacts, with cross-prefix identity coverage.\n\n### P2 #2: Canonical compatibility reporting breaks module scope and path-qualified identities\nStatus: FIXING\nPlan: separate scoped legacy projection counts/ok from full-inventory certificate semantics and derive prompt-root-relative qualified basenames, covering one-of-two scope and duplicate nested leaf names.\n\n### P2 #3: Protected signer subprocesses have no timeout\nStatus: FIXING\nPlan: use a finite protected timeout with process-group termination/reaping and stable errors for both signer adapters, tested against non-terminating helpers.\n\n### P2 #4: Repository identity initialization is not single-winner\nStatus: FIXING\nPlan: use a create-once final identity operation so concurrent losers read and return the persisted winner, with distinct-UUID concurrent initialization coverage.\n\nAll nine findings will follow strict red-first TDD with a pushed test commit and posted red evidence before implementation. |
Red TDD evidence for exhaustive xhigh remediationReview: #1985 (comment) Tests-only commit pushed before production changes. Focused local red run: The 11 failures cover transitive product closure, stable runner identity, both post-install transaction race windows, concurrent repository identity, both signer timeouts, seven-night downgrade/independent expectations, deleted committed policy, and scoped/path-qualified compatibility reporting. The skipped test is the adversarial worker-path discovery and dual-output forgery reproduction because protected validators intentionally fail closed without Linux namespaces on macOS. It will run in the focused Linux protected-runner lane. An isolated rerun confirmed the seven-night predicate reaches the intended assertion: |
Exhaustive code-quality reviewReviewed SHA: P1
P2
Prior review verification
Verification and scope
|
Implementer response to high reviewRe: #1985 (comment) P1 #1: Candidate can forge randomized checker result channelsStatus: FIXING P1 #2: Dynamic product dependencies remain outside the signed closureStatus: FIXING P1 #3: Linked-worktree policy deletion/rename disables protectionStatus: FIXING P1 #4: Recovery can false-commit after a raceStatus: FIXING P1 #5: Sync writes before canonical preflightStatus: FIXING P2 #1: Runner identity omits measured runtime/checker identityStatus: FIXING P2 #2: Path-qualified canonical selection is brokenStatus: FIXING P2 #3: Detached signer descendants can defeat timeoutStatus: FIXING I will follow strict red-first TDD: investigate sibling/history conventions, add tests only, run them red, commit and push the red commit, post exact red evidence, and only then edit production code. |
Red TDD evidence for high-review remediationReview: #1985 (comment) Local focused red run (excluding Linux-only sandbox case and the deliberately blocking signer probe): Failures reproduce:
The real detached-session signer test also fails its 1.5-second bound: the exact-head implementation waits for the detached pipe holder (about 30 seconds in this probe). The randomized worker-output forgery test is test-only updated to enumerate Proceeding to production implementation only after this pushed red commit. |
|
/heal |
Final evidence — high-review remediation completeReview addressed: #1985 (comment) Final pushed head: TDD commits
Red evidence: #1985 (comment) Outcomes
Verification
CI: https://github.com/promptdriven/pdd/actions/runs/29184616907 |
|
/heal |
Exhaustive read-only code-quality reviewReviewed SHA: P1
P2
Prior review verification
Verification and scope
|
Implementer response to high reviewP1 #1: Candidate code can enumerate and forge protected pytest result channelsStatus: FIXING P1 #2: Dynamic-product closure is bypassable through loader aliasesStatus: FIXING P2 #1: Runner identity omits checker and dependency bytesStatus: FIXING P2 #2: Signer timeout leaks untracked detached descendantsStatus: FIXING I will push the adversarial tests as a separate red commit and post their failing output before implementing the fixes. |
|
To use Codex here, create an environment for this repo. |
Red TDD evidence @
|
Final CI evidence: preserved XPASS diagnostics\n\nHead: \nFix commit: \n\n### Diagnosis and focused fix\n\nThe protected worker terminates through to prevent candidate finalizers from affecting the checker. On the failing Linux run, pytest's terminal XPASS diagnostic remained buffered when that immediate exit bypassed normal interpreter teardown. The checker therefore saw a valid JUnit report with one test and no failures, but no text, and normalized the result incorrectly to .\n\nThe fix flushes the restored worker and immediately after and before in both protected collection and execution workers. This preserves the existing isolated/randomized result channels and process-exit boundary; it only makes pytest's already-derived diagnostic observable to the checker.\n\nThe existing Linux regression was the red reproducer: it failed on with instead of .\n\n### Exact-head verification\n\n- Focused Ubuntu protected-runner lane: 132 passed, 1 warning, 73.44s. This includes . Run Unit Tests\n- Broad Ubuntu suite at the same SHA: 13,328 passed, 35 skipped, 1 xfailed, 795 warnings, 1452.27s. Unit Tests workflow\n- Required : passed. auto-heal workflow\n\nFinal repository state: passed, worktree is clean, and local HEAD exactly matches at the SHA above. |
2 participants
Summary
Adds the canonical
pdd.sync_corefoundation and routes the global-sync compatibility surface through its identity, manifest, classifier, reporting, transaction, and certificate APIs. The certificate binds the candidate wheel SHA and rejects unsafe protected mutations.pdd reconcileand existing global-sync commands retain their current compatibility behavior while using the canonical proof boundaries.Rollout status
The full global certificate remains red pending rollout: protected signer and release/workflow deployment, transactional staging, profile/evidence migration, and seven real nightly attestations are still unavailable. This PR intentionally does not claim global-certificate completion.
Verification
conda run -n pdd pytest -q tests/test_sync_core_*.py --timeout=120tests/test_commands_maintenance.pyconda run -n pdd pytest -q tests/e2e/test_issue_1932_continuous_sync_guarantee.py --timeout=120git diff --checkconda run -n pdd pylint pdd/sync_corereports onlyR0801 duplicate-code; score 10.00/10.