Commit be4e58f
refactor(capability): the tool-grant grammar moves to OpenRegister (ADR-099 §5) (#563)
* refactor(capability): the tool-grant grammar moves to OpenRegister (ADR-099 §5)
The hermiq half of a PR pair. Requires openregister#2868; this cannot merge
until a build carrying OCA\OpenRegister\Service\Capability ships.
ADR-099 §5 keeps two grant axes and forbids merging them. The CAPABILITY axis —
may agent X use tool T — is not agent-specific and resolves against
ToolRegistryFacade, which already lives in OpenRegister, so it moves to sit
beside what it resolves against. The Agent.tools BINDING stays here.
WHAT MOVED: ToolGrantSet, ToolGrantCodec, ToolGrantResolver, ToolReachResolver,
ToolGrantResolutionException, and four of their five test files. Verbatim — only
the namespace changed. That codec carries a measured scar (35 of 87 tools parsed
wrong) and ADR-095's persistence constraint, and a rewrite-while-moving reopens
both.
ToolGrantResolverTest STAYS here, repointed. It builds its catalog from hermiq's
real tool providers — nine of them — so it is an integration test against a
catalog, not a unit test of the grammar. It belongs where the catalog is.
🔴 THERE IS DELIBERATELY NO STUB FOR THE NEW NAMESPACE, and there must never be
one. hermiq's test bootstrap maps OCA\OpenRegister\ at tests/Stubs/, so the naive
move would have resolved the grammar to a stub — and fifteen test classes
exercise its BEHAVIOUR. A stubbed copy would not merely drift, it would make
fifteen test classes validate a fake while reporting green. This repo has already
paid for that on a THREE-METHOD facade: tests/Stubs/.../ToolRegistryFacade.php
drifted by one method and broke a whole matrix leg. This is ~2,400 lines of
security-relevant parsing.
So bootstrap.php, phpstan.neon and psalm.xml all read the REAL source at
../openregister/lib/Service/Capability. Both topologies put OpenRegister beside
hermiq — CI checks additional apps out at server/apps/openregister next to
server/apps/hermiq, and the dev tree mirrors it — so one relative path serves
both, and no CI change is needed. When it is absent nothing is registered: the
class is genuinely missing and the run fails loudly, which is the point.
TWO FILES NEEDED NEW IMPORTS THAT NOTHING WOULD HAVE FLAGGED. FacadeToolInvoker
and ToolLoop referenced these classes with no `use` at all, because they shared
the namespace. A relocation that only rewrote files carrying an explicit import
would have left them referencing classes that no longer exist — and PHP would not
have said so until the line ran, as a fatal in the middle of a tool call.
CheckOpenRegisterCompatibility now names ToolGrantResolver. The grammar used to
live here, so no OpenRegister could be too old to supply it; now one can, and
without this a stale instance surfaces as a bare "Class not found" from inside a
tool call at whatever hour an agent next ran, instead of a clear message at
upgrade time.
CONSERVATION CHECK, measured rather than assumed: development runs 1834 tests /
5776 assertions; this branch runs 1776 / 5623. The difference is 58 tests and 153
assertions — exactly what openregister#2868 gained. Nothing was dropped in
transit. phpcs, phpmd, psalm and phpstan clean; psalm and phpstan were RED on
this branch until they were pointed at the real source, which is how the missing
type resolution was found rather than assumed away.
* fix(capability): declaration-only stubs so static analysis can still see the grammar
CI found the hole my relative path did not cover. `phpstan.neon` and `psalm.xml`
pointed at ../openregister/lib/Service/Capability, which resolves in the PHPUNIT
job — that one checks additional apps out at server/apps/openregister next to
server/apps/hermiq — but the PHP-QUALITY job does a plain actions/checkout with
no additional apps at all. PHPStan aborted with `Scanned directory
/home/runner/work/hermiq/openregister/lib/Service/Capability does not exist`.
I had verified the path against ONE job's layout and generalised from it.
🔴 THE STUBS THROW, AND THAT IS THE WHOLE DESIGN. A stub that RETURNED plausible
values would make fifteen test classes validate a fake while reporting green —
the failure this repo already paid for on a THREE-METHOD facade, where
tests/Stubs/Service/Mcp/ToolRegistryFacade.php drifted by one method and broke a
whole matrix leg. This is ~2,400 lines of security-relevant parsing.
So the split is: signatures for the analysers, behaviour from the real source.
* php-quality — PHPStan/Psalm resolve the class from tests/Stubs and type-check
the thirteen call sites. Without this every one is an unknown-class error and
the gate goes quiet on all of them, which is strictly worse than a stub.
* phpunit — tests/bootstrap.php maps the REAL source under the longer PSR-4
prefix, which wins over the blanket tests/Stubs mapping regardless of order.
Measured both ways: 1776 tests / 5623 assertions green with the real source
present; 91 loud errors without it. A missing dependency cannot masquerade as
a passing suite.
A signature drift is therefore caught by the test run, which exercises the real
class. This file can fail to DESCRIBE a drift; it cannot hide one.
ToolGrantResolutionException is the one exception and is constructible: ToolLoop
throws it and ToolLoopTest catches it, so a throwing constructor would break a
test that never touches the grammar. It carries its grants and nothing else —
there is no logic in it to get wrong.
* docs(agent-identity): task 3.2 done, with what the relocation actually cost
Records the two things that were not visible when 3.2 was written: the stub
directory would have swallowed the move silently, and two files referenced the
grammar with no import at all because they shared its namespace. Also records
the conservation check — 58 tests and 153 assertions left hermiq and exactly 58
and 153 arrived in openregister — because that is the measurement that
distinguishes a relocation from a quiet drop.
* fix(manifest): three em-dashes in copy a reader sees (gate-96)
Gate 96 (manifest-copy-style) merged to .github/main today and fires full-tree,
so it lands on hermiq's existing manifest rather than on anything this branch
changed. Three user-visible strings break voice.md section 8 — a page
description, the setup step, and a walkthrough step.
Fixed rather than excluded: an em-dash in shipped copy is the tell the rule
exists for, the rewrite costs a colon or a full stop, and the gate is full-tree
by design so it would keep blocking every future PR until somebody did this.
Verified with the gate's own checker: 3 findings before, 0 after, across all 150
manifest strings including the src/manifest.d/ fragments.
---------
Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>1 parent d0473ce commit be4e58f
43 files changed
Lines changed: 753 additions & 3384 deletions
File tree
- lib
- Controller
- Mcp
- Repair
- Service
- Assistant
- Engine
- openspec/changes/agent-identity-narrows
- tests
- Stubs/Service/Capability
- Unit
- Controller
- Mcp
- Repair
- Service
- Assistant
- Engine
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
45 | | - | |
46 | | - | |
| 45 | + | |
| 46 | + | |
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
61 | | - | |
| 61 | + | |
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
43 | | - | |
44 | | - | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
| 31 | + | |
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| 48 | + | |
48 | 49 | | |
49 | 50 | | |
50 | 51 | | |
| |||
81 | 82 | | |
82 | 83 | | |
83 | 84 | | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
84 | 94 | | |
85 | 95 | | |
86 | 96 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
69 | | - | |
| 69 | + | |
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
| 41 | + | |
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
50 | | - | |
| 49 | + | |
| 50 | + | |
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| |||
0 commit comments