Skip to content

Report effective review authority and superseded Devin bridge drift - #988

Merged
jeffhuber merged 6 commits into
mainfrom
claude/955-adoption-polish
Sep 14, 2026
Merged

jeffhuber merged 6 commits into
mainfrom
claude/955-adoption-polish

Conversation

@jeffhuber

@jeffhuber jeffhuber commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Closes #955

Adoption reports now describe the effective review authority and Devin transport for the selected configuration, with portable setup commands and a concise doctor view.

  • Audit and session wording reuse the maintained role-eligibility decision. Audit wrappers resolve policy from the fetched base revision and carry that SHA through context and review. Informational or unavailable policy cannot be promoted by an operator override; explicit configuration selection and historical callers remain supported.
  • doctor --packaged-starter and init --packaged-starter select the maintained package resource consistently, preserve the profile, and reject contradictory positional config selection. Transport setup previews and stages from that resource, then verifies the installed repository configuration. Explicit paths and custom lane guidance remain pinned to their selected configuration.
  • doctor --concise emphasizes failures and owner actions while JSON, advanced and campaign views retain full detail. Unselected integrations remain quiet.
  • Setup drift identifies superseded Devin bridge files and provides a bounded migration preview. The optional prompt-pack setup guidance follows staging and normal PR review, preserving role and lease instructions.

Validation on d52782405b18d7e280a1330748daccccb0e9a74b:

  • Full canonical local suite: 3,658 tests passed in 600.978 seconds, with 16 skips.
  • All 75 adoption regressions pass, including stale/missing base refs, refs moving after fetch, config/profile selection, installed transport verification, and privacy. Ruff and the privacy scan pass.
  • CI passes on Python 3.12, 3.13 and 3.14, with both containment jobs green.
  • Independent exact-head Codex audit: PASS, no findings. The authoritative Code Mower gate passes.

Built by Claude through the maintained Code Mower local lane runner and independently reviewed through the Code Mower Codex audit wrapper. Raw execution evidence remains private. No hosted Devin session was created for this work.

Rendered status now describes the posture this run is actually configured
for, and setup drift names the superseded Devin transport instead of
reporting it as ordinary drift.

- Add review_authority, which reads the selected review lane declaration and
  the maintained role decision from role_eligibility and renders the effective
  posture. No new policy resolver: installation, lane labels, and provider
  identity confer nothing, and a lane that declares merge authority renders as
  informational when the role decision narrows it.
- Resolve the Claude and Codex audit comment posture from the repository
  configuration this run uses instead of defaulting to merge authority. An
  explicit --merge-authority/--informational flag or env override still wins,
  an explicitly selected --code-mower-config is never replaced by the starter,
  and a checkout that configures no lanes keeps the maintained lane default.
- Single-source the session reviewer wording through the same label helper.
- Detect the legacy Devin issue-comment bridge and labeler during setup drift
  and report a bounded migration to the maintained devin_api_v3 Sessions API
  transport, naming only the observed files. Nothing is deleted or rewritten.
- Add a concise posture-scoped doctor view behind --concise that leads with
  active failures and owner actions and counts the remaining warnings by group.
  JSON and the explicit --advanced and --campaign modes keep full detail.
- Add opt-in Devin setup guidance to the orchestrator prompt pack using the
  supported transport-selection and readiness commands, with generated setup
  staged for review rather than applied.

Recorded audit headers stay readable evidence without becoming a claim about
the current posture. The Claude + Codex default and the quiet default for
unselected optional providers are unchanged.

Closes #955

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeffhuber

Copy link
Copy Markdown
Contributor Author

Acceptance review for e4c88c8

Two P2 gaps remain against #955's bounded work order. These are broker acceptance checks, separate from the in-flight independent Code Mower Codex audit. Keep this PR draft; the same Claude builder will receive a consolidated fix round after the current review completes.

  1. [P2] Positive audit overrides bypass the selected repository policy. src/code_mower/review_authority.py:150 returns merge_authority=override before loading the requested configuration or calling the maintained role decision. With role_policy.codex.reviewer.enabled: false, the normal result is informational with policy_denied; passing override=True returns authoritative. The same path skips an explicitly missing configuration. Compute the effective configured decision first: an informational override may narrow it, while a positive flag/environment value cannot widen an informational, policy-denied, unavailable or unqualified role. Preserve explicitly selected configuration and cover Claude/Codex positive flag/env cases, informational lanes, denied role policy and missing explicit configuration.

  2. [P2] Required portable default Devin remediation is still missing. The installation-specific path is generated at runtime, so searching the source for a literal installed path does not verify this criterion. devin_readiness.select_transport_command still passes a resolved packaged-starter path through _pinned, and the resulting init/readiness commands contain that installation path. Render an equivalent portable supported selection for the default packaged starter, carrying the actual configuration source and profile. Preserve explicit repository configurations, custom lane names and paths containing spaces; do not shorten a command by silently selecting another config. Add command-equivalence and text/JSON regressions for packaged defaults and explicit/custom selections. A narrowly scoped explicit starter selector is acceptable if the existing CLI cannot express this portably; document and test the supported command rather than inventing a prose-only command.

The broker's canonical tests use an isolated temporary directory outside Git checkouts, preserving the context-store contract. The earlier two builder-environment failures do not establish a source regression. No paid provider work is part of these checks.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Codex audit (merge-authority lane)

Head SHA: e4c88c840c9ce532615aae257b51bd9ca5eb8555
Findings: P0=0, P1=0, P2=1, P3=0 (blocker policy: any P0/P1/P2 → BLOCKED)

Codex Audit: BLOCKED

Summary:

Default authority discovery reads proposed PR configuration instead of active repository policy, so audit comments can report an authority change before it is approved.

Findings:

  • [P2] Resolve default review authority from the trusted base -- src/code_mower/review_authority.py:130
    Finding ID: codex:91b1babfe144c3614baf
    When a PR changes code-mower.yml, this reads the proposed policy rather than the repository's active policy: both wrappers pass the PR-head checkout as repo_root, and the generated self-hosted workflow explicitly checks out the PR head there. Consequently, an unmerged promotion or demotion immediately changes the audit's authority header, misrepresenting its current posture. Resolve implicit configuration from the trusted base ref, as context_audit.required_for_repo already does, while retaining explicit operator configuration overrides.

Source locations normalized to repository-relative paths for public privacy; the original reviewer output is retained privately.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Code Mower Claude fix round for #955 / #988

The original Claude writer exited cleanly at e4c88c840c9ce532615aae257b51bd9ca5eb8555. Resume this same Claude-owned branch with one writer and keep the PR draft. Address every P0/P1/P2 before requesting a fresh exact-head independent Codex review. Do not merge or release.

Consolidated required fixes:

  1. Resolve independent finding codex:91b1babfe144c3614baf from the exact-head Codex audit. Implicit audit authority must use active trusted base policy rather than the PR-head checkout. Reuse the maintained trusted-base/config contract (for example the existing context-audit precedent), while retaining explicit selected configuration. Test a PR that changes its own lane authority and ensure the current audit reports the base policy. Preserve Builder takeover: make provenance, labels, and reviewer exclusion handoff-aware #963's separately owned admission/provenance blocks.
  2. Resolve both broker acceptance P2s. Positive flag/environment overrides cannot bypass an informational lane, disabled role policy, qualification or an explicitly invalid selected configuration. A negative override may narrow computed authority. Also complete the required portable packaged-starter Devin remediation: the path is generated at runtime, so literal source search does not verify it. Carry the selected configuration source/profile into supported command rendering, preserve explicit/custom configs and custom lane names, and test command equivalence with spaces. Never replace a repository config with a starter merely to shorten the command.
  3. Fix the privacy-scan failure at tests/test_adoption_polish_955.py:319. The new privacy test itself contains a forbidden private-home prefix literal. Follow the existing privacy-fixture conventions and run scripts/privacy_scan.py, rather than weakening the scanner or broadening an allowlist.

Broker validation of this head used canonical Python with a private temporary directory outside Git checkouts: 645 tests and 1,146 subtests passed; only the privacy scan failed. The earlier two context-store failures disappear with that correct test environment. The maintained runner intentionally gives the builder a temporary directory inside its checkout; tests that specifically require private stores outside Git must allocate a proper external test root or use an authorized external temporary directory for the test invocation. Preserve the context-store boundary, and do not call that behavior a product defect.

Retain all original work-order acceptance and coordinated ownership. Update the PR description around the completed implementation and actual checks, remove claims based only on literal source searches, and report the new full head. Include focused regressions, documented-command/release-hygiene checks, package materialization for new modules, Ruff and privacy scan. No test may start paid providers; no hosted Devin work, owner credential setup, auto-merge changes or provider authority promotion is authorized. All source changes stay with the same Claude builder.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Additional acceptance guards for the active Claude fix round:

  • --easy is not an explicit packaged-starter selector: doctor prefers a local code-mower.yml, and init prefers a local code-mower.example.yml. A directory can have only the latter, so a doctor finding that used the packaged starter and a subsequent init --easy can select different configurations immediately. Command-equivalence tests must include those local decoy files, and must verify the selected config/profile rather than only compare command strings. Use a supported explicit package selector if needed; preserve all explicit/custom config behavior.
  • Trusted-base lookup must distinguish confirmed absence from unavailable or malformed policy. Confirmed absence can retain maintained defaults. A failed Git lookup, invalid base ref, timeout or malformed tracked configuration must not become packaged_default with merge authority. Optional context-policy discovery's availability fallback is not evidence for an authority claim. Keep the outcome bounded and actionable rather than reading proposed PR policy or silently granting defaults.

These clarify the already outstanding exact-head findings; they do not expand source ownership or authorize a second writer.

…r commands

Implicit audit authority discovery now reads `code-mower.yml` at the trusted
base ref instead of the PR-head checkout the wrappers hand it, so a PR that
promotes or demotes its own review lane no longer changes the audit comment's
authority header before the change is approved. Discovery follows the existing
`context_audit.required_for_repo` contract: a base that does not track the file
configures nothing and keeps the maintained lane default, and discovery that
cannot run falls back to that default rather than to the proposed policy. An
explicitly selected configuration still wins, and a directory that is not a Git
checkout still reads its own file.

Operator overrides are read against the computed configured decision instead of
replacing it. A positive `--merge-authority` flag or environment value can no
longer widen an informational lane, a denied role policy, an unavailable
capability or an unqualified role, and it no longer skips an explicitly selected
configuration that could not be read; the rendered reason stays the configured
one and the ignored override is recorded. A negative override still narrows, and
whichever source decided the rendered posture is still named.

Devin readiness commands carry the configuration source, so a finding against
the packaged starter renders the supported `--easy` selector instead of the
installation-specific path the starter resolves to at runtime. `--easy` is an
alias for the recommended profile, so it only stands in for that profile; every
other posture keeps its pinned, shell-quoted path, and a repository
configuration is never replaced by the starter to shorten a command. The
prompt pack documents the same portable form and the boundary on it.

The new privacy assertion builds its home prefix the way `scripts/privacy_scan.py`
writes its own patterns, so the test no longer trips the scanner it asserts.

Tests: new trusted-base (promotion, demotion, absent base config, unavailable
discovery, explicit selection), non-widening override (informational lane,
denied role policy, agreeing configuration, negative narrowing, missing explicit
configuration) and portable-starter command cases (doctor, transport selection,
custom lane guidance, readiness remediation, repository config preserved,
non-recommended profile, paths and profiles containing spaces).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeffhuber

Copy link
Copy Markdown
Contributor Author

Claude fix round complete for #955 / #988

PR: #988
New head SHA: 6d418542904ddb0b4c073c70de1164acfca9f7c5
Branch: claude/955-adoption-polish (same Claude-owned branch, single writer, no force-push)

1. Codex finding codex:91b1babfe144c3614baf -- resolved

review_authority.resolve_repository_config now takes a base_ref and, for a
Git checkout, reads code-mower.yml at the trusted base through the same
git ls-tree / git show contract context_audit.required_for_repo already
uses, instead of the PR-head working tree both wrappers pass as repo_root.
Both wrappers pass their existing --base-ref through.

  • A base ref that does not track the file configures nothing -> maintained lane
    default.
  • Discovery that cannot run (missing ref, Git failure, unparseable base file)
    also falls back to the maintained default -- never to the proposed
    configuration, which is precisely what must not be read.
  • An explicit --code-mower-config selection still wins over the trusted base.
  • A directory that is not a Git checkout has no base to trust and still reads its
    own file.

Regressions in TrustedBaseAuthorityTests build real temporary Git repositories:
a PR promoting its own lane and a PR demoting its own lane both report the base
policy; absent base config keeps the default; unavailable discovery never falls
back to the head checkout; explicit selection still wins.

2. Broker acceptance P2 #1 (positive overrides) -- resolved

effective_merge_authority now computes the configured decision first and reads
the override against it. A positive flag/env value cannot widen an informational
lane, a denied role_policy, an unavailable capability or an unqualified role;
the configured reason stays the rendered one and override_ignored is recorded.
A negative override always narrows. Neither polarity skips an explicitly selected
configuration that could not be read (ConfigError, which both wrappers already
surface). Covered for Claude and Codex in NonWideningOverrideTests.

3. Broker acceptance P2 #2 (portable starter remediation) -- resolved

_Pin / _pinned / doctor_command / readiness_command /
select_transport_command / custom_lane_guidance / setup_instructions now
carry a config_source, threaded from doctor.py's existing
_doctor_config_source_label through doctor_checks/runner.py and
doctor_checks/devin.py. For config_source == "packaged_starter" under the
recommended profile, commands render the supported --easy selector
(code-mower doctor --easy --devin, code-mower init --easy --set-transport devin=devin_api_v3 --dry-run) rather than the installation path the starter
resolves to. No new CLI flag was invented; --easy is the documented starter
selector and is now also documented in the prompt pack with its boundary.

Preserved: explicit and repository configurations keep their pinned path (never
replaced by the starter to shorten a command); a non-recommended profile under
the starter keeps its explicit pin, because --easy is an alias for the
recommended profile; custom-named lanes keep bounded manual guidance; paths and
profile names containing spaces stay shlex-quoted.

Verification is by command equivalence, not literal source search:
PortableStarterCommandTests passes a stand-in installed path into the
renderers and into a full devin_readiness() run, then asserts that path is
absent from remediation text and JSON detail while --easy is present.

4. Privacy-scan failure -- resolved

tests/test_adoption_polish_955.py builds its home prefix as "/" + "Users/",
the convention scripts/privacy_scan.py uses for its own patterns and
tests/test_board.py:2095 uses for fixtures. The scanner and its allowlist are
unchanged. scripts/privacy_scan.py now passes.

Tests run (scripts/dev-python, Python 3.12.13)

  • tests/test_adoption_polish_955.py: 48 tests, OK (18 new).
  • test_release_hygiene (360), test_documentation, test_audit_comment_posture,
    test_claude_audit_pr, test_codex_audit_pr, test_migration_setup_drift,
    test_participant_sessions, test_provider_runners_comments,
    test_devin_setup_readiness, test_devin_provider_contract,
    test_devin_capabilities, test_operational_evidence and the test_doctor_*
    suites: pass.
  • scripts/privacy_scan.py: passes. ruff check src tests scripts: clean.
  • Package manifest: this round adds no new module, so the committed manifest is
    unchanged and the materializer/manifest hygiene tests still pass.

Environment-only failures on this runner, each reproduced identically with this
branch's changes stashed where checked (so not a source regression):

  1. test_role_eligibility.py::HostedRoleAdmissionTests (2) and
    test_context_readiness.py (10) -- the context store refuses state inside a
    Git checkout, and the maintained runner hands the builder a temporary
    directory inside its checkout. The broker's external test root clears these;
    the context-store boundary is preserved, not worked around.
  2. test_audit_comment_posture::test_cli_posture_defaults_can_be_overridden --
    CLAUDE_AUDIT_MERGE_AUTHORITY is set in the lane runner environment, so the
    parser default is not None here.
  3. test_doctor_registry::test_runner_doctor_uses_repository_template_source_root.

State

  • PR stays draft. Not merged, not released, no auto-merge change.
  • Labels: builder:claude kept; codex-audit-blocked removed and
    needs-codex-audit added to request a fresh exact-head independent Codex
    review on 6d418542904ddb0b4c073c70de1164acfca9f7c5.
  • No paid provider was invoked, no owner credentials touched, no hosted Devin
    work, no provider authority promotion.
  • PR description updated to the implemented behaviour and the checks actually
    run; the previous claim resting on literal source search was replaced with the
    command-equivalence assertion described above.

Remaining

  • Independent exact-head Codex audit on the new head.
  • CI and the authoritative gate on this head.
  • Nothing is blocked on the owner from this lane.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Code Mower Claude maintenance: finish #955 acceptance and integrate main

Previous writer exited cleanly at 6d418542904ddb0b4c073c70de1164acfca9f7c5. Continue the same Claude-owned branch and draft PR #988. Root merged #986 as 547a268e0eedf14b6bc2b4eaf0f36c784494387e; fetch current main and integrate it through this existing writer before final validation. Preserve the merged context/session behavior. No broker source edits and no second writer.

Two P2 acceptance checks still fail on this exact head, as anticipated in the prior guard comment:

  1. Unknown trusted policy must not grant default authority. Calling effective_merge_authority for Codex in a Git checkout with a nonexistent base_ref currently returns config_source=packaged_default and merge_authority=true. _trusted_base_config catches lookup/parse failures and treats them like a verified absent file. Only a successful trusted-tree lookup proving absence may select maintained defaults. Missing/invalid refs, failed Git commands, timeouts and malformed tracked configuration must produce a bounded actionable failure or a clearly non-authoritative unavailable result. Do not silently grant starter authority. Add focused tests for verified absence separately from each unavailable/malformed case; do not use optional-context discovery's permissive availability fallback for an authority claim.

  2. Use an explicit portable package selector, not --easy. A pure command-resolution check in a directory containing a local code-mower.example.yml and no code-mower.yml shows doctor --easy selecting the packaged starter while init --easy selects the local example. Both commands can also be redirected by cwd-local files, and init --easy forces the recommended profile. Add a narrow explicit supported packaged-starter selector to doctor and init (for example --packaged-starter) that directly chooses the maintained package resource, independently of cwd-local files, and retains the selected profile. Reject contradictory explicit positional-config selection instead of ignoring it. Render that selector for actual packaged-starter findings; retain exact quoted repository/custom config paths and custom-lane guidance. Test CLI-level equivalence with local decoy code-mower.yml and code-mower.example.yml, non-default profiles and paths containing spaces. Update the prompt pack, command documentation and command-contract tests for the real supported selector. Do not add a prose-only command or strip a selected config to make a shorter command.

Preserve the completed positive-override narrowing, trusted-base-vs-proposed-head distinction, bridge preview, concise/full/JSON doctor behavior, quiet unselected providers, campaign-intent behavior, and role/lease documentation. Keep the existing privacy scanner intact. Ensure tests that assert an unset audit environment explicitly control that environment, so ambient operator overrides do not invalidate an unrelated default-argument assertion. Use the maintained scripts/dev-python selector if host hooks require it; preserve private context stores outside Git checkouts for those tests.

Original ownership still applies: #963 owns admission/provenance/gate/controller/runner and init _author_exclusion_payload; this work owns presentation/config propagation, doctor/remediation, drift and optional setup guidance. Coordinate through the broker if integration overlaps those reserved blocks. No hosted Devin sessions, paid-provider tests, owner credential setup, automatic workflow deletion, merge or release.

Run meaningful focused regressions, package materialization/inventory where affected, documented commands/release hygiene, Ruff and privacy scan. Remove stale PR claims that unavailable policy grants maintained defaults or that --easy is an explicit package selector. Report the new full head and actual tests; keep draft for fresh independent Codex review and current-head CI/gate.

jeffhuber and others added 2 commits September 14, 2026 09:14
…selector

Unknown trusted base policy no longer grants maintained defaults. Only a
successful trusted-tree lookup that proves `code-mower.yml` is absent selects
lane defaults; a missing or invalid base ref, a failed or timed-out Git command
and malformed tracked configuration now report `trusted_base_unavailable`, which
renders informational with a bounded action instead of starter merge authority.
A positive operator override cannot widen it either.

Rendered packaged-starter commands name a real supported selector. `--easy` is a
first-run profile alias whose starter fallback depends on cwd-local files, and
doctor and init fall back to different ones, so it could select a different
configuration than the finding described. `code-mower doctor --packaged-starter`
and `code-mower init --packaged-starter` select the maintained package resource
directly, ignore cwd-local files, keep the caller's `--profile`, and reject a
contradictory explicit positional config rather than ignoring it. Repository and
custom config paths stay pinned and shell-quoted, and custom-lane guidance keeps
its bounded manual form.

Tests build real Git repositories for verified absence versus each unavailable
case, and assert CLI-level selection against `code-mower.yml` and
`code-mower.example.yml` decoys rather than comparing command strings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeffhuber

Copy link
Copy Markdown
Contributor Author

Claude fix round complete for #955 / #988

PR: #988
New head SHA: aa0be46b50a0c13351dfec7aabaa9d1904c7c609
Branch: claude/955-adoption-polish (same Claude-owned branch, single writer, no force-push)

0. Current main integrated

Merged origin/main (#986, 547a268) into this branch as ff3361f. The merge was clean -- no conflicts, and no edits to the merged Coworker citation or context/session behavior.

1. Unknown trusted policy no longer grants default authority -- resolved

_trusted_base_config no longer collapses lookup and parse failures into a verified absent file. Only a successful git ls-tree proving the file is absent returns packaged_default. Everything else returns the new review_authority.TRUSTED_BASE_UNAVAILABLE source:

  • missing or invalid base ref;
  • failed Git command (not a real checkout, bad repo);
  • timeout or OS error;
  • tracked configuration that does not parse, or parses to something other than a mapping.

effective_merge_authority renders that source as a bounded, clearly non-authoritative result: merge_authority=false, configured_merge_authority=false, scope=informational, policy_source=unavailable, reason=trusted_base_unavailable, plus an action naming what makes it resolvable (fetch the base ref, or pass --code-mower-config). A positive operator override cannot widen it -- it records override_ignored. No optional-context availability fallback is used for the authority claim.

The specific reported case now behaves correctly: Codex in a Git checkout with a nonexistent base_ref returns config_source=trusted_base_unavailable and merge_authority=false.

Tests cover verified absence separately from each unavailable/malformed case: test_verified_absence_is_distinguished_from_unavailability, test_a_missing_base_ref_never_grants_the_maintained_default, test_malformed_tracked_configuration_is_unavailable_not_default, test_a_failed_git_lookup_is_unavailable_not_default, test_a_positive_override_cannot_widen_an_unavailable_base.

2. Explicit portable package selector, not --easy -- resolved

Added --packaged-starter to both code-mower doctor and code-mower init. It selects the maintained package resource via package.packaged_starter_config_path(), which resolves relative to the installed module (code_mower/templates/code-mower.example.yml, or the source-checkout copy), never relative to cwd.

  • Ignores cwd-local code-mower.yml and code-mower.example.yml entirely.
  • Retains the selected --profile; it chooses the resource, not a profile. --easy remains a first-run profile alias and is unchanged.
  • Rejects a contradictory explicit positional config with exit 1 rather than ignoring either selection. Both parsers now default the positional after parsing, so an explicit code-mower.yml is distinguishable from the default of the same name.
  • code-mower init --packaged-starter routes through the CLI wrapper without a default positional config being injected.

devin_readiness renders --packaged-starter --profile <profile> for packaged-starter findings at any profile. Repository and custom config paths stay pinned and shell-quoted, a repository configuration is never replaced by the starter, and custom-lane guidance keeps its bounded manual form. Prompt pack updated to document the real selector and to say explicitly that --easy is not one and why.

Command equivalence is tested at CLI level in a directory carrying both decoy files (PackagedStarterSelectorTests), asserting the selected config and profile, not command strings.

Stale claims removed

The PR body no longer says unavailable policy falls back to maintained defaults, and no longer describes --easy as the packaged-starter selector or as limited to the recommended profile.

Tests run on this head

scripts/dev-python (Python 3.12.13), with this checkout's src pinned ahead of the ambient editable install (which resolves to the control checkout on this runner).

  • tests/test_adoption_polish_955.py -- 59 passed, 2 subtests.
  • test_release_hygiene, test_documentation, test_devin_setup_readiness, test_migration_setup_drift, test_audit_comment_posture, test_claude_audit_pr, test_codex_audit_pr -- 497 passed, 899 subtests.
  • test_doctor_*, test_init_*, test_package_*, test_smoke_* -- 301 passed, 106 subtests, 8 failed.
  • scripts/privacy_scan.py -- passes. ruff check src tests scripts -- clean.

The 8 failures are all tests/test_init_build_loop.py::InitBuildLoopTests cases that shell out to tools/lanes/run_mac_lane.sh on this host. I verified they are environment-only: stashing this branch's changes and rerunning that file reproduces the identical 8 failures. Package manifest is unchanged (no new module this round) and test_release_hygiene passes.

Remaining

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Codex audit (merge-authority lane)

Head SHA: aa0be46b50a0c13351dfec7aabaa9d1904c7c609
Findings: P0=0, P1=0, P2=2, P3=0 (blocker policy: any P0/P1/P2 → BLOCKED)

Codex Audit: BLOCKED

Summary:

Audit authority can use a different base revision than the review, and migration verification checks the packaged starter instead of the installed configuration.

Findings:

  • [P2] Resolve authority after fetching the audit's base revision -- src/code_mower/codex_audit_pr.py:2636
    Finding ID: codex:52d95056d251c9970ba2
    When the local base ref is stale or missing, comment posture is resolved before audit_pr() fetches the base. The review uses the refreshed base but retains authority calculated from the old policy, or reports informational because the ref was initially unavailable. This can incorrectly retain merge-authority wording after a repository demotion. Resolve posture against the fetched revision used for the review; the Claude wrapper has the same ordering issue.
  • [P2] Verify the installed config after switching starter transports -- src/code_mower/devin_readiness.py:917
    Finding ID: codex:a615178ef3ad7ed076a8
    For findings sourced from the packaged starter, the post-install verification command retains --packaged-starter. That selector ignores the newly installed repository configuration, so verification keeps reporting the unchanged starter transport instead of confirming the switch. Keep the starter selector for preview and staging, but point the final doctor command at the installed repository configuration.

Source locations normalized to repository-relative paths for public privacy; the original reviewer output is retained privately.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Code Mower Claude fix round: finish the two exact-head Codex P2 findings

Continue the same Claude-owned branch and draft PR #988. The prior runner exited 0 and Code Mower confirmed the writer finished and quiescent at aa0be46b50a0c13351dfec7aabaa9d1904c7c609. Current main 547a268e0eedf14b6bc2b4eaf0f36c784494387e is already integrated. Do not create another PR or alter another writer's branch.

Address both findings from the independently published exact-head Codex audit:

  1. codex:52d95056d251c9970ba2: effective authority must be computed from the already-fetched exact base snapshot used for this audit, after the existing fetch. The current main() calculation can retain stale authoritative policy after a repository demotion or misreport an initially missing ref. Preserve explicit configuration and non-widening override semantics. Add meaningful wrapper-level stale/missing-base regressions, using offline/fake provider execution, that prove the rendered posture and review consume the same fetched revision. Keep unavailable policy fail-closed.

    The orchestrator and Builder takeover: make provenance, labels, and reviewer exclusion handoff-aware #963 owner explicitly coordinated this narrow boundary: Claude _build_diff_context may expose its existing fetched_base_ref SHA on DiffContext (compatibly defaulted for historical fixtures). Codex may pin its already-fetched base SHA immediately after the existing fetch block and carry that same snapshot into review/context and effective-authority rendering. Do not add a second fetch or policy resolver. Do not modify Builder takeover: make provenance, labels, and reviewer exclusion handoff-aware #963's independent-review admission/helper logic, trusted comments, contributor/provenance inputs or state machine. That lane owns fetch/admission; this lane owns only presentation and the data needed to render the fetched snapshot accurately. Preserve backwards compatibility for direct AuditConfig callers and historical fixtures without letting the maintained CLI path bypass the new calculation.

  2. codex:a615178ef3ad7ed076a8: the final readiness check after transport installation must inspect the installed repository configuration and the same profile. Retain --packaged-starter for the initial preview/staging selection, then make the post-install command explicitly select the installed repository config, not the unchanged starter. Preserve explicit/custom configuration paths, custom lane guidance and quoted spaces. Add a regression that follows the staged/installed transport transition and verifies that the final command selects the changed installed configuration/profile while initial setup still selects the package resource. Keep preview/staging and normal reviewed installation distinct.

All 59 broker-run adoption regressions, Ruff and privacy scan passed on the prior head, and all CI package/containment jobs passed. The broker's full canonical suite is still running against that immutable prior head; do not change or interfere with its checkout. Use focused tests for your edits, the relevant audit/doctor/init/command/package/privacy regressions, and preserve all previously accepted behavior. The broker will run fresh final-head full checks and another exact-head Code Mower Codex audit after you finish.

Update the PR description to describe the final implementation and actual validation concisely; omit abandoned approaches and long chronological fix history. Keep Closes #955, builder:claude, draft status, and request needs-codex-audit after pushing. No hosted Devin creates, paid-provider tests, owner credential setup, automatic workflow removal, merge or release. Source author remains this Claude writer; the broker only coordinates and verifies.

…d config

Both audit wrappers resolved review authority before `audit_pr` fetched the
base ref, so a stale local ref kept merge-authority wording through a
repository demotion, and a base that was simply not fetched yet reported as
unavailable -- while the review itself used the refreshed revision.

`review_authority.AuthorityRequest` now carries the wrapper's product,
explicit configuration selection and operator override instead of a resolved
answer. Each wrapper validates an explicit `--code-mower-config` up front, so
an operator typo still fails before any network work, and stays fail-closed
informational until the posture resolves. Codex pins the base SHA immediately
after its existing fetch and resolves against it; Claude records the revision
`_build_diff_context` already fetched on `DiffContext.fetched_base_ref` and
resolves against that, so the rendered header and the diff describe the same
base. No second fetch and no second policy resolver. Configs without a request
keep the authority they were given, so direct callers and recorded fixtures are
unchanged.

Devin transport-switch steps keep `--packaged-starter` for preview and staging,
but the post-install check now selects the installed repository configuration
at the same profile: an install never rewrites the packaged starter, so
verifying through that selector re-reported the unchanged starter transport
instead of confirming the switch. Repository-sourced findings install over
their own file and keep their own path.

Regressions drive both wrappers over real temporary Git repositories with
offline provider fakes, advancing `origin/main` inside the fetch the way a real
one would.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeffhuber

Copy link
Copy Markdown
Contributor Author

Claude fix round complete for #955 / #988

PR: #988
New head SHA: 210660f07cfbdb07fb779164718cc548276f7122
Branch: claude/955-adoption-polish (same Claude-owned branch, single writer, no force-push)

Both exact-head Codex P2 findings are resolved. No new PR, no second writer, no
merge, no release, no hosted Devin or paid-provider work.

1. codex:52d95056d251c9970ba2 — authority is resolved from the fetched base

review_authority.AuthorityRequest now carries the wrapper's product, explicit
configuration selection and operator override instead of a resolved posture, and
the single existing resolver runs after the audit's existing fetch:

  • Codex (codex_audit_pr.audit_pr): the base SHA is pinned immediately after
    the existing _fetch_base_ref block and the posture is resolved against that
    pinned snapshot.
  • Claude (claude_audit_pr.audit_pr): _build_diff_context now records the
    revision it already fetched on DiffContext.fetched_base_ref, and the posture
    is resolved against that same revision — so the rendered header and the diff
    consume one snapshot. The force-push/STALE branch resolves too, since the base
    was fetched before the head mismatch was detected.

No second fetch, no second policy resolver. Explicit configuration and the
non-widening override semantics are unchanged — the override is still read
against the computed configured decision, and a positive override cannot widen a
fetched demotion. Unavailable policy stays fail-closed: each main() now
constructs the config with merge_authority=False plus the request, so anything
rendered before the resolution is informational rather than an unverified claim.
An explicit --code-mower-config is still validated up front via
AuthorityRequest.validate(), so an operator typo fails before any network work.

Backwards compatibility: a config carrying no authority_request is returned
unchanged, so direct AuditConfig / ClaudeAuditConfig callers and recorded
fixtures keep the authority they were given. DiffContext.fetched_base_ref
defaults to "", and the historical (stat, diff, truncated) unpacking is
unchanged. The maintained CLI path always carries a request, so it cannot bypass
the new calculation.

#963's boundary is respected: no changes to entry admission, trusted comments,
contributor/provenance inputs, the shared exclusion helpers or the state machine.
This lane touched only presentation and the fetched-revision data it renders.

Regressions (FetchedBaseAuthorityTests, real temporary Git repositories,
offline provider fakes, both wrappers driven through audit_pr with the fetch
advancing origin/main the way a real one would):

  • a stale local ref plus a fetched demotion renders informational even when the
    pre-fetch value was merge authority (Codex and Claude);
  • a base that only the fetch makes available renders its real policy instead of
    trusted_base_unavailable (Codex and Claude);
  • a positive override cannot widen the fetched demotion;
  • a config with no request keeps the authority it was given (Codex and Claude);
  • the rendered label equals the posture at the exact fetched revision and not the
    one the PR-head checkout declares;
  • a historically constructed DiffContext records no fetched revision and keeps
    its unpacking API.

2. codex:a615178ef3ad7ed076a8 — verification inspects the installed config

select_transport_command keeps --packaged-starter for the steps that read the
starter — the dry-run preview and the --apply staging — and the final check now
selects the installed repository configuration at the same profile
(code-mower doctor code-mower.yml --profile <profile> --devin). An install
writes the repository's own configuration and never rewrites the packaged
starter, so verifying through the starter selector re-reported the unchanged
starter transport instead of confirming the switch.

A finding already sourced from a repository configuration installs over that same
file, so its own path and profile stay the verification target. Custom lane
guidance, explicit configuration paths and shell quoting are unchanged.
Preview/staging and normal reviewed installation stay distinct.

Regressions: starter-sourced steps preview and stage with --packaged-starter
and verify with the installed code-mower.yml at a non-recommended profile
(advanced), with the runtime-shaped installation path absent from every step; a
repository-sourced finding keeps ops/mower.yml in its verification; and
devin_readiness.INSTALLED_CONFIG_PATH is pinned to init.ADOPTION_CONFIG_PATH
so the two cannot drift.

Validation on this head

Run with scripts/dev-python (Python 3.12.13) with this checkout's src ahead of
the ambient editable install:

  • tests/test_adoption_polish_955.py — 71 tests, all passing.
  • test_adoption_polish_955, test_claude_audit_pr, test_codex_audit_pr,
    test_audit_comment_posture, test_devin_setup_readiness,
    test_migration_setup_drift, test_documentation, test_release_hygiene
    568 passed, 901 subtests passed, 0 failures.
  • ruff check src tests scripts — clean. scripts/privacy_scan.py — passes.

The doctor / init_ / package_ / smoke_ sweep was still running on this
host when the lane's wall-clock budget ran out; the failures it had reported at
that point are the previously recorded environment-only
tests/test_init_build_loop.py::InitBuildLoopTests entries, which invoke
tools/lanes/run_mac_lane.sh on this machine. The broker's fresh full-head run
is authoritative for that sweep.

State

PR kept as a draft. builder:claude retained, needs-codex-audit re-requested on
this head. Nothing remains from the two findings; the next step is the broker's
exact-head Codex audit at 210660f.

🤖 Generated with Claude Code

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Code Mower Claude continuation: carry the fetched snapshot through review and context

The prior same-owner round exited 0 and Code Mower confirmed finished/quiescent at 210660f07cfbdb07fb779164718cc548276f7122. Continue only this Claude-owned branch and draft PR #988. No second writer or new PR.

One part of the prior snapshot requirement is incomplete: Codex passes _pinned_base_revision(...) into _resolve_fetched_authority, but that helper replaces only merge_authority. Subsequent review diagnostics, context and run_codex_review still consume the mutable named config.base_ref. Claude likewise exposes the fetched SHA on DiffContext for posture but must carry that snapshot to downstream context consumers too. This leaves the claim that all consumers describe one fetched revision unproven if the tracking ref moves after the fetch.

The orchestrator has explicitly required completion in #955, and the #963 owner confirmed the precise boundary: after the existing fetch, carry config.base_ref as the pinned SHA for downstream diff/context/review; keep the original named ref only as descriptive input if needed. Expose/reuse the existing fetched values. Do not move admission, introduce another fetch, add another policy resolver, or change contributor/provenance state. #963's later integration will consume this already-pinned SHA without resolving a mutable ref again. This clarification supersedes the earlier proposal to defer propagation to #963.

Add an offline regression that advances the named tracking ref after the fetch and proves both the rendered authority and downstream review/context stay on the fetched SHA. Cover both wrappers as applicable; keep direct-call/fixture compatibility, explicit configuration validation, fail-closed unavailable policy and non-widening overrides. Preserve the installed-config transport verification already fixed in the prior round.

Run the new regression and relevant audit/adoption/command/privacy/package-hygiene checks. Do not spend the round repeating the long doctor/init/package/smoke sweep: the broker independently completed the earlier frozen aa0be46 full suite, 3,642 tests in 565 seconds, 16 skips, including all InitBuildLoopTests. The builder-environment failures did not reproduce there. The broker will run the final full canonical suite and fresh exact-head Codex audit after this bounded continuation.

Update the PR's final behavior and actual test evidence concisely, push the same branch, and re-request needs-codex-audit. Keep draft. No hosted Devin creates, paid-provider tests, credentials, merge or release. Source author remains Claude; broker only coordinates and verifies.

Resolving the audit posture against the fetched base was not enough on its
own: base_ref stayed a mutable name, so the trusted-ref lookups, the review
context and the review itself could resolve it again later and read a
different commit than the one the audit fetched.

Both wrappers now pin the fetched revision onto config.base_ref right after
the existing fetch, so every downstream consumer -- decision authorities,
context preparation, plan context, review diagnostics, the review doctrine
load and the review's own --base and prompt -- describes one snapshot. Codex
pins the SHA it already resolved after _fetch_base_ref; Claude pins the
revision _build_diff_context already fetched. A ref that will not resolve
keeps its name, so an unavailable base stays unavailable. No second fetch, no
second policy resolver, no admission or provenance change.

The force-push race path carries the fetched revision too, through a local
FetchedHeadMismatch subclass, leaving the shared exception contract and every
existing handler unchanged.

Regressions advance origin/main after the fetch -- the way an upstream merge
landing mid-review would -- and assert the rendered posture and the downstream
review and context all stay on the fetched SHA, for both wrappers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeffhuber

Copy link
Copy Markdown
Contributor Author

Claude continuation complete for #955 / #988

PR: #988
New head SHA: d52782405b18d7e280a1330748daccccb0e9a74b
Branch: claude/955-adoption-polish (same Claude-owned branch, single writer, no force-push)

The incomplete part of the snapshot requirement is done: the fetched revision is
now carried through review and context, not only into the posture.

What changed

base_ref stayed a mutable name after the fetch, so the trusted-ref lookups,
the review context and the review itself could resolve it again and read a
different commit than the audit fetched. Both wrappers now pin the
already-fetched revision onto config.base_ref immediately after the existing
fetch:

  • Codex (codex_audit_pr.audit_pr): the SHA _pinned_base_revision already
    resolved after _fetch_base_ref is written onto the config, so
    _decision_authorities_for_repo, context_audit.prepare,
    render_plan_context, _build_review_context_diagnostics and
    run_codex_review's own --base all read that one revision.
  • Claude (claude_audit_pr.audit_pr): diff_context.fetched_base_ref is
    written onto the config, so the trusted-ref lookups, load_review_prompt's
    trusted_git_ref and the review prompt's base all match the diff.
  • Force-push race: the base is fetched before the head mismatch is detected,
    so the STALE path carries that revision too, via a wrapper-local
    FetchedHeadMismatch subclass. The shared exception in provider_runners is
    unmodified and every existing except FetchedHeadMismatch handler still
    catches it with its existing attributes.

A ref that will not resolve keeps its name, so an unavailable base stays
unavailable rather than being quietly replaced by something that resolves.
No second fetch, no second policy resolver, no admission, contributor or
provenance change. Explicit configuration validation, fail-closed unavailable
policy, non-widening overrides, direct-call/fixture compatibility and the
installed-config transport verification from the prior round are all unchanged
and still covered.

Regression

FetchedBaseAuthorityTests gains four tests on real temporary Git repositories
with offline provider fakes. Each advances origin/main to a re-promotion
after the audit already fetched it -- the way an upstream merge landing
mid-review would -- and then asserts, per wrapper, that the trusted-ref context
call, the review-context/doctrine call and the review itself all equal the
fetched SHA (never the name, never the re-promotion) while the comment still
renders informational. Plus the force-push race rendering the fetched base's
posture, and the subclass staying catchable as the shared exception.

All four fail on the parent commit 210660f with 'origin/main' != <sha>.

Tests run on this head

scripts/dev-python (Python 3.12.13), with this checkout's src ahead of the
ambient editable install.

  • test_adoption_polish_955.py: 75 passed (was 71).
  • 692 passed, 939 subtests across test_adoption_polish_955,
    test_claude_audit_pr, test_codex_audit_pr, test_audit_comment_posture,
    test_provider_runners_pr_worktree, test_provider_runners_git,
    test_provider_local_cli, test_decision_audit_integration,
    test_devin_cli_audit_pr, test_plan_context, test_checks,
    test_role_eligibility, test_context_audit, test_devin_setup_readiness,
    test_migration_setup_drift, test_documentation, test_release_hygiene --
    every suite in the tree that references base_ref, plus the adoption, doc and
    hygiene suites.
  • ruff check src tests scripts clean; scripts/privacy_scan.py passes.
  • No new module, so the package manifest is unchanged.

Twelve failures on this runner are environment-only and were reproduced
identically with this branch's src/ and tests/ stashed at the parent commit:
nine in test_context_audit.py and two in
test_role_eligibility.py::HostedRoleAdmissionTests (context store refuses state
inside a Git checkout), and test_checks.py::test_cli_detect_json (detects this
runner checkout's own configuration).

Per the continuation instruction I did not repeat the long doctor/init/package/
smoke sweep; the broker's frozen aa0be46 full run covered it.

Remaining

  • Fresh exact-head Codex audit on d52782405b18d7e280a1330748daccccb0e9a74b --
    needs-codex-audit re-requested.
    The stale codex-audit-blocked label from the aa0be46 audit is left in
    place for the audit run to replace; both of its P2 findings are addressed on
    this head.
  • Still a draft for orchestrator-controlled merge after CI and the authoritative
    gate pass. No merge, no release, no hosted Devin, no paid-provider work, no
    credentials.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Codex audit (merge-authority lane)

Head SHA: d52782405b18d7e280a1330748daccccb0e9a74b
Findings: P0=0, P1=0, P2=0, P3=0 (blocker policy: any P0/P1/P2 → BLOCKED)

Codex Audit: PASS

Summary:

No actionable regressions found relative to the merge base. Assessment based on code inspection; tests were not run.

Findings: none.

@jeffhuber
jeffhuber marked this pull request as ready for review September 14, 2026 19:23
@jeffhuber
jeffhuber merged commit db4506d into main Sep 14, 2026
16 checks passed
@jeffhuber
jeffhuber deleted the claude/955-adoption-polish branch September 14, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

builder:claude Code Mower generated label codex-audit-done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adoption polish: report effective review authority and superseded Devin bridge drift

1 participant