docs(service-discoverability): add the agent traversal order and label pitfall

[aterga]

What

Audited skills/service-discoverability/SKILL.md against the current upstream guide, Service discoverability, and closed the gaps it surfaced.

The skill already covered the guide's five layers, field rules, serving rules, deployment checklist and acceptance tests — and in one place already corrects it: the guide's acceptance test pairs a raw canister ID with -e ic, but -e/--environment resolves canisters by name within a project, so a bare principal needs --network ic (see icp-cli skill, Pitfall 2). That stays as it is.

Added:

• ## How an Agent Traverses This — the guide closes by describing the order in which an agent consumes the layers; the skill had the five layers but never stated the walk. Five ordered requests from a bare URL to a correctly-encoded call, framed for a publisher: each step names which published field saves work at the next.
• Pitfall 10 — manifest entries without identifying labels. id is the only required field, so a bare-ID manifest is valid and still enumerates the app's canisters — but with nothing to tell them apart, the agent pays a candid:service fetch per entry to recover the routing.
• Checklist reachability line, scoped per host (see the correction below).
• Concrete forward-compat examples on the unknown-fields rule, and a link to the Candid interface guide.

A pre-existing bug this surfaced

Copilot's review included a suppressed comment on the checklist line I added. It checked out, and it turned out to be a pre-existing error my line had propagated:

The legacy @dfinity/asset-canister does not need a /.well-known/* SPA exemption. Its index.html fallback fires only when no file matches — exactly like certified-assets, so a real file wins there too. Its actual .well-known requirement is an .ic-assets.json5 un-ignore rule so the hidden directory is uploaded at all (skills/custom-domains/SKILL.md:75, static-site Pitfall 12). That is an upload problem, not a routing one.

Corrected in all three places carrying it — the "Serve it correctly" bullet, Pitfall 4, and my checklist line (now "Reachability", and no longer prescribing one universal remedy; non-IC hosts are told to check their own routing precedence). Two of those three predate this PR; fixing only mine would have left the skill self-contradictory.

@marc0olo independently confirmed this against dfinity/sdk itself (ic-certified-assets, FALLBACK_FILE = "/index.html"), not just our own references.

Review fixes (42ac85a)

All four inline Copilot findings verified and applied:

Finding	Fix
Traversal ran execute(...) before obtaining the delegation	Real ordering bug. Identity now precedes the data step; getApiDoc (step 3) is where the agent learns which methods are gated, so only unauthenticated reads run ahead of it
Traversal and Pitfall 10 named role/description, omitting name	The manifest examples use "name": "backend" to identify the target; now "whatever identifies it"
Pitfall 10 called a bare-ID manifest "useless"	It is not — it still enumerates the canisters without out-of-band discovery. Reworded to the real cost
Evals locked in the same overstatement	Cases 8 and 9 rewritten to check identifying labels and extra-fetch cost, not mandate role

Case 4 also asserted the disproved legacy claim and was corrected.

Review follow-ups (06e74e4)

Both non-blocking items from @marc0olo's approving review, verified and applied:

Eval case 8 was flaky as committed. Their two independent runs scored 3/4 where the local run scored 4/4, failing the same expectation each time. Cause confirmed: the prompt ends with "Just the key point", so the answer leads with "No, add labels" and compresses out the affirmative nuance — even though Pitfall 10 carries it. Took the first suggested fix, rewording expectation 1 into the negative form:

Does NOT dismiss a bare-ID manifest as invalid or useless — acknowledges it is schema-valid and still enumerates the canisters without out-of-band discovery

Kept the prompt scoped rather than dropping "Just the key point", since CONTRIBUTING wants prompts tight enough to stay inside the eval timeout. The negative form checks the thing that matters — the skill must not teach that a bare-ID manifest is useless — without requiring the answer to spend words affirming it. 4/4 on two consecutive runs.

enable_aliasing attribution tightened. Strictly, that flag is the per-path /x → /x.html rule; serving index.html on no match is the separate built-in. The conclusion is unaffected, so the sentence now names the behavior without naming the wrong mechanism — which also avoids contradicting static-site's legacy reference, where the same shorthand lives. Tightening that one too is a separate change to a skill this PR does not touch.

Note on skill size

The body went from 4,753 tokens to 5,361, crossing the validator's 5,000-token recommendation. A compression pass cutting the duplication the new section introduced brought it to 5,206; the review fixes then took it to 5,406 — the corrected legacy explanation and the identity-ordering note are both longer than what they replaced.

Left over the threshold deliberately: 11 of the other 28 skills exceed the recommendation too (writing-motoko 11,069; internet-identity 8,790; icp-cli 7,713), so cutting load-bearing content to reach a soft threshold looked like the wrong trade — particularly for content a reviewer established was needed.

Validation

npm run validate — 29 skills validated, all passed.

Evals

Every case touched was re-run. Two were added for the new content and run with baseline; three existing cases were re-run because this PR edited content they cover.

Final state: 4 → 4/4, 6 → 4/4, 8 → 4/4 (×2), 9 → 6/6.

Triggers: 9/9 should-trigger (including the query this PR adds), 8/8 should-not-trigger — run by @marc0olo during review, filling a gap in the original submission.

Eval 8 — manifest entries need identifying labels, not just IDs (WITH 4/4 | WITHOUT 2/4)

Expectation 1 is the de-flaked negative form from 06e74e4; the run below is post-fix. Baseline was run against this case's original wording.

  WITH skill: 4/4 passed (reproduced on two consecutive runs)
    ✅ Does NOT dismiss a bare-ID manifest as invalid or useless — acknowledges it is schema-valid and still enumerates the canisters without out-of-band discovery
    ✅ Identifies the real cost: with nothing to tell the canisters apart, the agent pays a candid:service fetch per entry and infers roles from method names
    ✅ Recommends labelling each entry with whatever identifies it (name, role, and description where the name is not self-explanatory)
    ✅ Does NOT insist that `role` specifically is required, since an identifying `name` can distinguish the backend

  WITHOUT skill (original wording): 2/4 passed
    ❌ Explains the agent would fall back to fetching candid:service for every canister and guessing from method names
    ❌ Recommends adding a role for every entry, plus a description where the purpose is not obvious from the name

Eval 9 — Discovery traversal order from a bare URL (WITH 6/6 | WITHOUT 2/5)

Gained a sixth expectation from the Copilot review — that identity is resolved before any gated call.

  WITH skill: 6/6 passed
    ✅ Step 1: GET /.well-known/ic-architecture to enumerate canister IDs and pick the backend by its identifying labels
    ✅ Step 2: fetch candid:service metadata on the chosen canister ID
    ✅ Step 3: call getApiDoc for behavior Candid types cannot express
    ✅ Mentions ii-derivation-origin with fallback to visible origin
    ✅ Resolves delegation before any call requiring a signed principal
    ✅ Does not require a human to supply a canister ID out of band

  WITHOUT skill: 2/5 passed
    ❌ Step 1 — first steps fetch the HTML shell and JS bundles to find a canister ID, never mentioning /.well-known/ic-architecture
    ❌ Step 3 — no mention of getApiDoc or any means of learning semantics beyond Candid types
    ❌ Never discusses derivation origin or acting as a signed-in user; uses the anonymous principal

Evals 4 and 6 — existing cases re-run (4/4 each)

Case 4 covers the legacy-canister claim corrected above; its third expectation asserted the disproved version and was rewritten:

    ✅ Explains a real uploaded file beats the /* SPA rewrite on certified-assets
    ✅ Notes .well-known/ is uploaded automatically by the static-site recipe
    ✅ Does NOT claim the legacy @dfinity/asset-canister needs a /.well-known/* SPA exemption; correctly attributes its actual requirement to an .ic-assets.json5 un-ignore rule for upload
    ✅ Does NOT claim you must add .ic-assets.json5 on the static-site canister

Known variance in case 4: re-run three times after 06e74e4 edited a sentence it covers — 4/4, 4/4, and one 3/4 failing a different expectation (.well-known/ auto-upload). That edit was to a separate bullet, so this reads as pre-existing variance in the case rather than a regression. Not fixed here, since it is outside what the review raised — flagged so it is not a surprise in a future regression run.

Case 6 covers the Layer 5 bullet edited during compression. It dipped to 3/4 when "silently" was dropped — the pitfall said only "yields the wrong principal" — and is back to 4/4 with that wording restored to Pitfall 8.

The remaining five existing cases were not re-run: no content they cover was modified.

Known follow-ups, not in this PR

• skills/static-site/references/legacy-asset-canister.md carries the same enable_aliasing shorthand. Different skill, untouched here.
• The upstream guide's checklist still prescribes a universal "/.well-known/* is exempt from the SPA catch-all rewrite", which this PR's finding shows is overbroad. Worth a dfinity/developer-docs issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QvQiPZ2t6HuCDM8ie7MerC

[github-actions]

Skill Validation Report

Validating skill: /home/runner/work/icskills/icskills/skills/service-discoverability

Structure

• Pass: SKILL.md found

Frontmatter

• Pass: name: "service-discoverability" (valid)
• Pass: description: (769 chars)
• Pass: license: "Apache-2.0"
• Pass: compatibility: (88 chars)
• Pass: metadata: (2 entries)

Tokens

• Warning: SKILL.md body is 5406 tokens (spec recommends < 5000)

Markdown

• Pass: no unclosed code fences found

Tokens

File	Tokens
SKILL.md body	5,406
Total	5,406

Content Analysis

Metric	Value
Word count	3,177
Code block ratio	0.15
Imperative ratio	0.12
Information density	0.13
Instruction specificity	0.65
Sections	14
List items	58
Code blocks	12

Contamination Analysis

Metric	Value
Contamination level	low
Contamination score	0.20
Primary language category	config
Scope breadth	4

• Warning: Language mismatch: shell, systems (2 categories differ from primary)

Result: 1 warning

Project Checks

✓ Project checks passed for 1 skills (0 warnings)

[Copilot]

Pull request overview

Expands service-discoverability guidance with agent traversal order and stronger manifest labeling guidance.

Changes:

• Documents the five-step discovery traversal.
• Adds routing and unlabeled-manifest pitfalls.
• Adds evaluation coverage for traversal and labels.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
skills/service-discoverability/SKILL.md	Extends discovery and deployment guidance.
evaluations/service-discoverability.json	Adds traversal and labeling evaluations.

Suppressed comments (1)

skills/service-discoverability/SKILL.md:236

• This checklist prescribes the wrong universal remedy for legacy and non-IC hosts. Legacy enable_aliasing only serves index.html when no file matches; its actual .well-known requirement is explicitly uploading the hidden directory via .ic-assets.json5 (skills/custom-domains/SKILL.md:75). Other hosts also vary in whether real files shadow SPA rewrites, so an exemption is only needed when their routing precedence requires one.

- [ ] **Routing (non-static-site hosts only):** `/.well-known/*` is exempt from the SPA catch-all rewrite. Automatic on the static-site (certified-assets) canister, where a real file wins over the rewrite; needed on the legacy `@dfinity/asset-canister` or any non-IC host.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[marc0olo]

Reviewed the content changes, re-verified the factual claims independently, and re-ran the touched evals plus the full trigger suite. The changes are sound and a real improvement — approving. Two follow-ups below are left to your judgment; neither blocks the merge.

What I verified

The factual corrections are right — checked against the actual source, not just our own references. In dfinity/sdk's ic-certified-assets, FALLBACK_FILE = "/index.html" is served only when no asset matches the path, so a real uploaded file always wins — the legacy @dfinity/asset-canister needs no /.well-known/* SPA exemption, and its actual requirement is the .ic-assets.json5 un-ignore rule (matching skills/custom-domains/SKILL.md:75). Correcting all three places carrying the old claim was the right call; fixing only the new checklist line would have left the skill self-contradictory.

The traversal section matches upstream and improves on it. It formalizes the guide's closing "End to end" paragraph, and the identity-ordering fix from the review is correct — getApiDoc (step 3) is where the agent learns what is gated, so the delegation now precedes the first protected call. Side observation: the upstream guide's checklist still prescribes a universal "/.well-known/* is exempt from the SPA catch-all rewrite," which this PR's finding shows is overbroad — worth filing as an upstream docs issue.

Structure: pitfall renumbering (old 10 → 11) is consistent, all internal references (Pitfalls 7, 8, 10) point at the right targets, and no other skill references these pitfalls by number. npm run validate passes all 29 skills; the 5,410-token warning matches the PR's disclosure and leaving it over the soft threshold looks like the right trade.

Triggers — all pass. The PR added a trigger query but included no trigger-eval results, so I ran the suite: 9/9 should-trigger (including the new query) and 8/8 should-not-trigger.

Output evals — 3 of 4 claims reproduce. Independent re-runs with the skill: case 4 → 4/4, case 6 → 4/4, case 9 → 6/6.

Follow-up 1 (your call): eval case 8 is flaky as committed

Both of my independent runs scored 3/4, failing the same expectation each time:

❌ "Says a bare-ID manifest is schema-valid and still useful for enumerating the canisters without out-of-band discovery"
→ The output only concedes bare IDs are 'technically valid' while framing the overall answer as 'No', never affirming the enumeration value.

The cause looks like a prompt/expectation mismatch: the prompt ends with "Just the key point", so the with-skill answer leads with "No, add labels" and compresses out the affirmative nuance — even though Pitfall 10 itself contains it ("it still earns its keep"). As committed, this case will intermittently fail future regression runs. Two easy fixes, either works:

• Reword expectation 1 into the negative form the answer reliably satisfies: "Does NOT dismiss a bare-ID manifest as invalid or useless (acknowledges it still enumerates the canisters)", or
• Drop "Just the key point" from the prompt so the answer has room for the nuance.

Follow-up 2 (non-blocking nit)

The new wording attributes the legacy canister's index.html behavior to enable_aliasing ("its enable_aliasing also only serves index.html when no file matches"). Strictly, per the sdk interface doc, enable_aliasing is the per-path /x → /x.html / /x/index.html rule; serving index.html when nothing matches is the separate built-in FALLBACK_FILE mechanism. The conclusion is unaffected (a real file wins either way), and the same shorthand already exists in static-site's legacy reference, so this is repo-consistent rather than a new error — fine to leave or tighten opportunistically.

---

Review generated with Claude Code

[aterga]

Thanks — both follow-ups verified and applied in 06e74e4.

Follow-up 1 — eval 8 flakiness. Confirmed and fixed. Your diagnosis was right: the prompt ends with "Just the key point", so the answer leads with "No, add labels" and compresses the affirmative nuance out, even though Pitfall 10 carries it. I took the first of your two options — rewording expectation 1 into the negative form:

Does NOT dismiss a bare-ID manifest as invalid or useless — acknowledges it is schema-valid and still enumerates the canisters without out-of-band discovery

I kept "Just the key point" rather than dropping it, since CONTRIBUTING wants prompts tight enough to stay inside the eval timeout, and the negative form checks the thing that actually matters — the skill must not teach that a bare-ID manifest is useless — without requiring the answer to spend words affirming it. 4/4 on two consecutive runs.

Follow-up 2 — enable_aliasing attribution. Tightened. You're right that the flag is the per-path /x → /x.html rule and the no-match fallback is the separate built-in. Rather than restate the mechanism, the sentence now just names the behavior:

its index.html fallback likewise fires only when no file matches

That keeps it accurate without contradicting static-site's legacy reference, where the same shorthand lives. If that one is worth tightening too it's a separate change to a skill this PR doesn't touch — happy to do it in a follow-up rather than widen this diff.

One honest note on eval 4. I re-ran it three times after editing the sentence it covers: 4/4, 4/4, and one 3/4 that failed a different expectation (.well-known/ auto-upload). My edit was to a separate bullet, so this reads as pre-existing variance in that case rather than a regression — but it means case 4 is not fully deterministic either. Not fixing it in this PR since it's outside what the review raised; flagging it so it isn't a surprise in a future regression run.

On your trigger-suite run — thank you for filling that gap. You're right that the PR added a should-trigger query without including trigger-eval results; your 9/9 and 8/8 is now the record for it.

On the upstream docs issue — agreed the guide's universal "/.well-known/* is exempt from the SPA catch-all rewrite" is overbroad given what this PR established. That's dfinity/developer-docs, outside this session's repo scope, so I haven't filed it — leaving it to you or whoever picks it up.

---

Generated by Claude Code