Skip to content

github-devloop implementation for #3357 - #3360

Open
jason-aelf wants to merge 6 commits into
auto-work-dev-0701from
devloop/issue/aevatarAI/aevatar/3357/ready-github-devloop-issue-aevatarAI-aevatar-3357-intake-0278889703-4175285404
Open

github-devloop implementation for #3357#3360
jason-aelf wants to merge 6 commits into
auto-work-dev-0701from
devloop/issue/aevatarAI/aevatar/3357/ready-github-devloop-issue-aevatarAI-aevatar-3357-intake-0278889703-4175285404

Conversation

@jason-aelf

Copy link
Copy Markdown
Collaborator

github-devloop implementation PR for issue #3357

…solution integration and executable readiness handoff
@jason-aelf
jason-aelf requested a review from louis4li as a code owner August 9, 2026 04:11
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR child open

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR is ready for review

@jason-aelf jason-aelf added the fkst-dev:reviewing pr-review-consensus-in-progress label Aug 9, 2026
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR is ready for review

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR review decision: reject
Verdicts: teleology=reject parsimony=reject fidelity=reject natural-ownership=reject proportional-containment=reject
Blocking gap: capability-blind exact matching

teleology:
Established capability-based least-privilege and schema-first routing require deterministic correspondence between requested capability and selected operation. No peer claim defeats the root objection; all corroborate it. The resolver computes NormalizedCapability and CapabilityFingerprint (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:24) but exact selection uses only selector kind and TargetUserServiceId (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:327). Because the inventory aggregates all descriptors (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ExternalWorkflowCapabilityReadinessService.cs:21) and the producer emits one descriptor per endpoint (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:553), a sole unrelated endpoint is falsely selected while ordinary multi-endpoint services fail indiscriminately; the test explicitly preserves that service-cardinality behavior (test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:43). Thus the stated “matching exact descriptor” requirement remains unmet.

parsimony:
Established schema-first semantic routing requires a typed operation identity or deterministic capability matcher; the peers reinforce this root claim, and fidelity’s inventory evidence defeats any defense based on service cardinality. The resolver computes NormalizedCapability and CapabilityFingerprint but selects solely by TargetUserServiceId, returning one arbitrary endpoint or rejecting every multi-endpoint service (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:21, src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:309). NyxID produces a distinct descriptor per endpoint (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:553), while tests canonize single-endpoint selection and blanket multi-endpoint rejection without proving requested-capability correspondence (test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:10, test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:37). This fails the stated “matching exact descriptor” requirement.

fidelity:
Established capability-based security and schema-first routing require exact operation selection from authoritative capability semantics. The teleology peer’s stable typed capability-key claim reinforces my position; any counterclaim that target service identity is sufficient is defeated because the producer emits one descriptor per endpoint (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:553), while ResolveExactOperationDescriptor ignores NormalizedCapability and CapabilityFingerprint and matches only TargetUserServiceId (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:327). The tests then canonize sole-endpoint selection and blanket multi-endpoint rejection rather than proving semantic matching (test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:13, test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:43). Thus an unrelated sole endpoint can bypass managed discovery, violating the stated “matching exact descriptor” requirement.

natural-ownership:
Established DDD, ports-and-adapters, and capability-routing practice requires the Application policy to prove semantic correspondence between requested capability and selected operation; service membership is not that proof. No peer claim moved my position: teleology, parsimony, fidelity, and proportional-containment independently confirm the same root failure. The resolver computes NormalizedCapability and CapabilityFingerprint, but ResolveExactOperationDescriptor ignores both and filters only by selector kind and TargetUserServiceId, selecting any sole endpoint or rejecting every multi-endpoint service as ambiguous (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:24, src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:327). This is load-bearing because inventory aggregates all descriptors (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ExternalWorkflowCapabilityReadinessService.cs:21), while the producer emits one descriptor per endpoint (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:553); the tests merely canonize service-cardinality selection and blanket ambiguity instead of proving an unrelated sole endpoint misses or one matching endpoint wins among several (test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:13, test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:43). The stated matching-exact-descriptor requirement therefore remains unmet.

