Skip to content

Commit 6195e24

Browse files
anandgupta42claude
andcommitted
feat: add upstream_fix: marker convention with --audit-fixes command
Introduces a strategy for handling bug fixes to upstream code: **`upstream_fix:` convention:** - Tag in marker description distinguishes temporary bug fixes from permanent feature additions - Before each upstream merge, run `--audit-fixes` to review which fixes we're carrying and whether upstream has shipped their own **`--audit-fixes` flag on analyze.ts:** - Lists all `upstream_fix:` markers with file locations and descriptions - Prints merge review checklist **Retagged 3 existing upstream bug fixes:** - `locale.ts` — days/hours duration swap - `command/index.ts` — placeholder lexicographic sort - `home.tsx` — beginner UI race condition **Code review fixes from #546:** - Guard `--downstream` without `--model` (returns error instead of silently running full project build) - Remove unused `condition` field from `Suggestion` interface - Add test for `--downstream` error case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 384335d commit 6195e24

9 files changed

Lines changed: 160 additions & 32 deletions

File tree

packages/dbt-tools/src/commands/build.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ import type { DBTProjectIntegrationAdapter, CommandProcessResult } from "@altima
22

33
export async function build(adapter: DBTProjectIntegrationAdapter, args: string[]) {
44
const model = flag(args, "model")
5-
if (!model) return project(adapter)
65
const downstream = args.includes("--downstream")
6+
if (!model) {
7+
if (downstream) return { error: "--downstream requires --model" }
8+
return project(adapter)
9+
}
710
const result = await adapter.unsafeBuildModelImmediately({
811
plusOperatorLeft: "",
912
modelName: model,

packages/dbt-tools/test/build.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ describe("build command", () => {
4545
})
4646
})
4747

48+
test("build --downstream without --model returns error", async () => {
49+
const adapter = makeAdapter()
50+
const result = await build(adapter, ["--downstream"])
51+
expect(result).toEqual({ error: "--downstream requires --model" })
52+
expect(adapter.unsafeBuildProjectImmediately).not.toHaveBeenCalled()
53+
expect(adapter.unsafeBuildModelImmediately).not.toHaveBeenCalled()
54+
})
55+
4856
test("build surfaces stderr as error", async () => {
4957
const adapter = makeAdapter({
5058
unsafeBuildProjectImmediately: mock(() =>

packages/opencode/src/cli/cmd/tui/routes/home.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export function Home() {
3838
return Object.values(sync.data.mcp).filter((x) => x.status === "connected").length
3939
})
4040

41-
// altimate_change start — fix race condition: don't show beginner UI until sessions loaded
41+
// altimate_change start — upstream_fix: race condition shows beginner UI flash before sessions loaded
4242
const isFirstTimeUser = createMemo(() => {
4343
// Don't evaluate until sessions have actually loaded (avoid flash of beginner UI)
4444
// Return undefined to represent "loading" state

packages/opencode/src/command/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ export namespace Command {
5454
const result: string[] = []
5555
const numbered = template.match(/\$\d+/g)
5656
if (numbered) {
57-
// altimate_change start — fix lexicographic sort of multi-digit placeholders ($10 before $2)
58-
for (const match of [...new Set(numbered)].sort((a, b) => parseInt(a.slice(1), 10) - parseInt(b.slice(1), 10))) result.push(match)
57+
// altimate_change start — upstream_fix: lexicographic sort of multi-digit placeholders ($10 sorted before $2)
58+
for (const match of [...new Set(numbered)].sort((a, b) => parseInt(a.slice(1), 10) - parseInt(b.slice(1), 10)))
59+
result.push(match)
5960
// altimate_change end
6061
}
6162
if (template.includes("$ARGUMENTS")) result.push("$ARGUMENTS")

packages/opencode/src/skill/followups.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ export namespace SkillFollowups {
44
skill: string // skill name to suggest
55
label: string // short display label
66
description: string // why this is a good next step
7-
condition?: string // optional: when this suggestion applies
87
}
98

109
// Map from skill name to follow-up suggestions
@@ -151,7 +150,8 @@ export namespace SkillFollowups {
151150
}
152151

153152
// A special warehouse nudge for users who haven't connected yet
154-
const WAREHOUSE_NUDGE = "**Tip:** Connect a warehouse to validate against real data. Run `/discover` to auto-detect your connections."
153+
const WAREHOUSE_NUDGE =
154+
"**Tip:** Connect a warehouse to validate against real data. Run `/discover` to auto-detect your connections."
155155

156156
export function get(skillName: string): readonly Suggestion[] {
157157
return Object.freeze(FOLLOWUPS[skillName] ?? [])

packages/opencode/src/util/locale.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@ export namespace Locale {
5454
const minutes = Math.floor((input % 3600000) / 60000)
5555
return `${hours}h ${minutes}m`
5656
}
57+
// altimate_change start — upstream_fix: days/hours calculation were swapped (hours used total, not remainder)
5758
const days = Math.floor(input / 86400000)
5859
const hours = Math.floor((input % 86400000) / 3600000)
60+
// altimate_change end
5961
return `${days}d ${hours}h`
6062
}
6163

script/upstream/README.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,44 @@ When we modify upstream files (not fully custom ones), we wrap our changes with
222222

223223
These help during conflict resolution — you can see exactly what we changed vs upstream code. The `analyze.ts` script audits for unclosed marker blocks.
224224

225+
### Upstream Bug Fixes (`upstream_fix:` tag)
226+
227+
When fixing a **bug in upstream code** (not adding a feature), use the `upstream_fix:` tag in the marker description:
228+
229+
```typescript
230+
// altimate_change start — upstream_fix: days/hours calculation were swapped
231+
const days = Math.floor(input / 86400000)
232+
const hours = Math.floor((input % 86400000) / 3600000)
233+
// altimate_change end
234+
```
235+
236+
**Why this matters:** Regular `altimate_change` markers protect features we added — they're permanent. But upstream bug fixes are **temporary**: once upstream ships their own fix, we should drop our marker and accept theirs.
237+
238+
Without the `upstream_fix:` tag:
239+
- If upstream fixes the same bug, the merge creates a conflict (good — forces review)
240+
- But the reviewer doesn't know our change was a bug fix vs a feature, so they may keep both
241+
242+
With the `upstream_fix:` tag:
243+
- Before each merge, run `--audit-fixes` to see all bug fixes we're carrying
244+
- During conflict resolution, reviewers know to check "did upstream fix this?" and can safely drop our version
245+
- After merge, any remaining `upstream_fix:` markers represent bugs upstream hasn't fixed yet
246+
247+
**When to use which:**
248+
249+
| Scenario | Marker |
250+
|----------|--------|
251+
| New feature/custom code | `// altimate_change start — description` |
252+
| Fix bug in upstream code | `// altimate_change start — upstream_fix: description` |
253+
| Branding change | No marker (handled by branding transforms) |
254+
| Code in `keepOurs` files | No marker needed |
255+
256+
**Audit before merging:**
257+
258+
```bash
259+
# List all upstream bug fixes we're carrying
260+
bun run script/upstream/analyze.ts --audit-fixes
261+
```
262+
225263
## File Organization
226264

227265
```

script/upstream/analyze.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,18 @@ describe("parseDiffForMarkerWarnings", () => {
246246
expect(warnings[0].context).toContain("first")
247247
})
248248

249+
test("upstream_fix: tagged markers are recognized as valid markers", () => {
250+
const diff = makeDiff(
251+
`@@ -50,4 +50,6 @@
252+
const existing = true
253+
+// altimate_change start — upstream_fix: days/hours were swapped
254+
+const days = Math.floor(input / 86400000)
255+
+// altimate_change end
256+
const more = true`,
257+
)
258+
expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([])
259+
})
260+
249261
test("real-world scenario: upgrade indicator in footer.tsx", () => {
250262
// Simulates the exact diff that leaked: UpgradeIndicator added to
251263
// session footer without markers, adjacent to existing yolo marker block.

script/upstream/analyze.ts

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const { values: args } = parseArgs({
3535
version: { type: "string", short: "v" },
3636
branding: { type: "boolean", default: false },
3737
markers: { type: "boolean", default: false },
38+
"audit-fixes": { type: "boolean", default: false },
3839
strict: { type: "boolean", default: false },
3940
base: { type: "string" },
4041
verbose: { type: "boolean", default: false },
@@ -213,9 +214,7 @@ function printBrandingReport(report: BrandingReport, verbose: boolean): void {
213214
const maxLeaksToShow = verbose ? leaks.length : 5
214215
for (let i = 0; i < Math.min(leaks.length, maxLeaksToShow); i++) {
215216
const leak = leaks[i]
216-
const truncated = leak.content.length > 80
217-
? leak.content.slice(0, 77) + "..."
218-
: leak.content
217+
const truncated = leak.content.length > 80 ? leak.content.slice(0, 77) + "..." : leak.content
219218
console.log(` ${DIM}L${String(leak.line).padStart(4)}${RESET} ${YELLOW}${leak.pattern}${RESET}`)
220219
console.log(` ${DIM}${truncated}${RESET}`)
221220
}
@@ -298,14 +297,20 @@ async function analyzeVersion(version: string, config: MergeConfig): Promise<Ver
298297
// Check for marker files and potential conflicts
299298
for (const file of analysis.categories.transformable) {
300299
try {
301-
const content = await $`git show HEAD:${file}`.cwd(root).text().catch(() => "")
300+
const content = await $`git show HEAD:${file}`
301+
.cwd(root)
302+
.text()
303+
.catch(() => "")
302304

303305
if (content.includes(config.changeMarker)) {
304306
analysis.markerFiles.push(file)
305307
}
306308

307309
// Check if we've modified this file (potential conflict)
308-
const ourDiff = await $`git diff HEAD -- ${file}`.cwd(root).text().catch(() => "")
310+
const ourDiff = await $`git diff HEAD -- ${file}`
311+
.cwd(root)
312+
.text()
313+
.catch(() => "")
309314
if (ourDiff.trim().length > 0) {
310315
analysis.potentialConflicts.push(file)
311316
}
@@ -360,10 +365,16 @@ function printVersionAnalysis(analysis: VersionAnalysis): void {
360365
const line = "─".repeat(50)
361366
console.log(` ${line}`)
362367
console.log(` ${bold("Merge estimate:")}`)
363-
console.log(` Auto-resolvable: ${GREEN}${categories.keepOurs.length + categories.skipFiles.length + categories.lockFiles.length}${RESET}`)
368+
console.log(
369+
` Auto-resolvable: ${GREEN}${categories.keepOurs.length + categories.skipFiles.length + categories.lockFiles.length}${RESET}`,
370+
)
364371
console.log(` Need transform: ${categories.transformable.length}`)
365-
console.log(` Likely conflicts: ${analysis.potentialConflicts.length > 0 ? RED : GREEN}${analysis.potentialConflicts.length}${RESET}`)
366-
console.log(` Marker files: ${analysis.markerFiles.length > 0 ? YELLOW : GREEN}${analysis.markerFiles.length}${RESET}`)
372+
console.log(
373+
` Likely conflicts: ${analysis.potentialConflicts.length > 0 ? RED : GREEN}${analysis.potentialConflicts.length}${RESET}`,
374+
)
375+
console.log(
376+
` Marker files: ${analysis.markerFiles.length > 0 ? YELLOW : GREEN}${analysis.markerFiles.length}${RESET}`,
377+
)
367378
console.log()
368379
}
369380

@@ -446,7 +457,9 @@ function printMarkerAnalysis(config: MergeConfig): void {
446457
const complete = markers.filter((m) => m.endLine !== null)
447458
const incomplete = markers.filter((m) => m.endLine === null)
448459

449-
console.log(` Found ${bold(String(markers.length))} marker blocks in ${new Set(markers.map((m) => m.file)).size} files`)
460+
console.log(
461+
` Found ${bold(String(markers.length))} marker blocks in ${new Set(markers.map((m) => m.file)).size} files`,
462+
)
450463
console.log(` ${GREEN}Complete (start + end):${RESET} ${complete.length}`)
451464

452465
if (incomplete.length > 0) {
@@ -468,10 +481,50 @@ function printMarkerAnalysis(config: MergeConfig): void {
468481

469482
// Summary
470483
console.log()
471-
console.log(` ${bold("Integrity:")} ${incomplete.length === 0
472-
? `${GREEN}All blocks properly closed${RESET}`
473-
: `${RED}${incomplete.length} unclosed block(s)${RESET}`
474-
}`)
484+
console.log(
485+
` ${bold("Integrity:")} ${
486+
incomplete.length === 0
487+
? `${GREEN}All blocks properly closed${RESET}`
488+
: `${RED}${incomplete.length} unclosed block(s)${RESET}`
489+
}`,
490+
)
491+
}
492+
493+
// ---------------------------------------------------------------------------
494+
// Upstream fix audit (--audit-fixes)
495+
// ---------------------------------------------------------------------------
496+
497+
function auditUpstreamFixes(config: MergeConfig): void {
498+
const markers = findMarkers(config)
499+
const fixes = markers.filter((m) => m.startComment.includes("upstream_fix:"))
500+
501+
console.log()
502+
console.log(bold("=== Upstream Bug Fixes We're Carrying ==="))
503+
console.log()
504+
505+
if (fixes.length === 0) {
506+
console.log(` ${GREEN}No upstream_fix: markers found.${RESET}`)
507+
console.log(` All our markers are feature additions, not bug fixes.`)
508+
console.log()
509+
return
510+
}
511+
512+
console.log(` Found ${bold(String(fixes.length))} upstream bug fix(es) to review before merge:\n`)
513+
514+
for (const fix of fixes) {
515+
// Extract description after "upstream_fix:"
516+
const desc = fix.startComment.replace(/.*upstream_fix:\s*/, "").replace(/\s*\*\/\s*$/, "")
517+
const lines = fix.endLine ? `${fix.line}-${fix.endLine}` : `${fix.line}`
518+
console.log(` ${YELLOW}fix${RESET} ${fix.file}:${lines}`)
519+
console.log(` ${desc}`)
520+
console.log()
521+
}
522+
523+
console.log(` ${bold("Before each upstream merge:")}`)
524+
console.log(` 1. Check if upstream fixed each issue in their release`)
525+
console.log(` 2. If fixed upstream: accept their version, remove our marker`)
526+
console.log(` 3. If not fixed: keep our marker (it will survive the merge)`)
527+
console.log()
475528
}
476529

477530
// ---------------------------------------------------------------------------
@@ -490,6 +543,7 @@ function printUsage(): void {
490543
--version, -v <tag> Upstream version to analyze
491544
--branding Scan codebase for upstream branding leaks
492545
--markers Check changed files for missing altimate_change markers
546+
--audit-fixes List all upstream_fix: markers (bug fixes we made to upstream code)
493547
--base <branch> Base branch for --markers comparison (default: HEAD)
494548
--strict Exit with code 1 on warnings (for CI)
495549
--verbose Show all results (not just top 20)
@@ -509,6 +563,9 @@ function printUsage(): void {
509563
${dim("# Check PR for missing markers (CI)")}
510564
bun run script/upstream/analyze.ts --markers --base main --strict
511565
566+
${dim("# List upstream bug fixes we're carrying (review before merge)")}
567+
bun run script/upstream/analyze.ts --audit-fixes
568+
512569
${dim("# Machine-readable output for CI")}
513570
bun run script/upstream/analyze.ts --branding --json
514571
`)
@@ -530,14 +587,9 @@ function getChangedFiles(base?: string): string[] {
530587
const root = repoRoot()
531588
// Only check Modified files (M), not Added (A). New files don't exist
532589
// upstream so they can't be overwritten by a merge — no markers needed.
533-
const cmd = base
534-
? `git diff --name-only --diff-filter=M ${base}...HEAD`
535-
: `git diff --name-only --diff-filter=M HEAD`
590+
const cmd = base ? `git diff --name-only --diff-filter=M ${base}...HEAD` : `git diff --name-only --diff-filter=M HEAD`
536591
try {
537-
return execSync(cmd, { cwd: root, encoding: "utf-8" })
538-
.trim()
539-
.split("\n")
540-
.filter(Boolean)
592+
return execSync(cmd, { cwd: root, encoding: "utf-8" }).trim().split("\n").filter(Boolean)
541593
} catch {
542594
return []
543595
}
@@ -646,8 +698,14 @@ export function parseDiffForMarkerWarnings(file: string, diffOutput: string): Ma
646698
currentLine++
647699
const content = line.slice(1).trim()
648700

649-
if (content.includes("altimate_change start")) { inMarkerBlock = true; continue }
650-
if (content.includes("altimate_change end")) { inMarkerBlock = false; continue }
701+
if (content.includes("altimate_change start")) {
702+
inMarkerBlock = true
703+
continue
704+
}
705+
if (content.includes("altimate_change end")) {
706+
inMarkerBlock = false
707+
continue
708+
}
651709
if (content.includes("altimate_change")) continue
652710

653711
// Only flag added lines as violations — context lines are pre-existing
@@ -686,9 +744,7 @@ function checkFileForMarkers(file: string, base?: string): MarkerWarning[] {
686744
const { execSync } = require("child_process")
687745
const root = repoRoot()
688746

689-
const diffCmd = base
690-
? `git diff -U5 ${base}...HEAD -- "${file}"`
691-
: `git diff -U5 HEAD -- "${file}"`
747+
const diffCmd = base ? `git diff -U5 ${base}...HEAD -- "${file}"` : `git diff -U5 HEAD -- "${file}"`
692748

693749
let diffOutput: string
694750
try {
@@ -770,18 +826,26 @@ async function main(): Promise<void> {
770826
const hasVersion = Boolean(args.version)
771827
const hasBranding = Boolean(args.branding)
772828
const hasMarkers = Boolean(args.markers)
829+
const hasAuditFixes = Boolean(args["audit-fixes"])
773830

774-
if (!hasVersion && !hasBranding && !hasMarkers) {
831+
if (!hasVersion && !hasBranding && !hasMarkers && !hasAuditFixes) {
775832
// Default: run marker analysis
776833
printMarkerAnalysis(config)
777834

778835
console.log()
779836
logger.info("Use --version <tag> to analyze an upstream version")
780837
logger.info("Use --branding to audit for branding leaks")
781838
logger.info("Use --markers --base main to check for missing markers")
839+
logger.info("Use --audit-fixes to list upstream bug fixes we're carrying")
782840
return
783841
}
784842

843+
// ─── Upstream fix audit ──────────────────────────────────────────────────
844+
if (hasAuditFixes) {
845+
auditUpstreamFixes(config)
846+
if (!hasVersion && !hasBranding && !hasMarkers) return
847+
}
848+
785849
// ─── Version analysis ──────────────────────────────────────────────────────
786850

787851
if (hasVersion) {

0 commit comments

Comments
 (0)