fix(parser): stamp Permanent on duration-less additive type grants

[andriypolanski]

Closes #5950

Summary

Discord report: the bushido 1 + Samurai type that Sensei Golden-Tail's training ability grants ("Put a training counter on target creature. That creature gains bushido 1 and becomes a Samurai in addition to its other creature types.") wears off after one turn, even though the card states no duration.

The parse is faithful (a GenericEffect granting AddKeyword(Bushido 1) + AddSubtype(Samurai)), but its duration was None. Per CR 611.2a a resolution-generated continuous effect with no stated duration lasts until end of game (permanent). At the resolution seam (effect.rs::resolve) an unstated None duration is defaulted to Duration::UntilEndOfTurn (a documented known deviation, because None is overloaded there and a global flip is blocked on a parser-side task). So the grant was swept by prune_end_of_turn_effects at cleanup and "wore off."

Root cause: build_continuous_clause (parser/oracle_effect/subject.rs) parsed the "gains … and becomes a … in addition to its other creature types" predicate into the right modifications but left duration = None, relying on that wrong default.

Fix: follow the existing parser-side precedent (the Suspend grant and build_become_clause both stamp Duration::Permanent for known-permanent grants). When a continuous grant carries the additive "in addition to its other [creature] types" marker (CR 205.1b) and no duration was parsed, stamp Duration::Permanent (CR 611.2a). This gates the whole class of additive permanent type grants, not just Sensei Golden-Tail — the same defect affected Curious Colossus ("…loses all abilities, becomes a Coward in addition to its other types, and has base power and toughness 1/1"), whose effect is likewise indefinite and now correctly persists. The override only fires when the duration is unstated, so an explicit "…in addition to its other types until end of turn" keeps its parsed turn-scoped duration. No engine change is needed: the resolver already honors a Permanent duration and prune_end_of_turn_effects never sweeps it.

Changes

• crates/engine/src/parser/oracle_effect/subject.rs — in build_continuous_clause, stamp Duration::Permanent when the predicate carries the additive "in addition to its other [creature] types" marker (has_in_addition_to_other_types, already imported) and no duration was parsed. Mirrors the adjacent Suspend/build_become_clause permanent-default precedents.
• crates/engine/src/parser/oracle_effect/tests.rs — new building-block regression additive_type_grant_with_no_duration_is_permanent (Sensei Golden-Tail): the training grant's GenericEffect (bushido keyword + Samurai subtype) carries Some(Duration::Permanent). Updated comma_list_type_change_and_has_base_pt_parses_as_single_generic_effect (Curious Colossus) — the effect its own doc calls "indefinite" now asserts Permanent (previously codified the None bug).
• crates/engine/src/parser/oracle_tests.rs — updated curious_colossus_base_pt_comma_list_has_no_unimplemented_trigger_tail to assert Some(Duration::Permanent) (was None); Oracle text verified duration-less against Scryfall.

Test Plan

• cargo fmt --all -- --check
• cargo clippy -p engine --lib -- -D warnings
• cargo test -p engine --lib (full suite: 17,477 pass)
• cargo test -p engine --lib -- additive_type_grant_with_no_duration_is_permanent
• cargo test -p engine --lib -- curious_colossus_base_pt_comma_list_has_no_unimplemented_trigger_tail comma_list_type_change_and_has_base_pt_parses_as_single_generic_effect

Out of scope

The broader None-defaults-to-UntilEndOfTurn overload at effect.rs::resolve (blocked on the parser-side stated-duration distribution task) is unchanged — this fix is a targeted parser-side stamp for the additive-type-grant class, consistent with the existing per-class Permanent precedents.

Summary by CodeRabbit

• Bug Fixes

  • Fixed additive creature type/subtype grants with no stated duration to be treated as permanent rather than expiring at end of turn.

• Tests

  • Updated oracle parsing/unit test expectations so additive type grants resolve to permanent.
  • Added an integration regression test for Sensei Golden-Tail to verify the continuous grant persists correctly through end-of-turn cleanup.

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 68fbaaaa-8d90-459f-aec6-6e6ac2448de8

📥 Commits

Reviewing files that changed from the base of the PR and between 82f3eb6 and 810c54b.

📒 Files selected for processing (1)

• crates/engine/tests/integration/main.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/engine/tests/integration/main.rs

---

📝 Walkthrough

Walkthrough

The parser now resolves additive type grants without an explicit duration to Duration::Permanent. Regression tests cover Sensei Golden-Tail persistence through cleanup and Curious Colossus parsing expectations.

Changes

Additive type grant duration

Layer / File(s)	Summary
Permanent duration inference crates/engine/src/parser/oracle_effect/subject.rs	build_continuous_clause assigns Duration::Permanent to additive type grants when no duration is parsed.
Parser and integration regression coverage crates/engine/src/parser/oracle_effect/tests.rs, crates/engine/src/parser/oracle_tests.rs, crates/engine/tests/integration/main.rs, crates/engine/tests/integration/sensei_golden_tail_5950.rs	Tests verify permanent duration parsing for Sensei Golden-Tail and Curious Colossus effects, including persistence after end-of-turn cleanup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: needs-maintainer

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the parser change to stamp Permanent on duration-less additive type grants.
Linked Issues check	✅ Passed	The changes address #5950 by making Sensei Golden-Tail's granted Bushido 1 and Samurai subtype persist past cleanup.
Out of Scope Changes check	✅ Passed	The added Curious Colossus coverage and doc updates support the same duration-handling fix and are not unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[matthewevans]

Changes requested — the parser stamp is at the right seam, but the reported gameplay regression is not tested through resolution and cleanup.

🔴 Blocker

[HIGH] Missing a discriminating production-pipeline regression for the persistent grant. Evidence: crates/engine/src/parser/oracle_effect/subject.rs:3613 changes only the parsed duration, while the new crates/engine/src/parser/oracle_effect/tests.rs:210 test stops at the AbilityDefinition; the observable behavior is produced later by crates/engine/src/game/effects/effect.rs:59 and cleanup retains/removes effects in crates/engine/src/game/layers.rs:152. Why it matters: this PR fixes a resolution-to-cleanup rules result, but its new test cannot prove that activating Sensei Golden-Tail's ability registers a permanent effect which survives cleanup on the affected creature. Suggested fix: add a registered integration scenario that activates the actual ability (or an equivalent parsed production ability), advances through resolution and cleanup, and asserts the target still has both Bushido 1 and the Samurai subtype; it must fail when the parser's Duration::Permanent stamp is reverted.

🟡 Non-blocking

The required current-head parse-diff sticky artifact is still absent. GitHub shows the card-data job succeeded, but the GraphQL review packet at a215fc21dace322a03ecb6ce68f2d3e0e01ecdd4 has no <!-- coverage-parse-diff --> comment, so approval remains withheld until CI publishes it.

Recommendation: request changes for the runtime regression, then re-review the new head with its current parse-diff artifact.

[github-actions]

Parse changes introduced by this PR · 6 card(s), 6 signature(s) (baseline: main 23824afe7521)

🟡 Modified fields (6 signatures)

• 1 card · 🔄 ability/add subtype Demon · changed field duration: ∅ → permanent
  • Affected (first 3): Clavileño, First of the Blessed
• 1 card · 🔄 ability/grant Bushido, add subtype Samurai · changed field duration: ∅ → permanent
  • Affected (first 3): Sensei Golden-Tail
• 1 card · 🔄 ability/grant Defender · changed field duration: ∅ → permanent
  • Affected (first 3): Xathrid Gorgon
• 1 card · 🔄 ability/grant Haste, add subtype Zombie · changed field duration: ∅ → permanent
  • Affected (first 3): Halana and Alena and Gisa and Geralf
• 1 card · 🔄 ability/grant ability · changed field duration: ∅ → permanent
  • Affected (first 3): Assemble from Parts
• 1 card · 🔄 ability/remove all abilities, base power 1, base toughness 1, add subtype Coward · changed field duration: ∅ → permanent
  • Affected (first 3): Curious Colossus

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[matthewevans]

Approved: right parser seam, scoped six-card duration correction, and the registered production regression now proves the grant survives cleanup.

[andriypolanski]

@matthewevans
This pr is stopped, what happened?

[matthewevans]

The implementation review is approved and the required repository checks are green. The PR is currently held outside code review because the independent Contributor trust check remains ACTION_REQUIRED and the PR is marked contributor:flagged. It is not in the merge queue; we cannot re-enqueue it until that external check is cleared.

[matthewevans]

**Blocked — implementation review remains approved; enqueue is halted by the current required contributor-trust check.

🔴 Blocker

• GitHub reports Contributor trust as ACTION_REQUIRED for current head 82f3eb69da9184b22e80228076823f70fc8cb567; required repository checks otherwise pass. This is a maintainer policy decision, not an implementation finding.

Recommendation: resolve the contributor-trust gate, then a maintainer can re-enable merge-when-ready without further code changes.