proportional-containment:
Capability-based routing requires matching requested intent to authoritative operation semantics. The peer outputs reinforce rather than defeat the root claim: ResolveExactOperationDescriptor ignores NormalizedCapability and CapabilityFingerprint, filtering only by UserServiceId, so one unrelated endpoint is falsely selected and multiple endpoints are rejected without semantic matching (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:327). The producer emits one descriptor per endpoint but exposes no typed capability key beyond endpoint identity and display text (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:553), while tests codify service-cardinality behavior instead of proving a requested-capability match (test/Aevatar.Workflow.Application.Tests/ServiceApiWorkflowCapabilityResolutionServiceTests.cs:43). This directly fails the stated “matching exact descriptor” requirement.

⟦AI:FKST⟧

@jason-aelf jason-aelf added fkst-dev:fixing review-rejected-and-fix-in-progress and removed fkst-dev:reviewing pr-review-consensus-in-progress labels Aug 9, 2026
@jason-aelf

jason-aelf commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

github-devloop timeout redrive attempt: fixing 2

⟦AI:FKST⟧

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate failed: capability-blind exact matching
Reproduce locally with scripts/run.sh test from the repository root.

…on integration and executable readiness handoff
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop fix pushed for re-review

Previous reviewed head: 102259a
New head: e4efb56

@jason-aelf jason-aelf added fkst-dev:reviewing pr-review-consensus-in-progress and removed fkst-dev:fixing review-rejected-and-fix-in-progress labels Aug 9, 2026
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR is ready for review

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR review decision: reject
Verdicts: teleology=reject parsimony=reject fidelity=reject natural-ownership=reject proportional-containment=reject
Blocking gap: display-name proxy matching

teleology:
Established practice for this authority-routing class is schema-first, least-privilege capability routing: exact selection must bind to producer-owned typed operation facts, not human labels. I defend because the peer outputs reinforce the same root attack. The fix closes service-cardinality matching, but DescriptorCapabilityMatches still proves the request by normalized candidate text and CapabilityFingerprint (diff.patch:802), and those candidates are only descriptor.DisplayName plus the suffix after "/" (diff.patch:815). The NyxID producer constructs that field as DisplayName = $"{service.ServiceName} / {endpoint.Name}" (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:565). That remains proxy-over-truth: label equality is not an authoritative operation capability contract, so the named capability-blind exact matching gap is narrowed but not closed.

parsimony:
Established practice for this class is schema-first capability routing under least privilege: exact operation selection must bind requested capability to producer-owned operation facts, not presentation labels. No peer claim moves me; fidelity’s ContractDigest point reinforces the blocker rather than defeating it. The fix adds DescriptorCapabilityMatches, but its candidate set is still descriptor.DisplayName and the suffix after "/" (diff.patch:802, diff.patch:818), while the NyxID producer constructs that field as DisplayName = $"{service.ServiceName} / {endpoint.Name}" (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:565). Readiness/admission after selection is useful, but it cannot prove the selected endpoint is the authoritative match for the requested capability. The named capability-blind exact matching gap is narrowed to label matching, not closed.

fidelity:
Schema-first capability routing is the governing practice: exact operation selection must be proven from producer-owned typed operation semantics, not from presentation labels. I defend my Phase B position; the peer claims do not defeat it, and teleology/parsimony/natural-ownership instead corroborate the same root blocker. The fix changes service-cardinality matching into DescriptorCapabilityMatches, but that matcher reads candidates from DescriptorCapabilityCandidates and those candidates are descriptor.DisplayName plus the suffix after "/" (diff.patch:802, diff.patch:815). The producer already has a stronger endpoint contract surface in ContractDigest (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:57), yet the descriptor used for matching exposes the human label DisplayName = $"{service.ServiceName} / {endpoint.Name}" (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:553). That narrows capability-blind exact matching, but it still cannot prove a "matching exact descriptor" from authoritative operation facts.

natural-ownership:
The fix changes the prior service-cardinality failure into a display-label proxy, so the named gap remains. Established capability-based least privilege and schema-first contract routing require exact operation selection from producer-owned typed operation semantics; no peer claim supplies proof that this practice is inapplicable. DescriptorCapabilityMatches hashes normalized DescriptorCapabilityCandidates against input.CapabilityFingerprint (diff.patch:802), and those candidates are only descriptor.DisplayName plus the suffix after "/" (diff.patch:815). The producer constructs that text as DisplayName = $"{service.ServiceName} / {endpoint.Name}", not as a typed capability contract (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:565). Teleology, parsimony, fidelity, and proportional-containment therefore corroborate rather than defeat my Phase B root claim: readiness after selection cannot prove that selection matched the requested capability.

proportional-containment:
Established practice for this class is schema-first capability routing under least privilege: exact operation selection must be proven by a stable producer-owned contract key, not by presentation labels, unless the producer proves that label is canonical. The fix does not close capability-blind exact matching; it replaces service-cardinality matching with display-label matching. ResolveExactOperationDescriptor still delegates the semantic test to DescriptorCapabilityMatches (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:338), whose candidate set is descriptor.DisplayName and the suffix after "/" (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:367), while NyxID produces that field as $"{service.ServiceName} / {endpoint.Name}" (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:565). That defeats the root claim that an authoritative matching exact descriptor is proven; teleology, parsimony, fidelity, and natural-ownership corroborate this rather than moving my position.

⟦AI:FKST⟧

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop fix-loop reflection: continue

Reason:
Established PDCA/OODA closed-loop control and schema-first, least-privilege capability routing govern this class: exact selection must prove requested capability against producer-owned typed operation facts, not presentation text. The rounds are still converging on the original "matching exact descriptor" goal because the first rejection moved from service-cardinality selection to DescriptorCapabilityMatches, and the latest rejection narrows the same root gap to descriptor.DisplayName/suffix proxy matching against CapabilityFingerprint; that demand does not exceed the spec, but the next fix must replace label-derived matching with an authoritative typed descriptor key/contract or prove such practice inapplicable.

@jason-aelf

jason-aelf commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

github-devloop timeout redrive attempt: fixing 3

⟦AI:FKST⟧

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.47059% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.48%. Comparing base (d9677ed) to head (7caf9c5).
⚠️ Report is 1 commits behind head on auto-work-dev-0701.

Files with missing lines Patch % Lines
...s/ServiceApiWorkflowCapabilityResolutionService.cs 77.56% 57 Missing and 24 partials ⚠️
...ies/UnavailableServiceApiCapabilityFallbackPort.cs 0.00% 15 Missing ⚠️
...ies/DeferredManagedServiceApiSkillDiscoveryPort.cs 25.00% 6 Missing ⚠️
...nalCapabilities/ExternalWorkflowCapabilityPorts.cs 88.88% 2 Missing ⚠️
@@                  Coverage Diff                   @@
##           auto-work-dev-0701    #3360      +/-   ##
======================================================
- Coverage               84.53%   84.48%   -0.05%     
======================================================
  Files                    1531     1534       +3     
  Lines                  146193   146557     +364     
  Branches                17820    17851      +31     
======================================================
+ Hits                   123579   123826     +247     
- Misses                  14451    14544      +93     
- Partials                 8163     8187      +24     
Flag Coverage Δ
ci 84.48% <76.47%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...bilities/ManagedServiceApiSkillDiscoveryService.cs 97.08% <100.00%> (ø)
...Application/Workflows/WorkflowDefinitionCatalog.cs 99.49% <100.00%> (-0.04%) ⬇️
...DependencyInjection/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
...n/WorkflowCapabilityServiceCollectionExtensions.cs 94.59% <100.00%> (+0.07%) ⬆️
...es/ManagedCodexServiceApiSkillDiscoveryExecutor.cs 98.19% <100.00%> (+0.01%) ⬆️
...nalCapabilities/ExternalWorkflowCapabilityPorts.cs 90.63% <88.88%> (-0.15%) ⬇️
...ies/DeferredManagedServiceApiSkillDiscoveryPort.cs 25.00% <25.00%> (ø)
...ies/UnavailableServiceApiCapabilityFallbackPort.cs 0.00% <0.00%> (ø)
...s/ServiceApiWorkflowCapabilityResolutionService.cs 77.56% <77.56%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…on integration and executable readiness handoff
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop fix pushed for re-review

Previous reviewed head: e4efb56
New head: a068251

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR is ready for review

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR review decision: reject
Verdicts: teleology=reject parsimony=reject fidelity=reject natural-ownership=reject proportional-containment=approve
Blocking gap: capability-key intent mismatch

teleology:
The decisive root reason is missing-inevitability: the old display-name proxy is narrowed, but exact resolution now works only when requested_capability already equals the producer endpoint key. The producer sets CapabilityKey = endpoint.EndpointId (diff.patch:301), while the resolver requires normalized descriptor.CapabilityKey to equal input.NormalizedCapability (diff.patch:826-diff.patch:828), yet the model-facing tool field is still requested_capability and is demonstrated with natural text "Send a message" (diff.patch:1215). Under schema-first capability routing, this skips the purpose of Application-owned intent-to-operation resolution unless the contract is renamed/narrowed to an authoritative key or the resolver proves semantic matching from typed producer facts.

parsimony:
Established practice for this class is schema-first capability routing under least privilege: exact operation selection must be driven by a typed producer-owned key that is actually carried through the caller contract, or by a proved deterministic matcher. Essence: collapse model-owned Service API branching into one typed resolver; ideal: pass/select the authoritative operation key/selector directly. The decisive six-smell failure is missing-inevitability with proxy-over-truth: the tool-facing path still deals in requested_capability and even tests that as "Send a message" (diff.patch:1215), while the producer defines CapabilityKey = endpoint.EndpointId (diff.patch:301) and exact matching requires that key to equal input.NormalizedCapability (diff.patch:828). That proves only callers who already know the endpoint key reach nyxid_operation; ordinary requested capability text can still miss an existing exact descriptor and fall into managed/fallback, so exact-descriptor priority is not actually secured.

fidelity:
Under schema-first capability routing and capability-based least privilege, exact selection must compare producer-owned operation facts with an equally typed caller selector, not a model phrase. The fix still accepts requested_capability as ordinary text ("Send a message" in the new tool test), while the producer sets CapabilityKey = endpoint.EndpointId and DescriptorCapabilityMatches requires that normalized key to equal input.NormalizedCapability (diff.patch:1215, diff.patch:301, diff.patch:828). That is evidenced proxy-over-truth: the branch proves only that free text happened to spell the endpoint id, so a real matching exact descriptor can be missed and managed/fallback routing can run despite the stated exact-descriptor priority.

natural-ownership:
The decisive root reason is narrative-over-verification: the proposal claims one executable Application-owned resolution path, but the DI composition cannot name its new adapters. ServiceCollectionExtensions registers DeferredManagedServiceApiSkillDiscoveryPort by short name (diff.patch:1083), while that adapter is declared under Aevatar.Workflow.Infrastructure.ExternalCapabilities (diff.patch:1116) and the DI file’s namespace is Aevatar.Workflow.Infrastructure.DependencyInjection (src/workflow/Aevatar.Workflow.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs:19). Without an import or qualification, the composition root cannot compile, so the natural owner of the invariant is not actually installed.

proportional-containment:
Essence: this is schema-first, least-privilege capability routing with zero-trust point-of-use admission; the natural owner is an Application policy that selects a typed contract while readiness stays a separate gate. Ideal would bind exact selection to producer-owned operation facts and leave only execution readiness to the readiness port. Six-smell comparison: the previous proxy-over-truth blocker is now contained because NyxID descriptors publish typed capability_key = endpoint.EndpointId, and exact matching requires descriptor.CapabilityKey to match both NormalizedCapability and CapabilityFingerprint (diff.patch:301, diff.patch:826). narrative-over-verification is addressed by tests proving typed-key match despite display-name mismatch and rejecting display-name proxy matching (diff.patch:1362). I see no blocking magic numbers, symptom branches, missing-inevitability, or skipped-purpose; the unavailable fallback remains a bounded Workstream D placeholder, not a second resolution system.

⟦AI:FKST⟧

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate failed: capability-key intent mismatch
Reproduce locally with scripts/run.sh test from the repository root.

@jason-aelf jason-aelf added fkst-dev:fixing review-rejected-and-fix-in-progress and removed fkst-dev:reviewing pr-review-consensus-in-progress labels Aug 9, 2026
…on integration and executable readiness handoff
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop fix pushed for re-review

Previous reviewed head: a068251
New head: 7caf9c5

@jason-aelf jason-aelf added fkst-dev:reviewing pr-review-consensus-in-progress and removed fkst-dev:fixing review-rejected-and-fix-in-progress labels Aug 9, 2026
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR is ready for review

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR review decision: approve
Verdicts: teleology=approve parsimony=approve fidelity=approve natural-ownership=approve proportional-containment=approve

teleology:
ESSENCE: this is schema-first, least-privilege capability routing with hexagonal/Application-owned orchestration and zero-trust readiness admission; its purpose is to prevent the model or Host from selecting execution capabilities by prose, labels, or fallback branching. IDEAL: a producer-owned operation key flows through the caller contract, the Application matches it against typed descriptors, then readiness/fallback are handled as typed outcomes. Six-smell comparison: the prior capability-key intent mismatch is closed, because the tool contract is narrowed to "capability_key" rather than "requested_capability" (diff.patch:155), NyxID descriptors now publish the producer key as CapabilityKey = endpoint.EndpointId (diff.patch:301), and exact matching uses descriptor.CapabilityKey against input.NormalizedCapabilityKey plus the fingerprint (diff.patch:845). That removes the decisive proxy-over-truth, missing-inevitability, and skipped-purpose smell: exact resolution is now forced by the authoritative key instead of display text or natural-language coincidence. I see no blocking magic numbers, symptom branches, or narrative-over-verification within the stated issue bounds.

parsimony:
The decisive root reason is that the prior capability-key intent mismatch is closed rather than papered over: the model-facing tool now requires the producer-owned capability_key instead of intent prose (diff.patch:155), NyxID descriptors publish that key from endpoint.EndpointId (diff.patch:301), and exact resolution only accepts descriptors whose normalized CapabilityKey matches NormalizedCapabilityKey and its fingerprint (diff.patch:847). Under schema-first, least-privilege capability routing, this is a clean enough narrowing: exact selection is no longer driven by display text, service cardinality, or natural-language coincidence.

fidelity:
Established practice is schema-first capability routing plus zero-trust readiness admission. The named capability-key intent mismatch is closed: the tool-facing contract now asks for capability_key, not natural-language requested_capability (src/Aevatar.AI.ToolProviders.Binding/Tools/DiscoverServiceApiWorkflowCapabilityTool.cs:30); the producer supplies that key from endpoint.EndpointId (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:566); and exact resolution verifies descriptor.CapabilityKey against input.NormalizedCapabilityKey before returning nyxid_operation (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:357). That is a justified narrowing to a producer-owned selector, so the prior proxy-over-truth blocker no longer holds.

natural-ownership:
Essence: this is schema-first, least-privilege capability routing with zero-trust readiness admission; the natural owner is the Application resolver, while producers own stable operation facts and prompts/tools consume typed results. The prior capability-key intent mismatch is closed by narrowing the caller contract to "capability_key" and removing "requested_capability" from the tool/DTO surface (diff.patch:155, diff.patch:1236), then matching exact descriptors only through producer-owned CapabilityKey = endpoint.EndpointId plus fingerprint rather than display text (diff.patch:301, diff.patch:845). Against the six-smell gate, I do not see a blocking proxy-over-truth, missing-inevitability, or skipped-purpose: tests cover same-service endpoint choice, display-name mismatch, unrelated descriptor miss, readiness handoff, retry identity reuse, and prompt delegation.

proportional-containment:
Schema-first, least-privilege capability routing is the governing practice here: exact selection must bind to producer-owned operation facts, while readiness remains a separate point-of-use gate. The named capability-key intent mismatch is closed because the model-facing contract is narrowed from requested_capability to required capability_key (diff.patch:155-diff.patch:170), the producer supplies that key from the endpoint identity (diff.patch:301), and exact matching now compares normalized descriptor.CapabilityKey against input.NormalizedCapabilityKey plus its fingerprint instead of display text (diff.patch:841-diff.patch:851). Tests also prove typed-key resolution despite display-name mismatch and reject display-name proxy matching, so I see no blocking proxy-over-truth, missing-inevitability, or narrative-over-verification regression inside the issue bounds.

⟦AI:FKST⟧

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate wait: ci-unknown

@jason-aelf jason-aelf added fkst-dev:merge-ready review-approved-and-ready-to-merge and removed fkst-dev:reviewing pr-review-consensus-in-progress labels Aug 9, 2026
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate wait: ci-unknown

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate wait: ci-unknown

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate wait: ci-unknown

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR head advanced after merge approval; re-entering review

Previous reviewed head: 7caf9c5
Current head: 6d0bc8f

@jason-aelf jason-aelf added fkst-dev:reviewing pr-review-consensus-in-progress and removed fkst-dev:merge-ready review-approved-and-ready-to-merge labels Aug 9, 2026
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR is ready for review

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop PR review decision: reject
Verdicts: teleology=approve parsimony=comment fidelity=reject natural-ownership=reject proportional-containment=reject
Blocking gap: missing namespace import

teleology:
ESSENCE: this is schema-first, least-privilege capability routing: the model/Host must not select executable service operations by prose, labels, or fallback branching; Application must select a typed producer-owned contract, then readiness must gate use. IDEAL: pass a producer operation key, match typed descriptors, return one terminal resolution or a typed handoff. Six-smell comparison: the settled blockers remain closed and the post-approval delta does not create a new blocking smell. discover_service_api_workflow_capability requires capability_key, not intent prose (src/Aevatar.AI.ToolProviders.Binding/Tools/DiscoverServiceApiWorkflowCapabilityTool.cs:30); the producer assigns CapabilityKey = endpoint.EndpointId (src/Aevatar.AI.ToolProviders.NyxId/ConnectedServices/NyxIdMcpOperationCatalog.cs:566); exact selection compares that key to NormalizedCapabilityKey before nyxid_operation (src/workflow/Aevatar.Workflow.Application/ExternalCapabilities/ServiceApiWorkflowCapabilityResolutionService.cs:357). No evidenced magic numbers, proxy-over-truth, symptom branches, narrative-over-verification, missing-inevitability, or skipped-purpose blocker remains within the stated issue bounds.

parsimony:
Established practice is schema-first, least-privilege capability routing with cohesive, single-purpose changes. ESSENCE: collapse model-owned Service API branching into one typed Application resolver. IDEAL: producer operation keys flow directly to exact selection, while readiness/fallback remain typed handoffs. Six-smell comparison: I do not see a stated-requirement blocker in the settled core; exact matching now compares descriptor.CapabilityKey with input.NormalizedCapabilityKey and the fingerprint (diff.patch:900). The parsimony concern is advisory under this review contract: the head advance also adds an unrelated schedule-provisioning branch with ScheduleProvisioningOneShotMinimumLeadTime = TimeSpan.FromSeconds(10) and preserves a fallback 30 second delay (diff.patch:9, diff.patch:51). That is magic numbers plus symptom branches, but it does not evidence failure of the Service API resolution requirement; delete or move that scheduling fix unless this PR is intentionally allowed to carry separate behavior.

fidelity:
ESSENCE: this is hexagonal, schema-first capability routing; the real thing to verify is not the model story but whether the typed Application port is actually installed from source. IDEAL: producer-owned capability_key selects the operation, readiness is a separate typed gate, and Infrastructure wires the adapters with compile-time-verifiable names. Six-smell result: the prior proxy-over-truth capability-key issue is not reopened; the decisive current failure is narrative-over-verification in composition. The DI file is in Aevatar.Workflow.Infrastructure.DependencyInjection (src/workflow/Aevatar.Workflow.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs:19), but the diff registers DeferredManagedServiceApiSkillDiscoveryPort by short name (diff.patch:1157) while declaring that adapter in Aevatar.Workflow.Infrastructure.ExternalCapabilities (diff.patch:1190). With no qualification/import in the patch, the source does not prove the new executable resolution path is wired; it cannot compile as written.

natural-ownership:
Established ports-and-adapters practice puts adapter installation in the composition root, and that root must compile before the Application-owned resolver can own anything at runtime. The patch still has ServiceCollectionExtensions registering DeferredManagedServiceApiSkillDiscoveryPort by unqualified short name (diff.patch:1157), while the new adapter is declared in Aevatar.Workflow.Infrastructure.ExternalCapabilities (diff.patch:1190) and the DI file is in Aevatar.Workflow.Infrastructure.DependencyInjection (src/workflow/Aevatar.Workflow.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs:19). With no import or qualification in the diff, this is narrative-over-verification: the proposal says Infrastructure composes the new resolution path, but the composition root cannot name the adapter.

proportional-containment:
Essence/ideal: under schema-first capability routing and ports-and-adapters, the Application may own resolution, but the Binding/Host boundary must return a workflow-authoring selector the model can copy without inventing translations. The blocking smell is skipped-purpose with narrative-over-verification: the new tool serializes the entire typed result via raw proto JSON and only runs NormalizeAuthoringAliases (diff.patch:261), while the prompt tells /api/chat to author an nyxid_request resolution’s returned admitted selector directly as capability.nyxid_request (diff.patch:1095). That is not proven executable: the workflow parser accepts authoring method values like GET/POST, not protobuf enum spellings (src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.cs:257), and the added Binding tool test covers only nyxid_operation. The prior capability_key issue is closed, but the nyxid_request handoff still leaves the model to bridge proto-to-YAML shape at the very boundary this proposal was meant to centralize.

⟦AI:FKST⟧

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop timeout redrive attempt: fixing 1

⟦AI:FKST⟧

@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop merge gate failed: missing namespace import
Reproduce locally with scripts/run.sh test from the repository root.

@jason-aelf jason-aelf added fkst-dev:fixing review-rejected-and-fix-in-progress and removed fkst-dev:reviewing pr-review-consensus-in-progress labels Aug 9, 2026
…on integration and executable readiness handoff
@jason-aelf

Copy link
Copy Markdown
Collaborator Author

github-devloop fix pushed for re-review

Previous reviewed head: 6d0bc8f
New head: 639ea01

@jason-aelf jason-aelf added fkst-dev:reviewing pr-review-consensus-in-progress and removed fkst-dev:fixing review-rejected-and-fix-in-progress labels Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fkst-dev:reviewing pr-review-consensus-in-progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant