Skip to content

Commit 6766457

Browse files
author
Haider
committed
fix: session env reads Layer-1 attrs from latest project_scan span
Addresses GPT consensus-review finding M2 on PR #938. `annotateSession` was deriving `de.env.*` session attributes by regex- parsing the `project_scan` tool's output text via `detectEnvFromProjectScan()`. Two architectural problems: 1. Layer-1 violation: `project-scan.ts:935` already writes the authoritative `de.env.dbt_present`, `de.env.dbt_manifest_present`, `de.env.warehouse_type`, `de.env.tools_detected` onto the tool span's attributes (via the tracer's metadata-channel lift). The session rollup ignored that authoritative source and re-parsed the formatted output text, which is fragile to format drift and can disagree with the per-span Layer-1 values. This violated the PR's documented merge rule: Layer 1 (caller/tool-provided) wins over Layer 2 (derived). 2. Stale-state bug: used `find()` to pick the first project_scan span. A session that ran project_scan twice — e.g., scan, configure a new warehouse, scan again — reflected the first scan's state in the session root, not the latest. Fix: - Pick the LAST project_scan span in the toolSpans array (insertion order = wall-clock order, since spans are pushed in logToolCall). - For each `de.env.*` key, read the span's Layer-1 attribute first. Fall back to the text-parse only when the Layer-1 value is absent. - Text-parse fallback is computed lazily (only runs if at least one key needs the fallback) so the common case where all four keys hit Layer 1 skips the regex work entirely. Verified with 3 cases: Layer-1 wins over conflicting text, text fallback fills in absent attrs, latest of 2 scans wins.
1 parent 3cda78f commit 6766457

1 file changed

Lines changed: 43 additions & 8 deletions

File tree

packages/opencode/src/altimate/observability/annotator.ts

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -501,14 +501,49 @@ export function annotateSession(trace: TraceFile): Record<string, unknown> {
501501
if (filesRead.size > 0) out[DE.ARTIFACTS.FILES_READ] = [...filesRead].slice(0, 100)
502502
if (filesEdited.size > 0) out[DE.ARTIFACTS.FILES_EDITED] = [...filesEdited].slice(0, 100)
503503

504-
// Environment capabilities (deterministic if project_scan ran — parse its output text)
505-
const projectScanSpan = toolSpans.find((s) => s.name === "project_scan")
506-
if (projectScanSpan) {
507-
const env = detectEnvFromProjectScan(projectScanSpan)
508-
if (env.dbtPresent != null) out[DE.ENV.DBT_PRESENT] = env.dbtPresent
509-
if (env.manifestPresent != null) out[DE.ENV.DBT_MANIFEST_PRESENT] = env.manifestPresent
510-
if (env.warehouseType) out[DE.ENV.WAREHOUSE_TYPE] = env.warehouseType
511-
if (env.toolsDetected.length > 0) out[DE.ENV.TOOLS_DETECTED] = env.toolsDetected
504+
// Environment capabilities. Prefer the project_scan tool's Layer-1
505+
// `de.env.*` attributes (authoritative, set in project-scan.ts:935) over
506+
// re-parsing its output text. Falls back to text parsing only when the
507+
// Layer-1 attribute is absent.
508+
//
509+
// Picks the LATEST project_scan span (last one in the array — spans are
510+
// pushed in insertion order in logToolCall) so that a session running
511+
// project_scan twice (e.g., after adding a warehouse) reflects the
512+
// current state, not the initial one. (GPT PR-938 consensus review M2.)
513+
const projectScanSpans = toolSpans.filter((s) => s.name === "project_scan")
514+
const latestScan = projectScanSpans[projectScanSpans.length - 1]
515+
if (latestScan) {
516+
const attrs = latestScan.attributes ?? {}
517+
// Text-parse fallback computed lazily — only used per key when Layer-1
518+
// is absent, so we still skip the regex work when all 4 keys hit Layer 1.
519+
let fallback: ScanEnv | undefined
520+
const parsed = (): ScanEnv => {
521+
if (!fallback) fallback = detectEnvFromProjectScan(latestScan)
522+
return fallback
523+
}
524+
const pickBool = (key: string, fb: () => boolean | undefined): boolean | undefined => {
525+
const v = attrs[key]
526+
if (typeof v === "boolean") return v
527+
return fb()
528+
}
529+
const pickString = (key: string, fb: () => string | undefined): string | undefined => {
530+
const v = attrs[key]
531+
if (typeof v === "string" && v) return v
532+
return fb()
533+
}
534+
const pickStringArray = (key: string, fb: () => string[]): string[] => {
535+
const v = attrs[key]
536+
if (Array.isArray(v) && v.every((x) => typeof x === "string")) return v as string[]
537+
return fb()
538+
}
539+
const dbtPresent = pickBool(DE.ENV.DBT_PRESENT, () => parsed().dbtPresent)
540+
const manifestPresent = pickBool(DE.ENV.DBT_MANIFEST_PRESENT, () => parsed().manifestPresent)
541+
const warehouseType = pickString(DE.ENV.WAREHOUSE_TYPE, () => parsed().warehouseType)
542+
const toolsDetected = pickStringArray(DE.ENV.TOOLS_DETECTED, () => parsed().toolsDetected)
543+
if (dbtPresent !== undefined) out[DE.ENV.DBT_PRESENT] = dbtPresent
544+
if (manifestPresent !== undefined) out[DE.ENV.DBT_MANIFEST_PRESENT] = manifestPresent
545+
if (warehouseType) out[DE.ENV.WAREHOUSE_TYPE] = warehouseType
546+
if (toolsDetected.length > 0) out[DE.ENV.TOOLS_DETECTED] = toolsDetected
512547
}
513548

514549
// Outcome.executed: was a dbt-family DML command (build/run/test/seed/

0 commit comments

Comments
 (0)