fix(templates): ship the canonical .gitignore in all 14 visible bundled templates (#1908) - #1936
Conversation
Rule #1 — requirements before implementation. Trinity grades every agent against docs/agent-validation-spec.md but shipped no gate on the templates it ships itself: all 14 visible bundled templates failed the same four HARD security checks at birth (S-001 .env, S-002 .mcp.json, S-004 .claude/projects/, S-005 .trinity/). Adds core-agent.md §4.1.1 stating the contract, the canonical-list provenance, the G-001 trap (no wholesale .claude/ exclusion), the second-order effect on this repo's own view of config/agent-templates/, the .trinity/* escape hatch for a future template shipping committed hooks, and two honest gaps: hiding a template does not stop it birthing findings (the resolver never reads `hidden`), and T-004/T-005 remain for the three starters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ed templates (#1908) An agent created from `sage`, `scout` or `scribe` was born with four HARD compatibility findings and no user changes: the templates shipped no .gitignore at all. The 11 visible `dd-*` templates shipped one, but its entire content was `outputs/` + `*.log` — covering none of `.env`, `.mcp.json`, `.claude/projects/` or `.trinity/`. So 14 of 14 visible templates failed the same four checks, and hiding a template does not fix it: the resolver never reads `hidden`, so a hidden template stays creatable by id and births the same findings. `_GITIGNORE_PATTERNS` was already merged into every agent's .gitignore, but only at git init / first Push — too late for the first compatibility report and never at all for a `local:` agent that never syncs. Shipping the same list in the template moves protection from first sync to first boot. It is byte-identical to what the sync merge and the #668 auto-fix would write, so birth-state == post-auto-fix state == post-sync state. Content is derived mechanically from the fenced gitignore block in docs/TRINITY_COMPATIBLE_AGENT_GUIDE.md (itself parity-tested against the constant), never hand-typed. Each `dd-*` keeps its own `outputs/` rule in a trailing `# Template-specific` section, so this augments those files rather than clobbering their intent. No bundled template gains a bare `.claude/` line — that is HARD check G-001, and it would trade four findings for one. Measured with the real STATIC_CHECKS registry over a collector-shaped snapshot: the 11 `dd-*` go 4 HARD -> 0; the 3 starters go 6 HARD -> 2, the residual being T-004/T-005 (`resources.cpu`/`resources.memory` absent from template.yaml). Those are deliberately not fixed here — a template-level `resources` block overrides the admin's fleet-wide default (RES-001), so pin-vs-inherit is a product decision. AC#2 ("0 hard findings") is therefore met for 11 of 14 templates and not for the 3 starters. tests/unit/test_1908_bundled_template_gitignore.py is both the guard and the regenerator (`--regenerate`), so a new canonical entry is a one-command change however many templates are guarded. It evaluates the real `static_checks.run_static` rather than re-implementing the rules, asserts `status == "pass"` positively plus set-equality with the requested ids (a renamed id returns "skipped", so "nothing failed" would empty itself), pins the one legitimate precondition skip (D-003, no dashboard.yaml), fails if a new *visible* template is added outside GUARDED_TEMPLATES, and applies G-001 to every bundled directory including hidden ones. Verified non-decorative by mutation: appending `.claude/` to sage fails 5 tests and deleting its .gitignore fails 6. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…guard (#1908) The regenerator builds all 14 bundled `.gitignore` files by copying the ```gitignore``` fence out of `docs/TRINITY_COMPATIBLE_AGENT_GUIDE.md` verbatim, but nothing asserted that direction. `test_doc_and_constant_in_sync` checks only `_GITIGNORE_PATTERNS ⊆ doc`, and the two guards added in the previous commit are one-way too (`canonical ⊆ shipped`, then byte-identity *with the regenerator*). So any line added to that doc fence shipped, unreviewed and untested, into every bundled template — and thence into every new agent's own repository, where the sync-time `git rm --cached` sweep would untrack whatever it newly matched. Proven open, not assumed: injecting `skills/` into the fence and re-running the regenerator left `test_guarded_gitignore_mirrors_canonical_patterns` and `test_guarded_gitignore_matches_the_regenerator` both GREEN. - `ALLOWED_NON_CANONICAL` pins the delta that exists today (`!.env.example`, `!.mcp.json.template` — the guide's fence is a superset of the constant by exactly these two negations) with the reason each is admissible. - `test_guarded_gitignore_ships_no_unreviewed_pattern` closes the direction: a shipped line must be canonical, pinned, or that template's own `TEMPLATE_EXTRAS`. It REDs on the `skills/` injection above. - `test_allowed_non_canonical_is_not_stale` retires the pin if the guide drops an entry, so it can never become a standing licence. Also corrects three claims that did not hold: - `_EXPECTED_SKIPS`'s "no bundled template ships a dashboard.yaml" — `trinity-system` does; it is simply hidden and unguarded. - the module docstring's implication that the guide block is a faithful mirror of `_GITIGNORE_PATTERNS` (the parity test is one-way). - requirements §4.1.1's "byte-identical to what the sync merge and the #668 auto-fix would write" — both are append-if-missing, so the accurate and stronger statement is that they are *no-ops* on this file. Guard 78 -> 93 passed; full `tests/unit -m "not slow"` 6086 passed / 18 skipped (baseline 6071, delta = the 15 new cases). Regenerator still idempotent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Resolve by running |
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Verdict: the code is approve-grade. Filing as a comment rather than an approval because the only open item would be decided by side effect on merge — Fixes #1908 auto-closes an issue whose AC#2 is unmet and whose amendment is unratified. Four checkboxes at the bottom, none of them code changes.
Everything below was re-derived from source, not read off the PR body. Branch is already rebased onto current dev (8e924526); all checks green.
Verified independently
The measured outcome reproduces. Ran the real services.compatibility.static_checks.run_static over all 25 bundled templates:
| HARD fails | |
|---|---|
11 × dd-* |
— none — |
sage / scout / scribe |
T-004, T-005 only |
default (hidden) |
S-001,S-002,S-004,S-005,T-004,T-005 |
trinity-system (hidden) |
S-004,S-005 |
No functional regression. Rebuilt 5 agent repos using startup.sh:352's exact find … -print0 | cp -r semantics (find doesn't shell-glob, so .gitignore is copied). Every file tracked, 0 newly ignored. .trinity-initialized at startup.sh:346 confirms the non-retroactive claim.
The new seam really is closed — this is the strongest thing in the PR. Two-stage mutation:
- Injected
.claude/skills/into the guide's```gitignorefence, no regeneration → 14 RED. - Then ran
--regenerate, i.e. the exact command the failure message instructs → the hostile pattern landed insage/.gitignore, andtest_guarded_gitignore_ships_no_unreviewed_patternwas still RED on all 14.
So a pattern cannot reach every new agent's own repository without an explicit, justified ALLOWED_NON_CANONICAL entry. That direction was genuinely open before this PR. Guard runs clean at 93 passed.
One new finding: S-001 is a false-negative (pre-existing, NOT introduced here)
static_checks.py:295 accepts .env or .env.*. In gitignore syntax .env.* requires a literal dot after env, so it does not match the bare file. Proven with git rather than asserted — a repo whose .gitignore contains only .env.*, with a placeholder key written to .env:
TRACKED files : .env .gitignore <- the credential file is staged
check-ignore .env -> NOT IGNORED
check-ignore .env.local -> .gitignore:1:.env.*
A HARD check whose message is "credentials may be committed" reports pass on a config that commits credentials.
No bundled template is exposed: _GITIGNORE_PATTERNS ships both forms and this PR's own test_guarded_gitignore_mirrors_canonical_patterns enforces both reach every template. But the class is worth recording — when I stripped bare .env from sage/.gitignore, S-001 stayed green and only this PR's parity tests fired. The guard here is stronger than the oracle it is built on. Practically reachable for a github: template whose author writes .env.* alone: graded "compatible" until the sync-time merge repairs it.
Minor, same area: the ALLOWED_NON_CANONICAL justification says "K-004 HARD-scans it" — spec.py:146 declares K-004 soft, so the cited compensating control for the !.env.example re-inclusion is weaker than stated. Inert today (no bundled template ships a .env.example).
AC#2 — the amendment is right, and there is a second witness
Verified the override chain at current head: crud.py:900 reads the raw template resources → :1147-1155 normalizes with _get_default_resource as the fallback → :266-269 → settings_service.py:817 reads the admin's RES-001 row. A pinned resources: block does override fleet-wide policy.
The decisive evidence isn't in this PR — it is already merged. config/agent-templates/default/template.yaml, landed by #1759:
# DELIBERATELY DECLARES NO resources: BLOCK.… "aresources:block here would silently overwrite what a manifest asked for. The ent#125 resilient-deploy test usesresources: {cpu: "3"}on alocal:defaultagent as its deliberate pre-side-effect failure vector — aresources:block would neutralise it."
That makes this an established repo convention with an independent witness, not a judgement call — and default currently fails T-004/T-005 as punishment for doing the documented right thing. T-004/T-005 being HARD is the actual defect, exactly as the amendment argues. Satisfying AC#2 literally would make the three most-picked starters silently opt out of the operator's resource policy in order to clear a validation finding. Agreed: don't do that.
local:default — recommend filing it, not adding it here
Reachability is real and durable: system_service.py:742 infers template = 'local:default' for a template-less manifest entry, and sibling #1935 explicitly reaffirms the contract — "Hidden templates (hidden: true) stay resolvable by id: the barrier reads the name, never the flag (#1513 contract preserved)." Deliberate platform design, not a transient gap.
I did test the one-line fix: adding "default" to GUARDED_TEMPLATES + --regenerate takes the guard 93 → 99 passed, with 7 related files (incl. test_ent125_resilient_system_deploy, both 1759 files) at 93 passed and 0 newly-ignored files. Cheap and safe.
Still recommending against including it here, for one reason: AC#4 says "No change to the hidden internal fixtures." Adding default knowingly breaks a second stated AC of the issue this PR auto-closes — which is the same problem the AC#2 item exists to fix. Two knowingly-unmet ACs is a worse position than one, and it only takes default 6→2 HARD (same T-004/T-005 residual), so it closes nothing cleanly. Better as one coherent follow-up covering default, sleep-echo, the 6 test-* fixtures, and trinity-system's S-004/S-005 gap.
To unblock
- Edit the AC#2 amendment into #1908 — independently corroborated above.
- File the
T-004/T-005pin-vs-inherit spec question so it survives the auto-close. - File the hidden-template follow-up (
defaultreachable viasystem_service.py:742; #1935 reaffirms hidden stays creatable by id). Recommend not folding it in here. - File
S-001(static_checks.py:295). Pre-existing, no bundled template exposed.
Non-blocking nits: #1908 carries both status-in-progress and status-ready (/groom); the PR body's "sibling PR #1931" should read #1934 (#1931 is the issue, the mechanism cited is correct); and the generated # Keep: … settings.local.json comment is false — *.local.json ignores it (pre-existing in the constant, now with 14 more homes).
Happy to flip this to an approval the moment AC#2 is edited — the code itself needs nothing.
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Approving — with the auto-close removed
My earlier review said "happy to flip this to an approval the moment AC#2 is edited — the code
itself needs nothing." The code still needs nothing. I've taken the other exit instead.
What I changed: the PR body's Fixes #1908 → Refs #1908. Nothing else — no code, no tests,
no docs.
Why that exit rather than amending the AC: #1908 is @obasilakis's issue, and AC#2 ("an agent
created from each reports 0 hard compatibility findings") is his to rule on. Editing the amendment
into someone else's acceptance criteria and then approving against the bar I'd just lowered is the
wrong order of operations, however well-evidenced the amendment is — and it is well-evidenced:
T-004/T-005 being HARD is the real defect, and satisfying AC#2 literally would make sage/
scout/scribe pin resources: blocks that silently override the operator's RES-001 fleet
policy. config/agent-templates/default/template.yaml (landed by #1759) is the independent
witness, and it currently fails T-004/T-005 as punishment for doing the documented right thing.
So #1908 stays open. @obasilakis decides whether the amendment is right and closes it himself.
The merge no longer forces that decision by side effect, which was my only objection.
Everything I verified in the first pass stands and was re-derived, not read off the body: the
measured outcome reproduces across all 25 bundled templates, 0 newly-ignored files across 5
rebuilt agent repos, and the two-stage mutation confirms a pattern cannot reach every new agent's
repository without an explicit ALLOWED_NON_CANONICAL entry. Guard clean at 93 passed.
Still open, none of it blocking, none of it code:
docs/memory/feature-flows.md"Recent Updates" row is missing — filing separately rather than
writing it myself, since I'm the reviewer here.- The three follow-ups from the first pass (
T-004/T-005pin-vs-inherit spec question, the
hidden-templatelocal:defaultreachability viasystem_service.py:742, andS-001at
static_checks.py:295) are still unfiled. - #1908 carries both
status-in-progressandstatus-ready. - Body nit: "sibling PR #1931" should read #1934 (#1931 is the issue; the mechanism cited is
correct). #1934 is now merged asf5a21624.
Note dev has moved three times today (#1935 609f8dff, #1934 f5a21624, #1940 d632c3a5);
re-simulated this branch against the new tip and it still merges clean.
Refs #1908
What was wrong
Trinity grades every agent against
docs/agent-validation-spec.md, but nothing graded the templates Trinity itself ships. All 14 visible bundled templates failed the same four HARD security checks at birth:sage/scout/scribeshipped no.gitignoreat all;dd-*templates shipped a byte-identical two-line one (outputs/+*.log) covering 0 of 4 — none of.env(S-001),.mcp.json(S-002),.claude/projects/(S-004),.trinity/(S-005)._GITIGNORE_PATTERNSwas already merged into every agent's.gitignore, but only at git init / first push — too late for the first compatibility report, and never at all for alocal:agent that never syncs.The honest result
11 of 14 bundled templates reach 0 HARD findings. The 3 starters go 6 → 2.
This is deliberately not "0 hard findings across the board". The residual on
sage/scout/scribeisT-004/T-005(resources.cpu/resources.memoryabsent fromtemplate.yaml), left unfixed on purpose — see the AC#2 note below.It fixes new agents only.
startup.shcopies/templateonly when/home/developer/.trinity-initializedis absent, so agents already created keep their existing.gitignore. They are served by the per-agent auto-fix (POST /api/agents/{name}/compatibility/fix, #668) and the sync-time merge. Nothing here is retroactive.AC#2 needs amending
AC#2 as written ("an agent created from each reports 0 hard compatibility findings on first load") is not satisfiable by this fix, and I don't think it should be satisfied by it. Pinning a
resources:block in a bundled template overrides the admin's fleet-wide default (RES-001,PUT /api/settings/agent-defaults/resources). Whether the default starters should pin or inherit is a product decision, not a hygiene bug — and arguablyT-004/T-005shouldskiprather thanfailwhen a template deliberately inherits. Proposed amendment posted on the issue.Deliberate decisions
Scope is 14 templates, not the 3 in the issue title — an explicit human decision. The 3 starters had no
.gitignore(added); the 11dd-*had one covering 0/4 HARD checks (augmented). The decisive rationale is thathidden⇏ uncreatable: sibling PR #1931 hidesdd-*from the catalog, but hidden templates remain creatable by id —hiddenis read at exactly two sites (template_service.py:474parse,:529catalog filter) and by no resolver, andcrud.pyhas no hidden gate. Hiding a template does not fix its security posture.outputs/is preserved in a trailing# Template-specificsection rather than overwritten. The plan had said overwriting "loses nothing" because this repo ignoresconfig/agent-templates/**/outputs/— but that governs Trinity's repo, while the shipped file is copied to/home/developer/.gitignoreand governs the agent's own repo, where dropping it would auto-commitdd-*due-diligence artifacts on the 15-minute auto-sync push.A new closed seam.
render()copies the```gitignorefence out ofdocs/TRINITY_COMPATIBLE_AGENT_GUIDE.mdverbatim, and that fence is a superset of_GITIGNORE_PATTERNS— 48 vs 46 patterns, delta exactly!.env.exampleand!.mcp.json.template. The existingtest_doc_and_constant_in_synconly assertsconstant ⊆ doc, so before this PR a pattern injected into that doc fence would ship, unreviewed and untested, into every new agent's own repository. Proven open, not assumed: injectingskills/into the fence and re-running the regenerator left both other guards GREEN. Now pinned viaALLOWED_NON_CANONICALplustest_guarded_gitignore_ships_no_unreviewed_pattern, which REDs on that injection, andtest_allowed_non_canonical_is_not_staleretires the pin if the guide drops an entry.No bundled template gains a bare
.claude/line — that is HARD check G-001, and it would trade four findings for one. That single assertion runs over every bundled directory, hidden included.docs/memory/architecture.mdis deliberately not changed — no endpoint, schema, model, or component change (verified: 0 matches forAPIRouter/@router./CREATE TABLE/BaseModeladditions in the diff). The contract lands indocs/memory/requirements/core-agent.md§4.1.1.Guard design
tests/unit/test_1908_bundled_template_gitignore.pyis both the guard and the regenerator (--regenerate), so a new canonical entry is a one-command change however many templates are guarded (same pattern astests/lint_sys_modules.py --regenerate-baseline). It evaluates the realservices.compatibility.static_checks.run_staticover a collector-shaped snapshot rather than re-implementing the rules, assertsstatus == "pass"positively plus set-equality with the requested ids (a renamed id returns "skipped", so a bare "nothing failed" would silently empty itself), pins the one legitimate precondition skip, fails if a new visible template is added outsideGUARDED_TEMPLATES, and waives exactlyT-004/T-005with a staleness test.Evidence
1908,github_init_gitignore,953_startup_sh_no_gitignore_writes,compatibility_checks)tests/unit -m "not slow"(-p no:randomly)tests/lint_sys_modules.py--regenerate→ 0 changed files/verify-local --skip-agentMutation-proven non-decorative (each mutation applied then reverted, guard re-confirmed at 93 green):
.claude/appended tosage→ 6 failedsage/.gitignoredeleted → 7 failedskills/injected into the guide fence → REDs the new unreviewed-pattern test (was green before this PR)Empirical collateral proof. 14 throwaway git repos built with
startup.sh's exactfind … -print0 | cp -randcore.excludesFile=/dev/null: all 14 track every file,ignored=[none]. Separately re-verified here that no tracked file underconfig/agent-templates/became newly ignored (106 tracked files, 0 newly ignored, 0 untracked).Verification limitation — stated honestly
config/agent-templateshas noCOPYin any Dockerfile. It is a bind mount (docker-compose.yml:288,docker-compose.prod.yml:306→/agent-configs/templates:ro); the built image's/app/config/contains onlymanifests,process-docs,process-templates, and/agent-configs/is an empty mount point. So/verify-localproves nothing about these files landing in an image — no image-coverage claim is made. The equivalent-coverage evidence is the bare guard run plus the 14-repo experiment above.Test-run notes
test_1081_physical_meter.py::test_max_respects_506_clamp[sqlite]and::test_available_floors_at_zero_over_ceiling[sqlite]. Under deterministic order (-p no:randomly) the suite is fully green (0 failed), as reported above. Both also pass in isolation (14 passed), and both pass when run alongside the new guard (107 passed together, so this PR's test file is not the polluter). Both depend on the shared fleet-widemax_parallel_tasks_ceilingsystem_settingsrow. This branch changes zero backend Python — only 14.gitignorefiles, one requirements doc, and its own new test file.test_1771a/b/c(4 files) fail collection withModuleNotFoundError: No module named 'hypothesis'on the local interpreter; the branch touches no1771file. Excluded from the counts above.Follow-ups (not filed — listed here deliberately)
T-004/T-005pin-vs-inherit is the product decision described above; arguably both shouldskipwhen a template deliberately inherits the fleet default.test_doc_and_constant_in_syncis one-way (constant ⊆ doc); the reverse direction arguably belongs intest_github_init_gitignore.pynext to it rather than in this issue's guard.local:defaultships no.gitignoreat all and is the inferred template for a template-less manifest entry (system_service.py:742—template = 'local:default'), so it is genuinely user-reachable.# Keep: … settings.local.json, while*.local.json(git_service.py:1064) ignores it. This is wrong for every agent today, not just these templates.config/agent-templates/README.mdshould carry the hygiene contract once bug(library): the agent catalog still ships 11 dd-* VC-demo templates and a Dec-2025 GitHub repo list as real options #1931's rewrite lands.template.yaml) should converge with this guard — same hook, one assertion wider.crud.pymaterialize step would extend this protection togithub:and deploy-local templates, which never see a bundled.gitignore./tmp/trinity-1908-regen.db(cosmetic).Merge order
docs/memory/requirements/core-agent.md§4.1.1 collides with both siblings — this PR inserts### 4.1.1after §4.1's bullets at hunk@@ -121,6 +121,17 @@. PR #1899'score-agent.mdhunk is 7 lines clear. First-merged wins; the others rebase.Checks
Required checks on
devareAnalyze (python),Analyze (javascript-typescript),schema-parity,verify-non-root.schema-parityis a no-op here (no schema change).backend-unit-testis currently red-but-not-blocking ondev.🤖 Generated with Claude Code