Skip to content

refactor: use types.Issue in merge driver to prevent field drift #1481

@turian

Description

@turian

Problem

internal/merge/merge.go defines its own Issue struct (lines 43-66) that duplicates types.Issue. When fields are added to types.Issue, developers must remember to update the merge driver's copy. This has caused silent data loss:

Root Cause: Struct Duplication Drift

Go's JSON unmarshaler ignores unknown fields by default. When types.Issue gained new fields (like Metadata), the merge driver's local struct wasn't updated. Tests passed because they didn't check field preservation, and the compiler didn't complain.

Current State

The merge driver already depends on internal/types:

import "github.com/steveyegge/beads/internal/types"

const (
    StatusTombstone = string(types.StatusTombstone)
    StatusClosed    = string(types.StatusClosed)
)

So "vendored code purity" is not a constraint.

Proposed Fix: Use types.Issue Directly

Refactor internal/merge/merge.go to use types.Issue instead of its local struct.

Changes Required

  1. Remove local Issue struct definition
  2. Use types.Issue throughout merge logic
  3. Handle merge-specific fields:
    • RawLine — Add to types.Issue with json:"-" tag, or use a wrapper struct
    • Time fields — types.Issue uses time.Time, merge uses string (needs adapter)

Wrapper Approach (if needed)

type MergeIssue struct {
    types.Issue
    RawLine string `json:"-"` // Merge-specific: original JSON line for conflict output
}

Alternatives Considered

Option Pros Cons
A. Use types.Issue Guaranteed sync, compiler enforces May need wrapper for merge-specific fields
B. map[string]json.RawMessage Auto-preserves unknown fields Complex custom marshal/unmarshal
C. Struct parity test Low risk, catches drift Doesn't fix architecture

Recommendation: Option A. The dependency already exists, and this prevents future data loss.

Acceptance Criteria

  • internal/merge/merge.go imports and uses types.Issue (or a thin wrapper)
  • All fields from types.Issue are preserved during merge
  • Existing merge tests pass
  • Add test: round-trip an issue with metadata through merge, verify preservation

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions