fix(agent): resolve a forced skill by the same rule as a forced snippet (#810)

Two issues in the same three functions. #781 is a forced skill and a
forced snippet answering the same question differently; #777 is the same
forced skill set being ordered two ways.

## What the issues got wrong, and what the code says

#781 described the disagreement as living inside `AgentRunRequestCodec`.
It is wider: **three** places rebuild a forced skill set from persisted
uids, and the playground has the same disagreement as the codec.

`AgentRunRequestCodec::skillsByUids()` also carried a docblock saying
the missing `enabled` filter was deliberate — "forcing a skill overrides
its global toggle, the same semantics the playground's force-inject
control has". That reads like an intended rule, so it was checked rather
than taken: no record decides it, and the playground does not behave
that way — `availableSkills()` lists enabled skills only, so a disabled
skill can reach the forced set only from a stale form or a hand-built
body. The sentence described neither an intent nor the thing it claimed
to copy.

## The decision comes from ADR-166, not from me

ADR-166 already wrote the rule down twice. `findByUids()` is "the lookup
for a prompt being assembled *now*, and a snippet an operator switched
off must not enter one", and "a fresh run that forces it gets it through
the active-only lookup like any other".

Its "**the skill half was already correct**" is true of the path it was
judging — the resume — and says nothing about the two that compose. So
the fix direction is not a coin toss between #781's two options; the
record picks one, and this change applies it to the path ADR-166 never
looked at. ADR-175 records that and ADR-166 gains the matching
`:Amended:`.

| Path | Composing or re-gating | Lookup |
|---|---|---|
| `ToolPlaygroundController::resolveForcedSkills()` | composing now |
`findByUids()` — enabled only |
| `AgentRunRequestCodec::skillsByUids()` | composing now (dequeue) |
`findByUids()` — enabled only |
| `ToolLoopService::augmentationFrom()` | re-gating sent text |
`findExistingByUids()` — existence |

## The ordering half (#777)

The resume path iterated `findAll()` and so returned
`$defaultOrderings`, `name ASC`; the two composition paths returned the
persisted uid order. `InputContextClassification::withStricter()` keeps
the **later** source on an equal data class, and that source's name is
what the refusal message and the governance row carry — so one run
blamed one skill before it suspended and another after it resumed. Same
ceiling, same outcome, different name in the audit. All three now use
the caller's order.

## Verification

Ten functional cases against real rows. The fixture is its own and its
names sort **against** its uids — `21` is "Zulu", `22` is "Alpha" — so
an order assertion distinguishes the caller's order from `name ASC`
instead of passing under both.

Each assertion was seen to fail before being trusted:

| Control | Result |
|---|---|
| drop the `enabled` clause from `orderedByUid()` |
`findByUidsSkipsDisabledSkills` fails |
| return query order instead of the caller's | 3 tests fail, including
both `findExisting…` order cases |

Restored, green, `git status` clean.

**One existing unit test broke, and that was a real signal rather than
noise.** `ToolLoopServiceTest::resumeRebuildsTheForcedSkillsToo` stubbed
`findAll()`. Its assertion — a disabled skill still resolves on resume —
is exactly what must not regress, so the double was moved to the new
lookup rather than the test being relaxed. It now also asserts *which*
uids reach that lookup, so the seam is covered here while the filtering
is proven against the database, where a stub cannot flatter it.

Gates: `phpstan` level 10 clean, `unit` 7137 pass, `fuzzy` pass,
`functional -d sqlite` on the touched classes 38 pass, `cgl` and `rector
-n` both stable at PHP 8.2 after applying.

## Deliberately not in this PR

A forced source that disappears before a run starts is **silent** — now
uniformly, where before it was silent for snippets and absent for
skills. #781 asked for it to be visible. That needs somewhere to show it
and a decision on whether a queued run should refuse instead, so it is
filed as #809 rather than guessed at here.

The ADR is in this PR rather than ahead of it because it amends a
decision instead of taking one.

Closes #781, closes #777

Author: CybotTM

CHANGELOG.md

@@ -203,6 +203,24 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   A row suspended before this change has neither key and resumes exactly as it
   did. A uid that no longer resolves — the snippet was deleted meanwhile —
   contributes nothing rather than stranding the run.
+- A forced skill now resolves enabled-only where it is being composed, and by
+  existence only where it is being re-gated — the rule ADR-166 already gave
+  snippets, applied to skills (ADR-175).
+
+  A run queued with a forced skill that an operator disabled before the run
+  started used to keep that skill, while a forced snippet deactivated in the
+  identical situation was dropped. The two source kinds now answer the same
+  question the same way on the playground's synchronous send and on a queued
+  run's dequeue: a source switched off does not enter a prompt being assembled
+  now.
+
+  A resume is unchanged and deliberately so — its text is already in the
+  transcript, so dropping the source there would lower the ceiling of content
+  still going out.
+
+  `SkillRepository` gains `findByUids()` and `findExistingByUids()`. It is
+  `@internal`, so no frozen surface moves.
+
 
 ### Fixed
 
@@ -256,6 +274,16 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   so the profile had to be found again by hand, one screen after reading its
   name. The readout has always honoured a `profile` query parameter; only the
   link never sent one.
+- The same forced skill set is no longer ordered two different ways.
+
+  Three places rebuilt it from persisted uids; two kept the order the run was
+  started with and the resume path returned `name ASC`. On an equal data class
+  the later source in the fold wins, so one run named one skill in a refusal
+  before it suspended and a different one after it resumed — same ceiling, same
+  outcome, different name in the message and in the governance row. All three
+  now use the caller's order (ADR-175).
+
+
 
 
 ## [0.29.1] - 2026-08-13

Classes/Controller/Backend/ToolPlaygroundController.php

@@ -983,25 +983,7 @@ private function uidListFromBody(mixed $body, string $key): array
      */
     private function resolveForcedSkills(array $uids): array
     {
-        if ($uids === []) {
-            return [];
-        }
-
-        $byUid = [];
-        foreach ($this->skillRepository->findAll() as $skill) {
-            if ($skill instanceof Skill && $skill->getUid() !== null) {
-                $byUid[$skill->getUid()] = $skill;
-            }
-        }
-
-        $skills = [];
-        foreach ($uids as $uid) {
-            if (isset($byUid[$uid])) {
-                $skills[] = $byUid[$uid];
-            }
-        }
-
-        return $skills;
+        return $this->skillRepository->findByUids($uids);
     }
 
     /**

Classes/Domain/Repository/SkillRepository.php

@@ -76,4 +76,97 @@ public function countEnabled(): int
 
         return $query->count();
     }
+
+    /**
+     * Find enabled skills by uid, preserving the input order.
+     *
+     * Unknown and disabled uids are silently skipped.
+     *
+     * This is the lookup for NEW composition — a skill an operator switched
+     * off must not enter a prompt that is being assembled now. Do not merge it
+     * with {@see self::findExistingByUids()}: the two answer different
+     * questions and the difference is deliberate (ADR-175, mirroring the
+     * snippet pair ADR-166 introduced).
+     *
+     * @param list<int> $uids
+     *
+     * @return list<Skill>
+     */
+    public function findByUids(array $uids): array
+    {
+        return $this->orderedByUid($uids, true);
+    }
+
+    /**
+     * Find skills by uid regardless of their enabled flag, preserving the
+     * input order.
+     *
+     * Same contract as {@see self::findByUids()} — unknown uids are silently
+     * skipped, a deleted record still resolves to nothing — except that a
+     * disabled skill still resolves.
+     *
+     * This is the lookup for text that is ALREADY in a transcript: a resumed
+     * agent run re-loads its forced sources to re-gate them (ADR-165), and
+     * disabling a skill mid-run must not silently drop the classification of
+     * text that is still on the wire (ADR-166). "Disabled" means "not for new
+     * composition", not "already-injected text loses its class".
+     *
+     * @param list<int> $uids
+     *
+     * @return list<Skill>
+     */
+    public function findExistingByUids(array $uids): array
+    {
+        return $this->orderedByUid($uids, false);
+    }
+
+    /**
+     * Resolve uids to skills in the caller's order, optionally restricted to
+     * enabled ones.
+     *
+     * The repository's default query settings ignore enable fields, so TYPO3's
+     * own `hidden` plays no part here either way; `enabled` is the extension's
+     * own field and $enabledOnly is what decides whether it applies. The
+     * deleted restriction is untouched in both modes — a deleted record never
+     * resolves. The caller's order replaces $defaultOrderings, which is the
+     * point: the fold in {@see \Netresearch\NrLlm\Domain\ValueObject\InputContextClassification::withStricter()}
+     * lets the later source win a tie, so the order a run was started with has
+     * to be the order every later lookup reproduces.
+     *
+     * @param list<int> $uids
+     *
+     * @return list<Skill>
+     */
+    private function orderedByUid(array $uids, bool $enabledOnly): array
+    {
+        if ($uids === []) {
+            return [];
+        }
+
+        $query      = $this->createQuery();
+        $uidMatches = $query->in('uid', $uids);
+        $query->matching(
+            $enabledOnly
+                ? $query->logicalAnd($query->equals('enabled', true), $uidMatches)
+                : $uidMatches,
+        );
+
+        /** @var array<int, Skill> $skillsByUid */
+        $skillsByUid = [];
+        foreach ($query->execute() as $skill) {
+            $uid = $skill->getUid();
+            if ($uid !== null) {
+                $skillsByUid[$uid] = $skill;
+            }
+        }
+
+        $ordered = [];
+        foreach ($uids as $uid) {
+            if (isset($skillsByUid[$uid])) {
+                $ordered[] = $skillsByUid[$uid];
+            }
+        }
+
+        return $ordered;
+    }
 }

Classes/Service/Agent/AgentRunRequestCodec.php

@@ -205,9 +205,15 @@ private function uidList(mixed $value): array
     }
 
     /**
-     * Forced skills by uid, preserving order. Resolved without the enabled
-     * filter — forcing a skill overrides its global toggle, the same semantics
-     * the playground's force-inject control has.
+     * Forced skills by uid, preserving order, enabled only.
+     *
+     * A queued run has composed nothing yet, so dequeuing it is new
+     * composition: a skill switched off between enqueue and start must not
+     * enter it, exactly as {@see self::snippetsByUids()} has always held for
+     * snippets (ADR-166, ADR-175). This docblock previously claimed the
+     * opposite — that forcing overrides the global toggle — which no record
+     * decided and which the force-inject picker does not offer, since it lists
+     * enabled skills only.
      *
      * @param list<int> $uids
      *
@@ -219,21 +225,7 @@ private function skillsByUids(array $uids): array
             return [];
         }
 
-        $byUid = [];
-        foreach ($this->skillRepository->findAll() as $skill) {
-            if ($skill instanceof Skill && $skill->getUid() !== null) {
-                $byUid[$skill->getUid()] = $skill;
-            }
-        }
-
-        $skills = [];
-        foreach ($uids as $uid) {
-            if (isset($byUid[$uid])) {
-                $skills[] = $byUid[$uid];
-            }
-        }
-
-        return $skills;
+        return $this->skillRepository->findByUids($uids);
     }
 
     /**

Classes/Service/Tool/ToolLoopService.php

@@ -171,9 +171,16 @@ private function uidsOf(array $records): array
      * already in the transcript and
      * still goes on the wire, so dropping it here would lower the ADR-164
      * ceiling for content that is still being sent. "Inactive" bars a snippet
-     * from new composition; it does not un-classify text already injected. The
-     * skill half needs no counterpart: {@see SkillRepository::findAll()} ignores
-     * enable fields and the filter below is by uid only.
+     * from new composition; it does not un-classify text already injected.
+     *
+     * The skill half is the same shape and uses
+     * {@see SkillRepository::findExistingByUids()} for the same reason. It also
+     * fixes an ordering divergence: this path used to iterate
+     * {@see SkillRepository::findAll()} and so returned name order, while the
+     * two composition paths returned the order the run was started with. On an
+     * equal data class the later source in the fold wins, so the same run
+     * blamed one skill before it suspended and another after it resumed
+     * (ADR-175).
      */
     private function augmentationFrom(SuspendedRunState $state): ?RunAugmentation
     {
@@ -185,15 +192,9 @@ private function augmentationFrom(SuspendedRunState $state): ?RunAugmentation
             ? $this->promptSnippetRepository->findExistingByUids($state->forcedSnippetUids)
             : [];
 
-        $skills = [];
-        if ($state->forcedSkillUids !== [] && $this->skillRepository instanceof SkillRepository) {
-            $wanted = array_flip($state->forcedSkillUids);
-            foreach ($this->skillRepository->findAll() as $skill) {
-                if ($skill instanceof Skill && $skill->getUid() !== null && isset($wanted[$skill->getUid()])) {
-                    $skills[] = $skill;
-                }
-            }
-        }
+        $skills = $state->forcedSkillUids !== [] && $this->skillRepository instanceof SkillRepository
+            ? $this->skillRepository->findExistingByUids($state->forcedSkillUids)
+            : [];
 
         if ($snippets === [] && $skills === []) {
             return null;

Documentation/Adr/Adr166DeactivationDoesNotLowerACeiling.rst

@@ -7,6 +7,9 @@ ADR-166: Deactivating a source does not lower a ceiling
 :Status: Accepted
 :Date: 2026-08-13
 :Amends: :ref:`ADR-165 <adr-165>` (which lookup the resume re-gate uses)
+:Amended: 2026-08-18 by :ref:`ADR-175 <adr-175>` (its "the skill half was
+    already correct" held for the resume path and not for the two composition
+    paths)
 :Authors: Netresearch DTT GmbH
 
 Context

Documentation/Adr/Adr175ForcedSkillsBindByTheSnippetRule.rst

@@ -0,0 +1,117 @@
+.. _adr-175:
+
+==================================================================
+ADR-175: A forced skill binds by the same rule as a forced snippet
+==================================================================
+
+:Status: Accepted
+:Date: 2026-08-18
+:Amends: :ref:`ADR-166 <adr-166>` (whose "the skill half was already correct"
+    held for the resume path it was judging and not for the two composition
+    paths it did not look at)
+:Authors: Netresearch DTT GmbH
+
+Context
+=======
+
+:ref:`ADR-166 <adr-166>` split the snippet lookup in two.
+:php:`PromptSnippetRepository::findByUids()` stayed active-only — "the lookup
+for a prompt being assembled *now*, and a snippet an operator switched off must
+not enter one" — and :php:`findExistingByUids()` was added for text that is
+already in a transcript, so deactivating a snippet mid-run cannot quietly lower
+the :ref:`ADR-164 <adr-164>` ceiling of content still going out.
+
+That record then said the skill half needed no counterpart, because
+:php:`ToolLoopService::augmentationFrom()` filtered :php:`findAll()` by uid
+alone and so ignored ``enabled`` on both sides. **That was true, and it was
+about one path.** Three places rebuild a forced skill set from persisted uids,
+and only that one is a resume:
+
+- :php:`ToolPlaygroundController::resolveForcedSkills()` — a synchronous send,
+  composing now;
+- :php:`AgentRunRequestCodec::skillsByUids()` — a queued run being dequeued,
+  composing now;
+- :php:`ToolLoopService::augmentationFrom()` — a resume, re-gating text already
+  sent.
+
+On the first two, snippets went through the active-only lookup and skills did
+not. So a forced **snippet** switched off between enqueue and start was gone
+when the run began, while a forced **skill** disabled in the identical
+situation survived (issue `#781`). The run then did not do what the person who
+queued it asked for.
+
+The three copies also disagreed about order. The two composition copies iterate
+the persisted uid list. The resume copy iterated :php:`findAll()` and therefore
+returned :php:`SkillRepository::$defaultOrderings`, ``name ASC``. Order is not
+cosmetic:
+:php:`InputContextClassification::withStricter()` keeps the *later* source on an
+equal data class, and it is that source's name the refusal message and the
+governance row carry. A run started with two equally-classified skills
+therefore blamed one before it suspended and the other after it resumed —
+same ceiling, same outcome, different name in the audit (issue `#777`).
+
+Decision
+========
+
+**Skills get the pair snippets already have.**
+:php:`SkillRepository::findByUids()` resolves enabled skills only;
+:php:`findExistingByUids()` drops the ``enabled`` clause and nothing else. Both
+preserve the caller's order and both keep the deleted restriction, so a deleted
+record still resolves to nothing either way.
+
+**The two composition paths use the enabled-only lookup.** ADR-166's own words
+decide this rather than a new principle: a source an operator switched off must
+not enter a prompt being assembled now, and "a fresh run that forces it gets it
+through the active-only lookup like any other". A queued run has composed
+nothing at enqueue time; dequeuing it *is* that assembly.
+
+**The resume path uses the existence lookup**, which keeps ADR-166's resume
+semantics exactly as written.
+
+**One ordering rule: the caller's uid order, on all three paths.** The order a
+run was started with is the order every later lookup reproduces, so the fold
+names the same source at every point in that run's life.
+
+What this corrects
+==================
+
+:php:`AgentRunRequestCodec::skillsByUids()` carried a docblock stating that
+forcing a skill overrides its global toggle, "the same semantics the
+playground's force-inject control has". No record decided that, and the
+playground does not offer it: :php:`availableSkills()` lists enabled skills
+only, so a disabled skill can reach the forced set only from a stale form or a
+hand-built request body. The sentence described neither an intended rule nor
+the behaviour it claimed to copy, and it is replaced rather than kept.
+
+What this does not do
+=====================
+
+**It does not change what the ceiling reads.** The forced set is the set
+:ref:`ADR-164 <adr-164>` defined; this changes which rows resolve on which
+path, not which rows are asked for.
+
+**It does not make a disabled skill usable.** Nothing composes it, and the
+picker does not offer it.
+
+**It does not surface the drop.** A forced source that disappears before a run
+starts is silent — now uniformly, where before it was silent for snippets and
+absent for skills. That is issue `#809`, split out deliberately: making it
+visible needs a place to show it and a decision about whether a queued run
+should refuse instead, and neither follows from this one.
+
+**It is not an API change.** :php:`SkillRepository` is ``@internal``
+(:ref:`ADR-127 <adr-127>`), so the two new methods change no frozen surface.
+
+Consequences
+============
+
+A run queued with a skill that is disabled before it starts runs without that
+skill, as it already did for a snippet.
+
+The resume path returns uid order where it returned name order. On a
+classification tie the source named by a refusal can differ from what the old
+code would have named — which is the defect this closes, not a new one: the
+name is now the same before and after the suspension.
+
+An installation that forces nothing, or that has classified nothing, is
+unaffected.

Documentation/Adr/Index.rst

@@ -526,3 +526,4 @@ Tools
    Adr172FourEyesApprovalPerConfiguration
    Adr173SelfApprovalIsVisibleInTheRecord
    Adr174PerCallCostAndPreRoutingFacts
+   Adr175ForcedSkillsBindByTheSnippetRule

Tests/Functional/Fixtures/SkillsByUid.csv

@@ -0,0 +1,7 @@
+"tx_nrllm_skill"
+,"uid","pid","source","identifier","name","description","body","body_checksum","source_sha","support_status","unsupported_notes","allowed_tools","orphaned","enabled","tstamp","crdate","deleted","hidden"
+,21,0,2,"2:zulu/SKILL.md","Zulu","Enabled, sorts last by name","Body Z.","z1","sha2","full","","",0,1,1703347200,1703347200,0,0
+,22,0,2,"2:alpha/SKILL.md","Alpha","Enabled, sorts first by name","Body A.","a1","sha2","full","","",0,1,1703347200,1703347200,0,0
+,23,0,2,"2:bravo/SKILL.md","Bravo","Disabled","Body B.","b1","sha2","full","","",0,0,1703347200,1703347200,0,0
+,24,0,2,"2:delta/SKILL.md","Delta","Deleted","Body D.","d1","sha2","full","","",0,1,1703347200,1703347200,1,0
+,25,0,2,"2:echo/SKILL.md","Echo","Hidden but enabled","Body E.","e1","sha2","full","","",0,1,1703347200,1703347200,0,1