config: shadow-value check, sources/version presence tracking, additive install --generate

[NikashPrakash]

Follow-up batch to #535. Four items, all in the same hazard class the manifest-corruption fix opened up: a value that is present in a repo manifest wins the layer merge whether or not anyone meant to write it.

1. Shadow-value check — surfaced through da config verify

Repos that were hit by the injection bug (or hand-edited) are left carrying explicit repo-local values identical to what the layer stack would supply. Nothing in the resolved output distinguishes a deliberate override from a leftover: the repo-local key simply wins, silently, forever.

Surface chosen: da config verify, not da doctor. Both exist. doctor is a fixed sequence of report* functions printing untyped prose to stdout with no JSON mode; config verify already has the exact shape this needs — a []VerifyCheck{Name, Status, Detail} report, a stable --json contract, a per-check pass|warn|fail status, and an established "one verify_<check>.go + verify_<check>_test.go per check" convention. It is also the command whose remit is offline manifest contract checks, which is precisely what this is.

For each repo-local scalar key the stack below also supplies:

	status	meaning
REDUNDANT	warn	repo value == effective layer value. Harmless today, but it silently pins the repo against a future org-layer change — the failure mode where an org flips a default and one repo mysteriously does not follow. Actionable: delete the key.
OVERRIDE	pass	repo value != layer value. The feature working as designed, so it does not warn — but both values and the losing layer are named, because an operator auditing an org rollout needs to see which repos flipped a layer value and to what.

Nothing here can fail the report.

Candidate keys come from the resolver's own category table through a new exported FieldMergeCategory, so the check cannot drift from the merge semantics it reasons about. Only CategoryScalar keys qualify — set-union, map-merge and ordered-replace keys combine rather than shadow, and sources is repo-local by design. version and $schema are exempt as manifest structure (version is schema-required, so restating it is mandatory, not redundant), as are the repo-local-only protected fields (repo_id, project).

"What the stack would supply" is computed by re-asking Snapshot.FieldAt against the layer slice with the repo-local layer removed — reusing the resolver's own merge rather than reimplementing it. Only repo-local is dropped: the .agentsrc.local.json overlay is uncommitted but still outranks every imported layer, so it genuinely is what would take effect.

E2E against a copy of this repo's own manifest ("settings": false beside an extends; the live manifest is untouched), with the org layer pointed at a local fixture:

  [ok ] shadow:gitignore_projections OVERRIDE — repo-local gitignore_projections=false replaces gitignore_projections=true from orgsrc:base.json
  [ok ] shadow:hooks                 OVERRIDE — repo-local hooks=true replaces hooks=false from orgsrc:base.json
  [warn] shadow:mcp                   REDUNDANT — repo-local mcp=["code-review-graph","sonarqube"] restates what orgsrc:base.json already supplies; remove the key to defer to the layer stack
  [warn] shadow:settings              REDUNDANT — repo-local settings=false restates what orgsrc:base.json already supplies; remove the key to defer to the layer stack

Summary: 6 passed, 3 warning(s), 0 failed — OK

2. sources presence-tracking hazard

sources had no omitempty, and LoadAgentsRC synthesizes a default local source when the key is absent — so that synthesized value serialized back on the next save, writing a declaration the author never made. sources merges as an ordered replace, so an injected local-only list wholly replaces the source set an org layer supplies, exactly the way the injected false beat an org layer's true. Reachable from every manifest write: skills promote, agents import/new, install --generate.

Presence-tracking mechanism. A slice cannot carry presence in its own value the way a pointer can: omitempty treats nil and [] alike, and the whole point of the load-time default is that Sources is never empty by the time anything can save it — so the pointer/nil-slice discipline that fixed hooks/mcp/settings does not transfer directly. Presence is therefore tracked out-of-band on an unexported sourcesSynthesized field, deliberately oriented so its zero value means "declared":

• false (the default) ⇒ serialize Sources normally. Every AgentsRC built anywhere else — struct literals in other packages, a plain json.Unmarshal, GenerateAgentsRC — behaves exactly as before. There is no invisible-flag hazard where a manifest built outside this package silently loses its sources.
• Only LoadAgentsRC's own synthesis sets it to true, and MarshalJSON then nils the mirror field so omitempty drops the key.
• Suppression is additionally value-guarded: it applies only while Sources still holds exactly what was synthesized. A caller that appends a git source (agents import) or replaces the list has made a real declaration, and it serializes despite the flag. No real edit can be swallowed by a stale flag.
• Unexported on purpose — nothing outside the package should be able to mark an author's declaration as synthetic.

Full field lifecycle applied per schema-usage.md: struct, agentsRCCore mirror, MarshalJSON, and schemas/agentsrc.schema.json (sources documented as optional-and-kept-absent; it was already not in required, and agentsRCKnown already listed it).

Verified through a real da skills promote on a fixture manifest that omits both keys — the save adds the skill and leaves sources and version absent.

3. Version re-emitting 0

Only affects already-invalid files, but re-emitting "version": 0 writes a value the schema's enum: [1,2] rejects. Plain omitempty on the int rather than a pointer: 0 is outside the enum, so zero can only mean absent — presence is encoded losslessly without a pointer.

4. MergeGenerateAgentsRC overwrite contract

Decision: changed, but scoped — and the load-bearing half of the old contract is preserved. The blanket "generated values fill absent fields only" would have broken a real flow, so the change splits along the line the pointer types made visible:

• hooks/mcp/settings — changed. These are scan-detectable but author-declarable. A successful scan replacing an explicit "hooks": false silently re-enables a projection the repo deliberately turned off. Since config: fix silent manifest corruption — refresh rewrote .agentsrc.json and disabled org-layer hooks/mcp/settings #535 made them pointers, "the author wrote false" is finally distinguishable from "the key is absent", so the scan now fills an absent declaration only. --force-generate opts back into replacing.
• skills/agents/rules — unchanged, and this is the conflict worth reporting. These are scan-derived sets with no other convergence path. Nothing else prunes a manifest entry for a resource deleted under ~/.agents/; da refresh deliberately refuses to touch the manifest. Replacing them is the documented cleanup for a stale or over-captured manifest, pinned by an explicit regression test (TestInstallGenerateOverStaleManifestDropsGlobalOverDeclarations, whose comment states "with no separate prune step needed"). Making these fill-absent-only would silently delete that cleanup path, so they still replace in both modes.

The base also flips from generated to existing, which is the larger structural fix. Under out := *generated, every field the generator does not produce was dropped on each --generate unless someone had added a bespoke preservation clause — and clauses were in fact added one incident at a time (project, repo_id, ExtraFields, observability, stage_profiles, manifests), each after a field went missing in the field. Destructive-by-default with a hand-maintained exception list is not a contract anyone can hold: every new typed field silently joined the casualty list. Basing on existing makes extends, kg, features, packages, execution_profile, pr_source, precondition_policies, locks and every future field safe without anyone having to notice.

That also fixes a latent bug found on the way: work_tracking is set by the generator on any git repo and had no preservation clause, so a committed non-default backend was reset by every --generate — despite the generator's own comment claiming "existing manifests are untouched".

Absent scalars still fill from the scan (project, repo_id, $schema, version, work_tracking), so the v1-manifest bootstrap keeps working, and repo_id stays protected per org-config-resolution §7.4.

Contract docs updated where documented: the install cobra Long + Example, da explain's flag list, and docs/COMMANDS.md (both the install and config verify rows).

Verification

go test ./internal/config/... ./commands/...          # pass
go test ./... -skip TestConfigLoadSave               # pass

New-file coverage — commands/config/verify_shadow.go is at 100% on every function (the 100% ratchet for new files):

verifyLayerShadows      100.0%
layersBelowRepoLocal    100.0%
shadowCandidateKeys     100.0%
shadowCheck             100.0%
shadowValuesEqual       100.0%
renderShadowValue       100.0%

Tests are table-driven and same-package, with thin t.Run closures — seeding and assertion bodies are extracted into named helpers (mergeOverStaleManifest, assertStaleScanSetsPruned, assertSOBNames, shadowSnapshot, assertCheckNames/Statuses/Details, roundTripWithMutation) to stay under the S3776 cognitive-complexity gate.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
1.3% Duplication on New Code

See analysis details on SonarQube Cloud