Skip to content

repos install (converge) does not detect or repair scaffold content drift #6576

Description

@ggallen

What happens

repos status correctly detects scaffold content drift after #6559, but repos install (converge path) does not. The notify-scaffold-sync workflow runs repos install --direct --force on every push to main, yet converge reports "already current" for repos whose scaffold files have stale template content.

$ fullsend repos status -f repos.yaml
REPO                     REF   STATUS          DRIFT
fullsend-ai/agents       main  installed       .github/workflows/fullsend.yaml differs
fullsend-ai/experiments  main  installed       .github/workflows/fullsend.yaml differs
fullsend-ai/fullsend     main  installed       .github/workflows/fullsend.yaml differs
fullsend-ai/metrics      main  installed       .github/workflows/prioritize.yml differs

$ fullsend repos install -f repos.yaml --direct --force
✓ Install complete: 0 installed, 0 converged, 4 already current, 0 failed

What should happen

Both repos status and repos install should perform the same generalized drift check: compare the full desired state (what a fresh install would produce) against the actual state on the forge. The check should cover every component category — files, variables, secrets — and detect drift in both directions:

Drift type Example
Missing Template produces a file/var/secret that doesn't exist on the forge
Orphaned A file/var exists on the forge that the current template no longer produces (component removed from scaffold)
Content mismatch File exists but its content differs from the rendered template (after ref normalization)
Value mismatch Static variable exists but its value differs from expected
Presence-only Secret or dynamic variable exists — value can't or shouldn't be compared

This must be template-driven, not file-list-driven: the check iterates over whatever BuildScaffoldFiles returns and whatever installVarsForForge/installSecretsForForge return, not a hardcoded list. When a component is added to or removed from the scaffold in a future release, drift detection should automatically cover it without code changes to the probe layer.

Both paths should share a single implementation so they can't diverge.

Variable classification: static vs dynamic

Variables fall into two categories that require different drift checks:

Category Check Examples
Static — value determined at install time, not mutated at runtime Presence + value match FULLSEND_MINT_URL, FULLSEND_GCP_REGION, FULLSEND_REVIEW_CLIENT_ID, guard var
Dynamic — mutated by agents or schedulers at runtime Presence only FULLSEND_LAST_POLL_AT_FAST, FULLSEND_LAST_POLL_AT_FULL, FULLSEND_LABEL_STATE

Dynamic variables must never have their values compared — poll timestamps change on every run and would always show as drifted. ProbeComponents already supports this distinction via the expectedVarValues parameter: only variables included in that map get value-checked. But today only FULLSEND_MINT_URL is passed — other static variables like FULLSEND_GCP_REGION and FULLSEND_REVIEW_CLIENT_ID are presence-checked only, so manual changes to them go undetected.

The classification must travel with the variable definition so it works automatically for future variables. Today installVarsForForge returns map[string]string — callers have no way to distinguish static from dynamic without maintaining a separate hardcoded list. Change installVarsForForge to return a structured type that carries the classification:

type ManagedVar struct {
    Name    string
    Value   string
    Dynamic bool // true = presence-only (value mutated at runtime)
}

Each variable is defined once with its Dynamic flag. The shared drift function reads the flag to decide whether to compare values — no second list, no heuristic, no risk of forgetting to exclude a new dynamic variable. Adding a variable means adding one ManagedVar entry; drift detection picks it up automatically.

Current gaps

Converge path (the main bug)

  • ProbeComponents (probe.go:92) sets Match = true for workflow and thin-caller components based on presence only — content is not compared.
  • convergeRefFiles only writes files when replaceShimRef changes the ref string — template structure changes with the same ref are invisible.
  • convergeScaffoldFiles only fires when !c.Present — stale-but-present files are skipped.
  • GitLab auxiliary templates (agent, poll) are only updated when the ref changes. Content changes with the same ref are invisible.
  • No code path detects or repairs orphaned components (files from a previous scaffold version that the current template no longer produces).
  • Only FULLSEND_MINT_URL is value-checked; other static variables (FULLSEND_GCP_REGION, FULLSEND_REVIEW_CLIENT_ID) are presence-only.

Status path (secondary bugs)

  • RunnerTags always empty: ExpectedScaffoldContent renders with empty tags. GitLab repos with manifest-configured runner tags get false content drift.
  • No PrebuiltScaffoldFiles: When fullsend_ref pins a version different from the running binary, status compares against embedded templates instead of the pinned version's — wrong baseline.
  • VendorBinary always false: Vendor-mode repos produce different scaffold files. Status compares against non-vendored templates.
  • Version marker not normalized: FormatVersionMarker embeds a # fullsend-ref: comment in the GitLab dispatch file. replaceShimRef normalizes ref: YAML keys but not comment lines, so ref-format differences in the marker survive normalization — false content drift.
  • No orphan detection: Status doesn't flag files that exist on the forge but are no longer part of the scaffold.
  • Same static variable gap as converge — only FULLSEND_MINT_URL is value-checked.

Shared design gap

#6557 explicitly required: "The two paths should share this code." #6559 built a separate checkScaffoldContentDrift + ExpectedScaffoldContent used only by status, leaving converge untouched. Each path has different bugs that would be caught if they shared logic.

Desired approach

The converge path has the right inputs (full InstallConfig with RunnerTags, PrebuiltScaffoldFiles, resolveTargetRef). The status path has the right comparison logic (render expected, fetch installed, normalize refs, bytes.Equal). Neither is complete on its own.

One shared function should:

  1. Render the full desired state (files, variables, secrets) using the same inputs converge uses
  2. Fetch the actual state from the forge
  3. Compare each component, normalizing refs before file comparison
  4. Classify variables as static (value-check) or dynamic (presence-only)
  5. Return a list of drifted components with their drift type (missing, orphaned, content, value)

Status maps results to Drift entries. Converge maps results to repair actions. Both call the same function.

Context

Related: #6557, #6559, #6553, #6555

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugcomponent/installCLI install and app setuppriority/highSignificant impact, address soonready-to-codeTriaged and ready for the code agenttype/bugConfirmed defect in existing behavior

    Type

    No type

    Projects

    Status
    In progress

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions