Skip to content

[Phase 4] Post-conflict-resolution integrity check — catch duplicated/corrupted content before it's trusted #1482

Closed
#1496
Closed
[Phase 4] Post-conflict-resolution integrity check — catch duplicated/corrupted content before it's trusted#1482
#1496
Labels
bugBug reportsdev-leadFor dev-lead agent pickupinitiativeEpic / initiative tracking issue

Description

@don-petry
don-petry
opened on Aug 7, 2026

Story

As a maintainer relying on dev-lead/auto-rebase to resolve conflicts automatically,
I want a post-resolution integrity check that catches content corruption (duplicated declarations, silently dropped content) before the resolution is trusted,
so that a botched conflict resolution is caught immediately at the point it happens, not discovered hours later by an unrelated test that happens to exercise the corrupted code.

Problem — conflict resolution completed, but the content was wrong, and nothing checked

#1449, overnight 2026-08-06→07: two auto-rebase-conflict events (01:37Z, 09:37Z) against main. After the second, scripts/engine.sh — a file this PR's own story explicitly says it shouldn't touch — had grown from main's 2688 lines to 4011 (1323 extra), with core functions (run_writer, parse_reset_time, extract_verdict_json, etc.) duplicated roughly 2x their count on main. This wasn't caught by any purpose-built guard: it surfaced only because the Test Dev-Lead Agent / unit job happened to exercise the corrupted function, producing 60+ unrelated-looking failures (rate-limit handling, model-chain fallback, redaction) that took real investigation to trace back to the actual cause. It was found manually during steering review, not by CI design.

This had already happened once, earlier in the same PR's lifetime: commit f5bbcc5c fix(ci): remove duplicated function/variable declarations that broke unit and bats tests shows the exact same failure mode was hit and patched — and then recurred on the very next rebase. The fix didn't generalize into a check; it fixed the instance.

This is a distinct gap from what's already tracked

Acceptance Criteria

  1. After any automated conflict-resolution commit (auto-rebase's update-branch, or dev-lead's rebase intent in dev-lead-fix-reviews.sh), a lightweight integrity check runs against the resolved tree, at minimum detecting duplicate top-level function/variable declarations within a file that were not already duplicated in both parent versions.
  2. The check runs as part of the conflict-resolution path itself — not dependent on an unrelated test suite happening to exercise the corrupted code.
  3. On detecting an anomaly, the PR gets a specific, actionable comment naming the file and the duplicated symbols (mirroring the manual finding on feat: implement issue #687 — [Phase 2] Generalized CI compliance test: reject SHA / # main pins on any first-party reusable #1449) — the resolution is flagged, not silently trusted.
  4. Signal-only initially: post a comment, do not auto-block or auto-revert (mirrors [Phase 4] Detect merge-ready PRs stranded with no agent acting and no signal #1469's caution about automated actions on PRs re-arming agent churn) — escalate to blocking only after the detector has run advisory for a period with no false positives.
  5. Regression coverage using feat: implement issue #687 — [Phase 2] Generalized CI compliance test: reject SHA / # main pins on any first-party reusable #1449's actual corruption (scripts/engine.sh 2688→4011 lines, run_writer/parse_reset_time/extract_verdict_json duplicated) as the fixture.
  6. Scoped to avoid false positives on legitimate large diffs: a file that's supposed to grow substantially (e.g., a real upstream feature merge) shouldn't trip this — duplicate-declaration detection is the precise signal; a raw line-count-delta heuristic is advisory-only, not a trigger on its own.

Tasks / Subtasks

Dev Notes

References

Filed from steering review of epic #1402, 2026-08-07, per maintainer request after #1449's engine.sh corruption was found and flagged.

Activity

  1. don-petry
    added a parent issue on Aug 7, 2026
  2. don-petry
    added
    bugBug reports
    initiativeEpic / initiative tracking issue
    dev-leadFor dev-lead agent pickup
    on Aug 7, 2026

  3. don-petry commented on Aug 12, 2026

    @don-petry
    don-petry
    on Aug 12, 2026
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #1482 — [Phase 4] Post-conflict-resolution integrity check — catch duplicated/corrupted content before it's trusted

    Scope

    Add a narrow, mechanical duplicate-top-level-declaration detector for shell scripts and run it on the dev-lead rebase (conflict-resolution) path immediately after a resolution lands, posting an advisory, signal-only comment naming the file and duplicated symbols when the resolution introduced new duplicate declarations — without auto-blocking or auto-reverting.

    Design

    • scripts/lib/conflict-integrity.sh (new pure-logic lib): extract_top_level_symbols (column-0 fn:/var: declarations), symbol_counts, and new_duplicate_symbols <resolved> <parentA> <parentB> — flags a symbol only when its declaration count in the resolved file exceeds its count in both parents (so pre-existing/legit repetition and large legitimate upstream merges don't trip it — AC feat: feature ideation framework — adversarial proposer/challenger/synthesizer #6). Plus format_integrity_findings to render the comment body.
    • scripts/dev-lead-fix-reviews.sh rebase intent: capture the pre-resolution SHA before the engine runs; after a successful resolution, diff changed *.sh files, extract each file's two parent blobs (origin/$BASE_REF and the pre-resolution SHA), run the detector, and post one idempotent advisory comment (<!-- dev-lead-conflict-integrity ... -->) on any finding. The check never changes the exit code (signal-only, AC Optimize review: small-PR and incremental fast paths #4).

    Implementation Checklist

    Tests to Write

    Test Command

    bats tests/dev-lead/unit/test_conflict_integrity.bats (full dev-lead suite: bats tests/dev-lead/unit/); lint: bash .dev-lead/scripts/dev-lead-lint.sh

  4. don-petry commented on Aug 12, 2026

    @don-petry
    don-petry
    on Aug 12, 2026
    CollaboratorAuthor

    Dev-Lead: Implementation Complete

    Plan Execution

    How it works

    The detector emits top-level fn:/var: declarations (column-0 only, so nested/local decls are ignored) and flags a symbol only when its declaration count in the resolved file exceeds its count in both parents — the base ref (origin/<base>) and the branch tip captured before the resolution. A raw line-count delta is deliberately not a trigger (AC #6); duplicate-declaration count-growth is the precise signal. It is signal-only: the advisory comment is idempotent (<!-- dev-lead-conflict-integrity pr=N -->) and the call is || true-guarded so a detector fault can never fail a real resolution.

    Scope note

    Wired into dev-lead's rebase intent only. auto-rebase.yml is a thin caller stub delegating to a cross-repo reusable (petry-projects/.github) that AGENTS.md/CLAUDE.md forbid modifying here; the issue's Dev Notes name dev-lead-fix-reviews.sh's rebase intent as the shared home, which is where it landed. If the check is later promoted into the central auto-rebase reusable, the pure lib is reusable as-is.

    Test Results

    # new detector/regression suite
tests/dev-lead/unit/test_conflict_integrity.bats — 8/8 ok
  ✓ extract_top_level_symbols: emits top-level functions and vars
  ✓ extract_top_level_symbols: ignores indented / nested declarations
  ✓ new_duplicate_symbols: flags the #1449 doubled engine.sh symbols
  ✓ new_duplicate_symbols: reports the resolved declaration count
  ✓ new_duplicate_symbols: a correct union resolution is clean
  ✓ new_duplicate_symbols: does not flag a symbol already duplicated in a parent
  ✓ new_duplicate_symbols: missing parent blob is treated as empty (new file)
  ✓ format_integrity_findings: names the file and the duplicated symbols

# full dev-lead unit suite (test-dev-lead.yml auto-discovers tests/dev-lead/unit/*.bats)
783 ok, 0 not-ok

# repo lint
bash .dev-lead/scripts/dev-lead-lint.sh → all checks passed (shellcheck 104 files + agent frontmatter)
shellcheck --severity=warning -x on changed scripts → clean

    An end-to-end smoke test in a scratch git repo (base → branch → botched resolution on HEAD, origin/main set) confirmed the wrapper's git plumbing extracts both parent blobs via git show and reproduces the exact #1449 finding:

    - `f.sh` — duplicated top-level declarations after resolution:
  - `fn:extract_verdict_json` (declared 2 times)
  - `fn:parse_reset_time` (declared 2 times)
  - `fn:run_writer` (declared 2 times)
  - `var:MAX_RETRIES` (declared 2 times)

    Files Changed

    • scripts/lib/conflict-integrity.sh (new): extract_top_level_symbols, symbol_counts, new_duplicate_symbols, format_integrity_findings.
    • scripts/dev-lead-fix-reviews.sh: source the lib; capture pre-resolution SHA; run the check + post the advisory comment (post_integrity_warning, run_post_resolution_integrity_check) in the rebase intent.
    • tests/dev-lead/unit/test_conflict_integrity.bats (new): regression + false-positive coverage.
    • tests/dev-lead/fixtures/conflict-integrity/ (new): parent_base.sh, parent_branch.sh, resolved_corrupted.sh (the feat: implement issue #687 — [Phase 2] Generalized CI compliance test: reject SHA / # main pins on any first-party reusable #1449 fixture), resolved_clean.sh, parent_predup.sh, resolved_large_legit.sh.

    Notes

  5. don-petry
    added a commit that references this issue on Aug 14, 2026

    feat: implement issue #1482 — [Phase 4] Post-conflict-resolution inte…

    f0764b0
  6. don-petry
    closed this as completedin #1496on Aug 14, 2026
  7. don-petry
    added a commit that references this issue on Aug 18, 2026

    feat: implement issue #1482 — [Phase 4] Post-conflict-resolution inte…

    d74d06d
  8. don-petry
    added a commit that references this issue on Aug 30, 2026

    feat: implement issue #1532 — conflict-integrity parser misses next-l…

    e036040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickupinitiativeEpic / initiative tracking issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions