fix(sync): enforce protected approved alias policy

[gltanaka]

Summary

• Load approved aliases only from an immutable base/head-equal policy.
• Wire one policy through snapshots, finalization, transactions, and recovery.
• Persist and revalidate canonical WAL destinations before descriptor-pinned writes.

Test plan

• Focused sync-core pytest coverage, pylint, diff check, built-wheel smoke, and CLI pdd recover crash-recovery workflow.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[gltanaka]

Pre-fix investigation

• PathPolicy already accepts an explicit alias-to-canonical mapping and rejects target retargeting, but build_unit_snapshot constructed it with no map. finalize_unit separately constructed TransactionManager with no map. CLI recover, baseline, and reporting recovery inspection also constructed managers without alias authority.
• Sibling mechanisms: verification profiles and sync waivers load protected base/head inputs and reject candidate weakening; language aliases are an unrelated registry mechanism. Direct PathPolicy tests already established the intended symlink behavior.
• Prior commits: 4351d7320 introduced the canonical sync foundation; be77d756f hardened transaction writes with descriptor-pinned parents. This PR follows that shape rather than bypassing descriptor validation.
• Policy decision: .pdd/sync-aliases.json has strict schema version 1 and alias/canonical repository-relative rows. It is authoritative only when the immutable base blob exists and the checked candidate blob is byte-identical. Candidate additions, removals, or edits fail closed, so candidate-controlled policy cannot authorize itself.
• COMMITTING recovery: WAL entries persist canonical repository-relative destination identity plus the prepared alias map. Recovery reconstructs that prepared map only to revalidate the logical alias against the persisted canonical destination before every descriptor-pinned install/rollback; target changes fail closed.

[gltanaka]

Strict TDD evidence

Red commit pushed first: d22a30edd test(sync): reproduce protected alias policy gaps (rebased as 5c1875740).

Red command:

conda run -n pdd pytest -q tests/test_sync_core_alias_policy.py ...

Red result:

ModuleNotFoundError: No module named pdd.sync_core.alias_policy

Green implementation: fd7027df4 fix(sync): wire protected alias policy end to end (rebased as df8e05781).

Post-rebase verification: focused alias/path/snapshot/transaction/reporting pytest suite, pylint for touched production modules (10.00/10), and git diff --check all passed.

[gltanaka]

E2E and parity evidence

• Built wheel smoke: pytest -q tests/test_sync_core_lifecycle_scenarios.py::test_built_wheel_runs_in_clean_virtual_environment passed.
• Source CLI workflow: created an approved alias -> canonical directory symlink, prepared a transaction, simulated process death after the durable COMMITTING marker, then ran:

PYTHONPATH=<worktree> pdd recover --transaction e2e-alias-recovery

It returned phase: COMMITTED, listed alias/widget.py, and the canonical target contained the intended recovered content.

• Rebased onto foundation 8a8cf5137 before the final verification and pushed with --force-with-lease.

[gltanaka]

Rebased locally onto origin/codex/issue-1932-verifiable-goal at f78bad5. Local rebased head: e742141. Validation: git diff --check passed; focused suite had 42 passed, 1 failed in tests/test_sync_core_transaction.py::test_descriptor_time_parent_symlink_swap_cannot_redirect_commit. The behavior still raises TransactionConflict, but the foundation changes its message to 'destination alias policy changed: src/widget.py' rather than the test's expected 'parent changed or is unsafe'. Per no-feature-edits, this branch was not pushed; worktree is clean.

[gltanaka]

Post-rebase blocker acknowledged: I am updating only the affected descriptor-time parent-symlink-swap assertion to accept the legitimate protected-path conflict outcome, then will run focused alias/transaction tests and push the rebased stack with force-with-lease.

[gltanaka]

Resolved in 44a3434 (test(sync): accept protected alias conflict) and pushed the rebased stack with --force-with-lease. The descriptor-time parent-symlink-swap test now requires TransactionConflict and permits only the two exact protected-path outcomes for src/widget.py: descriptor parent-safety rejection or alias-policy rejection. Evidence: conda run -n pdd pytest -q tests/test_sync_core_transaction.py tests/test_sync_core_alias_policy.py tests/test_sync_core_identity_path_policy.py -> 30 passed (one existing Pandas deprecation warning); git diff --check passes; worktree is clean and matches origin.

[gltanaka]

Codex xhigh adversarial review — PR #1991 @ 226c480

Verdict: DON'T MERGE
Findings: 3 total (1 P1, 2 P2)

P1 (blocking)

• [P1] Approved alias policy is consumed but not accounted as protected inventory — pdd/sync_core/manifest.py:705-720
  This PR introduces .pdd/sync-aliases.json as the authority for approved symlink aliases, and snapshot/finalize now depend on it. The manifest's protected-control allowlist still omits that file, so any repository that actually has a base-approved alias policy records the policy path as unowned/invalid unless a separate ownership rule happens to cover it. That keeps invalid > 0 and prevents the global-sync predicate from ever reaching green for the very repositories that need approved aliases. The new tests exercise snapshot/finalize behavior but do not assert that build_canonical_report remains ok with a legitimate protected alias policy.
  Suggested fix: Make .pdd/sync-aliases.json a first-class protected control input in manifest/reporting, validate base/head immutability there, and add a report-level test that a protected unchanged alias policy contributes zero invalid/unaccounted paths while candidate-added/changed/removed policy remains blocking.

P2 (non-blocking)

• [P2] Recovery can adopt alias authority from the local WAL without reloading protected policy — pdd/sync_core/transaction.py:641-704
  recover() constructs a default TransactionManager(root) with no manifest context, then _adopt_journal_aliases() accepts the aliases serialized in journal.json whenever the current manager has no aliases. That means the recovery write path is authorized by mutable transaction state rather than by the immutable base/head-equal policy this PR is trying to enforce. The canonical destination is revalidated, but the authority that makes a symlink approved is no longer protected policy at recovery time.
  Suggested fix: Persist the protected base/head refs or an alias-policy digest in the WAL and have recovery reload/compare the protected alias policy before adopting journal aliases; fail closed when it cannot prove the journal aliases match protected policy.
• [P2] Transaction plans do not reject duplicate canonical destinations — pdd/sync_core/transaction.py:357-369,381-400
  Duplicate detection only compares logical write.relpath values before aliases are resolved. A plan can include both alias/widget.py and canonical/widget.py, which resolve to the same destination after alias canonicalization. With different contents the commit partially installs one entry and then rolls back on the second; with identical contents it can report two logical changed paths for one file. This is avoidable ambiguity in the transactional boundary.
  Suggested fix: Canonicalize every planned write during _validate_plan()/prepare() and reject duplicate canonical_relpath values before writing WAL blobs.

Notes

• Reviewed against actual base codex/issue-1932-verifiable-goal @ 63d31004f3e9633d11683cf13e2573f9fb8a2d39, not main.

[gltanaka]

Implementer response to Codex review

@codex Re: #1991 (comment)

P1 #1: Approved alias policy is consumed but not accounted as protected inventory

Status: FIXING
Plan: Make .pdd/sync-aliases.json an explicit protected control input in manifest/reporting and add report-level tests for unchanged, changed, added, and removed policy files.

P2 #1: Recovery can adopt alias authority from the local WAL without reloading protected policy

Status: FIXING
Plan: Treat this as a real recovery authority gap; persist policy identity/digest information in the WAL and require recovery to prove the journal aliases match protected policy before adoption.

P2 #2: Transaction plans do not reject duplicate canonical destinations

Status: FIXING
Plan: Treat this as a real transactional ambiguity; canonicalize planned writes before WAL creation and reject duplicate canonical destinations with a focused regression.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[gltanaka]

Follow-up fix evidence

Fixed Codex xhigh findings from #1991 (comment).

Commits:

• 3fc872cc0 test(sync): reproduce protected alias review gaps — pushed before implementation. Red run: conda run -n pdd pytest -q tests/test_sync_core_reporting.py::test_trusted_finalizer_commits_artifact_through_protected_alias tests/test_sync_core_transaction.py::test_plan_rejects_duplicate_canonical_destinations_through_alias tests/test_sync_core_transaction.py::test_recovery_requires_protected_alias_authority_not_only_journal failed with all 3 expected failures: protected alias policy inventory invalid, duplicate canonical destination accepted, and default recovery adopted WAL aliases.
• 251967179 fix(sync): enforce protected alias recovery policy — accounts .pdd/sync-aliases.json plus protected alias/canonical target paths in manifest inventory, rejects duplicate canonical transaction destinations before WAL creation, and requires recovery-time protected alias authority/digest to match the journal.

Verification:

• Exact red tests now pass: 3 passed.
• conda run -n pdd pytest -q tests/test_sync_core_transaction.py tests/test_sync_core_alias_policy.py tests/test_sync_core_identity_path_policy.py tests/test_sync_core_reporting.py tests/test_sync_core_manifest.py -> 70 passed.
• conda run -n pdd pylint pdd/sync_core/manifest.py pdd/sync_core/transaction.py reports only the existing manifest too-many-lines module-size baseline; no functional production diagnostics remain.
• git diff --check passed before commit.

[gltanaka]

Codex xhigh adversarial re-review — PR #1991 @ 688ea85

Verdict: DON'T MERGE
Findings: 2 total (2 P1, 0 P2)

P1 (blocking)

• [P1] Approved alias target blanket-accounting hides unowned candidate files — pdd/sync_core/manifest.py:663-721
_candidate_records() now marks any path under any approved alias canonical target as MANAGED with provenance protected-alias-policy. That makes a protected alias like src -> canonical authorize the entire canonical/** subtree, not just the symlink and the canonical destinations that are actually prompt/profile/architecture-owned. A focused probe against this head showed a candidate-added canonical/rogue.py classified as MANAGED / approved-alias-target with zero invalid or unaccounted paths once ordinary base ownership was otherwise satisfied. This defeats the protected inventory predicate and lets unowned candidate files hide under an approved target directory.
  Suggested fix: Remove the blanket _under_approved_alias_target() accounting. Account the approved symlink policy/control path itself, and only account canonical counterparts of concrete managed logical paths after resolving the protected alias; otherwise require normal prompt/output/profile ownership or protected ownership rules. Add a report/manifest regression where a candidate adds canonical/rogue.py under an approved alias target and it remains invalid/unaccounted.

• [P1] Public recovery cannot recover legitimate approved-alias COMMITTING transactions — pdd/commands/sync_core.py:258-264, pdd/sync_core/transaction.py:664-729
  The prior WAL-authority issue was changed to fail closed, but the public pdd recover command still constructs TransactionManager(Path.cwd()) with no protected alias map. A transaction prepared through a legitimate approved alias records non-empty approved_aliases; during recovery _adopt_journal_aliases() compares those rows to self.policy.approved_aliases, which is empty in the CLI path, and raises TransactionConflict. A focused probe that crashed after the COMMITTING marker through src -> canonical reproduced pdd recover --transaction ... exiting with TransactionConflict and leaving the destination unrecovered.
  Suggested fix: Make the recover command reload the protected alias policy before constructing TransactionManager, using protected base/head or persisted protected policy identity/digest from the WAL, and add an end-to-end CLI recovery test for a COMMITTING approved-alias transaction.

P2 (non-blocking)

• None.

Prior findings verified

• .pdd/sync-aliases.json omitted from protected inventory: resolved for the policy file itself; it is now a protected control input.
• WAL-only alias adoption during recovery: the unsafe adoption is gone, but the replacement breaks the public recovery path; finding above.
• Duplicate canonical destinations: resolved; _validate_plan() now rejects duplicate canonical relpaths before WAL creation.

Notes

• Reviewed live open PR state against actual stacked base codex/issue-1932-verifiable-goal @ 9bd278dec20482e4d8be66b2202a6a3a025f2e03 and head 688ea85d67b30e428ecac3677cf86d0e51645375.

[gltanaka]

Re-review follow-up evidence

Fixed latest xhigh re-review findings from #1991 (comment).

Commits:

• 986772152 test(sync): reproduce alias rereview gaps — red tests proved canonical/rogue.py under an approved alias target was blanket-accounted as valid, and public pdd recover could not recover a legitimate COMMITTING approved-alias transaction.
• 0fceb4804 fix(sync): constrain alias inventory and recovery — removes blanket canonical-target ownership, accounts only exact canonical counterparts of concrete managed logical paths, and makes public recovery load the committed protected alias policy from HEAD before comparing to WAL alias digest.

Verification:

• Red tests before fix failed with invalid == 0 for rogue alias-target file and public recovery exiting with transaction alias policy changed before recovery.
• conda run -n pdd pytest -q tests/test_sync_core_reporting.py::test_approved_alias_target_does_not_blanket_account_candidate_files tests/test_sync_core_transaction.py::test_public_recover_loads_committed_protected_alias_policy tests/test_sync_core_transaction.py tests/test_sync_core_alias_policy.py tests/test_sync_core_identity_path_policy.py tests/test_sync_core_reporting.py tests/test_sync_core_manifest.py -> 72 passed.
• conda run -n pdd pylint pdd/sync_core/alias_policy.py pdd/sync_core/manifest.py pdd/commands/sync_core.py reports only the existing pdd/sync_core/manifest.py module-size baseline (too-many-lines).
• git diff --check passed.

Branch pushed; no merge performed.

[gltanaka]

Codex xhigh adversarial re-review round 3 — PR #1991 @ 66b5464

Verdict: DON'T MERGE
Findings: 2 total (1 P1, 1 P2)

P1 (blocking)

• [P1] Overlapping approved aliases can still hide unowned candidate files — pdd/sync_core/alias_policy.py:39-47, pdd/sync_core/manifest.py:646-678, pdd/sync_core/manifest.py:721-735
  The blanket target accounting from round 2 is gone for simple policies, but the protected alias parser still accepts overlapping alias paths. _managed_alias_counterparts() then maps every matching alias prefix for one logical managed path, even when PathPolicy would resolve only the outer symlink at runtime. A focused manifest probe with protected policy src -> canonical and src/nested -> other, architecture output src/nested/widget.py, and candidate-added other/widget.py classified other/widget.py as MANAGED with provenance architecture:protected-alias-policy, with no invalid or unaccounted paths. That silently accounts a file that is not the actual canonical target of the managed logical path.
  Suggested fix: Reject overlapping alias paths/canonical targets in protected alias policy, or compute alias counterparts through the same PathPolicy.resolve() result used for writes so each managed logical path authorizes exactly one canonical destination. Add a regression for src -> canonical plus src/nested -> other where other/widget.py remains invalid/unaccounted.

P2 (non-blocking)

• [P2] Alias and canonical transaction commits do not share a canonical destination lock — pdd/sync_core/transaction.py:483-494, pdd/sync_core/transaction.py:421-423, pdd/sync_core/transaction.py:616-638
  The same-plan duplicate canonical destination fix is present, but separate prepared transactions still lock only relpath strings. A transaction for alias/widget.py and another for canonical/widget.py resolve to the same canonical_relpath but take different lock files, so they can both validate the old precondition before either install occurs. A focused concurrency probe against this head forced both commits to the install point; both returned COMMITTED, and the final file was the last writer's content with no conflict.
  Suggested fix: Include each entry's persisted canonical_relpath in _locks() in addition to the logical relpath, and add a concurrent transaction regression proving alias/canonical writers serialize so the second observes a precondition conflict.

Prior Findings Verified

• .pdd/sync-aliases.json omitted from protected inventory: resolved for the policy file itself; it is now treated as protected control input.
• WAL-only alias adoption during recovery: unsafe adoption is gone; recovery now requires current approved aliases to match the journal digest.
• Duplicate canonical destinations within one transaction plan: resolved; _validate_plan() rejects duplicate canonical relpaths before WAL creation.
• Blanket approved-alias target accounting: resolved for ordinary alias targets; candidate-added canonical/rogue.py under a simple src -> canonical policy is no longer blanket-accounted.
• Public approved-alias recovery path: resolved for the tested committed-policy path; pdd recover now loads committed aliases before recovery.

Notes

• Reviewed live open PR state against actual stacked base codex/issue-1932-verifiable-goal @ b8f05dcd93d3c18067facaedde7bbc81216c4c6e and head 66b5464f89ea28c1a00906f1f78ca617f9df7061.
• Focused probes used disposable temporary repositories/worktrees only.

[gltanaka]

Implementer response to Codex re-review round 3

@codex Re: #1991 (comment)

P1 #1: Overlapping approved aliases can still hide unowned candidate files

Status: FIXING
Plan: Reject overlapping alias authorities in the protected alias policy so a logical managed path cannot authorize an alternate canonical destination that runtime path resolution would not use. I will add manifest/policy regressions for src -> canonical plus src/nested -> other.

P2 #1: Alias and canonical transaction commits do not share a canonical destination lock

Status: FIXING
Plan: Include persisted canonical destinations in transaction locking so alias and canonical writers serialize on the same destination, with a concurrent transaction regression proving the second writer observes a precondition conflict.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[gltanaka]

Round-3 follow-up evidence

Fixed latest xhigh re-review findings from #1991 (comment).

Commits:

• 0cc113667 test(sync): reproduce alias round-three gaps — red tests showed overlapping protected aliases were accepted and alias/canonical transactions used disjoint locks.
• b846c742b fix(sync): reject overlapping alias authorities — rejects overlapping alias paths and overlapping canonical targets in protected alias policy, and includes persisted canonical_relpath resources in transaction locks so alias/canonical writers share a destination lock.

Verification:

• Red tests now pass: overlapping alias policy and canonical lock-sharing regressions.
• conda run -n pdd pytest -q tests/test_sync_core_alias_policy.py tests/test_sync_core_transaction.py tests/test_sync_core_reporting.py tests/test_sync_core_manifest.py tests/test_sync_core_identity_path_policy.py -> 74 passed.
• conda run -n pdd pylint pdd/sync_core/alias_policy.py pdd/sync_core/transaction.py -> 10.00/10.
• Smoke/E2E: public approved-alias recovery, canonical lock-sharing, and overlap rejection -> 3 passed.
• git diff --check passed.

Branch pushed; no merge performed.

[gltanaka]

Codex xhigh adversarial re-review round 4 — PR #1991 @ 2b9f0e7

Verdict: DON'T MERGE
Findings: 1 total (1 P1, 0 P2)

P1 (blocking)

• [P1] Manifest/reporting still accepts overlapping approved aliases and hides unowned candidate files — pdd/sync_core/manifest.py:819-855, pdd/sync_core/manifest.py:721-735, pdd/sync_core/alias_policy.py:49-54
  The round-3 overlap rejection was added to alias_policy._parse(), so snapshot/finalize paths reject overlapping alias authorities, but build_unit_manifest() uses a separate _approved_aliases() parser in manifest.py that does not perform the new alias-path or canonical-target overlap checks. The reporting inventory path therefore still accepts a protected policy such as src -> canonical plus src/nested -> other, and _managed_alias_counterparts() maps a single logical managed path through both aliases. A focused disposable manifest probe against this head added candidate other/widget.py; it was classified as MANAGED with provenance architecture:protected-alias-policy, with no invalid reasons and no unaccounted paths. That is the same protected-inventory bypass from round 3, still live in the report/certificate inventory path.
  Suggested fix: Centralize protected alias parsing so manifest/reporting and snapshot/finalize use the same overlap validation, or add identical overlap checks to manifest._approved_aliases(), then add a report/manifest regression for src -> canonical plus src/nested -> other where other/widget.py remains invalid/unaccounted or the policy itself is invalid.

P2 (non-blocking)

• None.

Prior Findings Verified

• .pdd/sync-aliases.json omitted from protected inventory: resolved for the policy file itself; it is treated as protected control input.
• WAL-only alias adoption during recovery: unsafe adoption is gone; recovery compares the WAL alias digest against committed protected aliases.
• Duplicate canonical destinations within one transaction plan: resolved; _validate_plan() rejects duplicate canonical relpaths.
• Simple approved-alias target blanket accounting: resolved for non-overlapping policy; canonical/rogue.py under simple src -> canonical remains invalid in the focused test.
• Public approved-alias recovery path: resolved for the committed-policy path covered by test_public_recover_loads_committed_protected_alias_policy.
• Alias/canonical transaction lock sharing: resolved for the current regression; locks include persisted canonical destinations.
• Overlapping approved aliases: not resolved in manifest/reporting; finding above.

Read-only Probes

• conda run -n pdd pytest -q tests/test_sync_core_alias_policy.py tests/test_sync_core_transaction.py::test_plan_rejects_duplicate_canonical_destinations_through_alias tests/test_sync_core_transaction.py::test_public_recover_loads_committed_protected_alias_policy tests/test_sync_core_transaction.py::test_alias_and_canonical_transactions_share_destination_lock tests/test_sync_core_reporting.py::test_approved_alias_target_does_not_blanket_account_candidate_files -> 7 passed.
• Disposable manifest probe: overlapping policy src -> canonical, src/nested -> other plus candidate other/widget.py produced invalid_reasons == (), unaccounted == [], and other/widget.py inventory MANAGED.
• Control probe: load_protected_aliases() rejected the same overlapping policy, confirming the split parser is the gap.

Notes

• Reviewed live open PR state against actual base codex/issue-1932-verifiable-goal @ 87aea93087dc246c9224c2df4fd3b6fd8b102dcf and head 2b9f0e7e99717e38630de20115c162e169732e2b.
• Local inspection/probes used a disposable /tmp clone only; no repository files or PR branches were modified.

[gltanaka]

Implementer response to Codex re-review round 4

@codex Re: #1991 (comment)

P1 #1: Manifest/reporting still accepts overlapping approved aliases and hides unowned candidate files

Status: FIXING
Plan: Centralize protected alias policy parsing so manifest/reporting uses the same overlap rejection as snapshot/finalize. I will add a manifest/report regression proving an overlapping policy cannot account other/widget.py or any rogue canonical path through a second alias parser.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[gltanaka]

Round-4 follow-up evidence

Fixed latest xhigh re-review finding from #1991 (comment).

Commit:

• dfa00cb52 fix(sync): centralize protected alias parsing — exposes the strict protected alias parser from alias_policy.py, removes the duplicate manifest parser, and makes manifest/reporting apply the same alias-path and canonical-target overlap rejection as snapshot/finalize.

TDD evidence:

• Red regression before fix: tests/test_sync_core_reporting.py::test_overlapping_alias_policy_cannot_account_rogue_counterparts failed with invalid_reasons == () and no unaccounted paths for overlapping aliases src -> canonical plus src/nested -> other and candidate other/widget.py.
• Green after fix: the same regression passes and reports overlap invalidity instead of accounting the rogue counterpart.

Verification:

• conda run -n pdd pytest -q tests/test_sync_core_alias_policy.py tests/test_sync_core_transaction.py tests/test_sync_core_reporting.py tests/test_sync_core_manifest.py tests/test_sync_core_identity_path_policy.py -> 75 passed.
• conda run -n pdd pylint pdd/sync_core/alias_policy.py pdd/sync_core/manifest.py -> only existing manifest.py module-size baseline (too-many-lines); no new functional diagnostics.
• git diff --check passed.
• Smoke/E2E coverage remains in-suite for public approved-alias recovery and alias/canonical destination locking.

Branch pushed; no merge performed.

[gltanaka]

Codex xhigh adversarial re-review round 5 — PR #1991 @ e7c718c

Verdict: MERGE
Findings: 0 total (0 P1, 0 P2)

P1 (blocking)

• None.

P2 (non-blocking)

• None.

Prior Findings Verified

• .pdd/sync-aliases.json omitted from protected inventory: resolved. The policy file is treated as protected control input.
• WAL-only alias adoption during recovery: resolved. Recovery compares the journal alias digest/rows against the current committed protected alias policy; default recovery without protected authority fails closed.
• Duplicate canonical destinations in one plan: resolved. prepare() rejects duplicate canonical relpaths before WAL creation.
• Simple approved-alias target blanket accounting: resolved. A candidate-added canonical/rogue.py under src -> canonical remains invalid/unaccounted.
• Public approved-alias recovery path: resolved. pdd recover loads committed aliases before recovering a COMMITTING transaction.
• Alias/canonical concurrent transaction lock sharing: resolved. Locks include persisted canonical_relpath resources, so alias and canonical writers share the destination lock.
• Overlapping approved aliases in manifest/reporting: resolved. manifest.py now uses the centralized strict protected-alias parser; overlapping alias paths/targets become invalid policy instead of alternate managed counterparts.

Read-only Probes

• Live base/head: codex/issue-1932-verifiable-goal @ f1dca1995daf794f5d9889c9d640fc0c392ff698; head codex/issue-1932-alias-policy @ e7c718cf0e79b1d5c40ffcb415b9d2ba65891568.
• conda run -n pdd pytest -q tests/test_sync_core_alias_policy.py tests/test_sync_core_transaction.py tests/test_sync_core_reporting.py tests/test_sync_core_manifest.py tests/test_sync_core_snapshot.py tests/test_sync_core_identity_path_policy.py -> 79 passed.
• Disposable manifest probe for simple src -> canonical plus candidate canonical/rogue.py -> invalid/unaccounted as expected.
• Disposable manifest probe for overlapping src -> canonical, src/nested -> other plus candidate other/widget.py -> invalid policy/unaccounted as expected.
• git diff --check f1dca1995daf794f5d9889c9d640fc0c392ff698 e7c718cf0e79b1d5c40ffcb415b9d2ba65891568 -> clean.
• Current GitHub checks: auto-heal pass; heal pass. Merge state: CLEAN / MERGEABLE.

Notes

• Reviewed the full live diff and surrounding code in a disposable /tmp checkout. No repository files or PR branches were modified.

[gltanaka]

Codex xhigh adversarial re-review round 6 — PR #1991 @ 275b67d

Verdict: DON'T MERGE
Findings: 1 total (1 P1, 0 P2)

P1 (blocking)

• [P1] Protected alias policy can account alternate files when the alias path is not actually a symlink — pdd/sync_core/alias_policy.py:30-58, pdd/sync_core/manifest.py:647-655, pdd/sync_core/manifest.py:722-735, pdd/sync_core/path_policy.py:60-78
  The policy parser validates schema and overlap, but it never proves that each protected alias_path is an unchanged symlink in the protected trees. Reporting then maps every managed logical path below the alias to a canonical counterpart, while runtime PathPolicy.resolve() only applies approved aliases when the on-disk component is a symlink. If protected base has stale/mistyped policy src -> canonical while src is a real directory, runtime writes and snapshots still resolve src/widget.py, but manifest/reporting accounts candidate-added canonical/widget.py as MANAGED with no invalid/unaccounted path. A disposable probe reproduced counts.invalid == 0, counts.unaccounted_tracked_paths == 0, and errors == [] for a candidate-added alternate canonical/widget.py under this stale protected policy.
  Suggested fix: Validate protected aliases against the base/head tree before accepting them for manifest/reporting/finalize: each alias_path should be an unchanged symlink whose resolved target is exactly canonical_path, and counterpart accounting should be derived only from the same runtime resolution semantics used by PathPolicy. Add a report regression where src is a real directory but the policy says src -> canonical, and canonical/widget.py remains invalid/unaccounted or the policy itself is invalid.

P2 (non-blocking)

• None.

Prior Findings Verified

• .pdd/sync-aliases.json omitted from protected inventory: resolved for the policy file itself.
• WAL-only alias adoption during recovery: resolved. Recovery compares the journal alias rows/digest against current committed protected aliases.
• Duplicate canonical destinations in one plan: resolved. prepare() rejects duplicate canonical relpaths.
• Simple approved-alias target blanket accounting: resolved for an actual protected symlink alias; canonical/rogue.py under src -> canonical remains invalid/unaccounted in the existing regression.
• Public approved-alias recovery path: resolved. pdd recover loads committed aliases before recovering a COMMITTING transaction.
• Alias/canonical concurrent transaction lock sharing: resolved. Locks include persisted canonical_relpath resources.
• Overlapping approved aliases in manifest/reporting: resolved. Manifest uses the centralized strict parser, so overlapping alias paths/targets become invalid policy.
• New stale/non-symlink alias authority gap: not resolved; finding above.

Read-only Probes

• Live base/head: codex/issue-1932-verifiable-goal @ d25f0fbc927ae1c102e54093b6bd33d315a5a37b; head codex/issue-1932-alias-policy @ 275b67db4908acbb8458c6217f7dfad0ab797206.
• conda run -n pdd pytest -q tests/test_sync_core_alias_policy.py tests/test_sync_core_transaction.py tests/test_sync_core_reporting.py tests/test_sync_core_manifest.py tests/test_sync_core_snapshot.py tests/test_sync_core_identity_path_policy.py -> 79 passed.
• Disposable stale-alias probe: protected policy src -> canonical with src as a real directory plus candidate-added canonical/widget.py produced no invalid/unaccounted path and no report errors.
• git diff --check d25f0fbc927ae1c102e54093b6bd33d315a5a37b 275b67db4908acbb8458c6217f7dfad0ab797206 -> clean.
• Current GitHub checks: auto-heal pass; heal pass. Merge state: CLEAN / MERGEABLE.

Notes

• Reviewed the full live diff and surrounding code in a disposable /tmp checkout. No repository files or PR branches were modified.

[gltanaka]

Sol-high combined-base verification — PR #1991 @ 0fadd2c

Verdict: REQUEST_CHANGES

[P1] Rebase left three Alias fail-closed regressions asserting the obsolete manifest error

File: tests/test_sync_core_reporting.py:1137, tests/test_sync_core_reporting.py:1181

The combined foundation's finalize_unit() correctly rejects these alias collision/excluded-ownership manifests before validation or transaction writes, but it now emits canonical finalization requires a valid protected candidate manifest. The rebased Alias tests still require manifest is invalid, so all three cases fail at the context-manager match and never execute their run_profile.assert_not_called() / prepare.assert_not_called() proof.

Exact focused rerun:

• test_finalizer_rejects_invalid_alias_manifest_before_validation_or_writes[False]: failed
• ...[True]: failed
• test_excluded_project_alias_counterpart_is_invalid_before_finalization: failed

Smallest correction: Update these three expectations to the foundation's exact fail-closed message (or assert only ValueError while retaining the zero-call assertions), then rerun the focused combined suite.

Verified integration scope

• Exact base, merge-base, and live PR base: f29fec89596b980fddca4503a21ba1c39fcaf78c.
• Exact local/remote/live PR head: 0fadd2c58fe541707f3c9a9e07b8b71a71436d40.
• Range-diff retains all 29 previously approved commits in order; two additional integration commits only inventory the new Alias files and remove a now-unused import.
• Alias canonicalization, overlap/cycle/collision rejection, immutable candidate policy, HEAD-bound COMMITTING recovery, snapshot/report/transaction identity, legacy routing, denominator fixed point, verification-profile rotation, protected ownership, Jest, and supervisor/liveness behavior remain structurally intact.
• Focused combined run: 216 passed, 15 skipped, 7 failed. Three deterministic failures are the finding above; four other finalizer failures are the existing macOS protected-runner collection limitation, not a combined-base behavior regression.
• Exact-head CI at review time: CodeQL, package smoke, public CLI, repo-bloat, story, and language analyses passed; unit tests, heal, and auto-heal were still in progress. GitHub reported mergeable but blocked.

[gltanaka]

Exact-head correction verification — PR #1991 @ 1717b36

VERIFIED — MERGE

Verified only the prior Alias finding and affected behavior:

• Diff from approved head 0fadd2c5 is test-only: tests/test_sync_core_reporting.py; no production change.
• Both alias-collision parameter cases and the excluded-ownership case now assert the exact current fail-closed message: canonical finalization requires a valid protected candidate manifest.
• The corrected contexts complete, so the tests execute and prove run_profile.assert_not_called() and TransactionManager.prepare.assert_not_called(); collision cases also prove no metadata/evidence state was created.
• Alias-focused reporting slice: 13 passed, 2 expected Darwin skips.

Exact-head CI inspection: no failures at review time; Repo Bloat and JS/Actions analysis passed, CodeQL was neutral, while unit tests, smoke, CLI, story, Python analysis, heal, and auto-heal were still in progress. Local/remote/live PR heads match the requested SHA and the worktree is clean.