Skip to content

Commit d03d9d2

Browse files
committed
chore: add marker removal detection to CI and regression guard tests
Two new safety nets prevent `altimate_change` markers from being silently removed: 1. **CI marker guard** (`analyze.ts --markers`): now detects net marker removal in diffs — if more markers are deleted than added, the check warns/fails. This is what would have caught #316 before merge. 2. **Regression test** (`upstream-merge-guard.test.ts`): minimum marker count per file. If a file's marker count drops below its recorded floor, the test fails. Counts must be updated when adding new markers (never lowered). Also adds `tool/bash.ts`, `tool/skill.ts`, and `skill/skill.ts` to the `requiredMarkerFiles` list so their marker presence is enforced. Closes #321
1 parent bc81a6a commit d03d9d2

2 files changed

Lines changed: 67 additions & 7 deletions

File tree

packages/opencode/test/branding/upstream-merge-guard.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,9 @@ describe("altimate_change marker integrity", () => {
483483
"src/index.ts",
484484
"src/agent/agent.ts",
485485
"src/tool/registry.ts",
486+
"src/tool/bash.ts",
487+
"src/tool/skill.ts",
488+
"src/skill/skill.ts",
486489
"src/telemetry/index.ts",
487490
"src/global/index.ts",
488491
"src/util/token.ts",
@@ -514,4 +517,36 @@ describe("altimate_change marker integrity", () => {
514517

515518
expect(mismatched).toEqual([])
516519
})
520+
521+
// Minimum marker counts per file — prevents accidental marker removal.
522+
// When you ADD markers to a file, update the count here. If a count drops,
523+
// it means someone removed markers that protected custom code from upstream overwrites.
524+
const minimumMarkerCounts: Record<string, number> = {
525+
"src/session/compaction.ts": 2,
526+
"src/session/prompt.ts": 2,
527+
"src/installation/index.ts": 2,
528+
"src/flag/flag.ts": 2,
529+
"src/config/config.ts": 2,
530+
"src/config/paths.ts": 2,
531+
"src/index.ts": 2,
532+
"src/agent/agent.ts": 2,
533+
"src/tool/registry.ts": 2,
534+
"src/tool/bash.ts": 2,
535+
"src/tool/skill.ts": 20,
536+
"src/skill/skill.ts": 8,
537+
"src/telemetry/index.ts": 2,
538+
"src/global/index.ts": 2,
539+
"src/util/token.ts": 2,
540+
"src/storage/db.ts": 2,
541+
}
542+
543+
for (const [relPath, minCount] of Object.entries(minimumMarkerCounts)) {
544+
test(`${relPath} has at least ${minCount} altimate_change markers (regression guard)`, () => {
545+
const fullPath = join(pkgDir, relPath)
546+
expect(existsSync(fullPath)).toBe(true)
547+
const content = readText(fullPath)
548+
const actual = (content.match(/altimate_change/g) || []).length
549+
expect(actual).toBeGreaterThanOrEqual(minCount)
550+
})
551+
}
517552
})

script/upstream/analyze.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -631,20 +631,27 @@ function checkFileForMarkers(file: string, base?: string): MarkerWarning[] {
631631
let newCodeStart = 0
632632
let newCodeContext = ""
633633

634+
// Track marker additions and removals to detect net marker loss.
635+
// A removed marker line (prefixed with -) without a corresponding addition
636+
// means someone deleted a marker while keeping or modifying the code it protected.
637+
let markersAdded = 0
638+
let markersRemoved = 0
639+
let firstRemovedMarkerContext = ""
640+
634641
for (const line of lines) {
635-
const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)/)
642+
const hunkMatch = line.match(/^@@ -(\d+)(?:,\d+)? \+(\d+)/)
636643
if (hunkMatch) {
637-
currentLine = parseInt(hunkMatch[1]) - 1
644+
currentLine = parseInt(hunkMatch[2]) - 1
638645
continue
639646
}
640647

641648
if (line.startsWith("+") && !line.startsWith("+++")) {
642649
currentLine++
643650
const content = line.slice(1).trim()
644651

645-
if (content.includes("altimate_change start")) { inMarkerBlock = true; continue }
646-
if (content.includes("altimate_change end")) { inMarkerBlock = false; continue }
647-
if (content.includes("altimate_change")) continue
652+
if (content.includes("altimate_change start")) { inMarkerBlock = true; markersAdded++; continue }
653+
if (content.includes("altimate_change end")) { inMarkerBlock = false; markersAdded++; continue }
654+
if (content.includes("altimate_change")) { markersAdded++; continue }
648655

649656
if (!content) continue
650657
if (content.startsWith("//") && !content.includes("TODO")) continue
@@ -658,8 +665,15 @@ function checkFileForMarkers(file: string, base?: string): MarkerWarning[] {
658665
newCodeContext = content
659666
}
660667
}
661-
} else if (line.startsWith("-")) {
662-
// deleted line, don't increment
668+
} else if (line.startsWith("-") && !line.startsWith("---")) {
669+
// deleted line, don't increment currentLine
670+
const content = line.slice(1).trim()
671+
if (content.includes("altimate_change")) {
672+
markersRemoved++
673+
if (!firstRemovedMarkerContext) {
674+
firstRemovedMarkerContext = content.slice(0, 80)
675+
}
676+
}
663677
} else {
664678
currentLine++
665679
}
@@ -674,6 +688,17 @@ function checkFileForMarkers(file: string, base?: string): MarkerWarning[] {
674688
})
675689
}
676690

691+
// Detect net marker removal: more markers deleted than added means protection was stripped.
692+
if (markersRemoved > markersAdded) {
693+
const net = markersRemoved - markersAdded
694+
warnings.push({
695+
file,
696+
line: 0,
697+
context: firstRemovedMarkerContext,
698+
reason: `${net} altimate_change marker(s) removed — custom code may lose upstream merge protection`,
699+
})
700+
}
701+
677702
return warnings
678703
}
679704

0 commit comments

Comments
 (0)