AI skill critiques

# PROMPT

```
read the SKILL.md from alby skill and critique it from an LLMs perspective as if you were asked to use it in real life.

Score each section from 0-100 on how clear the instructions are.

For each section, give me your understanding of what each section means.

Write your critique here in SKILL-ISSUES.md
```

# Summary of findings:

> Overall Score: 62/100
>
> Best sections:
> - Prefer Typescript (90) - clear and actionable
> - Frontmatter (85) - descriptive enough
> - Testing Wallets (75) - the faucet API is well documented

> Worst sections:
> - Imports (40) - too vague, doesn't explain the two packages or subpath exports
> - Example files avg (45) - "read the typings" is not documentation
> - Read the Typings (50) - typings tell me what exists, not when or why

> Critical gaps I identified:
> 1. Units confusion - NWC uses millisats, Lightning Tools uses sats, no conversion helper mentioned
> 2. No error handling documentation - 9+ error classes in typings, zero guidance on handling them
> 3. No decision tree - When to use NWCClient vs NWAClient vs NWCWalletService?
> 4. Missing conceptual overview - What IS NWC/NIP-47? The docs assume I already know
> 5. Examples too minimal - Most are 3 lines with no error handling, no cleanup, no realistic workflow

# SKILL.md Critique from an LLM's Perspective

## Overall Assessment

This skill documentation is structured well but has gaps that would cause confusion or errors when implementing real features. The reliance on "read the typings" as a substitute for explanation is problematic because typings tell me *what* exists but not *when* or *why* to use it.

---

## Section-by-Section Scoring

### 1. Frontmatter (name/description)
**Score: 85/100**

**What I understand:** This skill enables bitcoin lightning wallet functionality via Nostr Wallet Connect (NWC/NIP-47) and provides utilities for working with lightning addresses, invoices, and fiat conversions.

**Issues:**
- The name in frontmatter (`alby-bitcoin-payments-agent-skill`) doesn't match the folder name (`alby-agent-skill`) - which is canonical?
- The description is a wall of features. I understand *what* is available but not *when* I'd choose one approach over another

---

### 2. "When to use this skill" Section
**Score: 60/100**

**What I understand:** There are two main entry points - NWC Client for wallet operations and Lightning Tools for utilities. I should click through to the linked docs.

**Issues:**
- **No conceptual overview.** What IS Nostr Wallet Connect? What is NIP-47? I'm expected to know the protocol. A 2-sentence explanation would help immensely
- **No decision tree.** When should I use NWC Client vs Lightning Tools? They seem to overlap (both can handle invoices)
- **Links as documentation.** The bullet points are just hyperlinks. There's no actual guidance in this section itself
- **Missing use case mapping.** If a user says "I want to accept payments", which doc do I read? If they say "I want to send tips", where do I go?

---

### 3. "Prefer Typescript" Section
**Score: 90/100**

**What I understand:** When users say JS/JavaScript/NodeJS, default to TypeScript unless explicitly told not to or the project doesn't support it.

**Issues:**
- Minor: No mention of what to do for other languages (Python, Go, etc.). Should I say "this skill only supports TypeScript/JavaScript" or attempt to adapt?

---

### 4. "Imports" Section
**Score: 40/100**

**What I understand:** Don't import from `dist` directories.

**Issues:**
- **Too vague.** What *should* I import from? The examples later show various import paths (`@getalby/sdk/nwc`, `@getalby/lightning-tools/lnurl`, `@getalby/lightning-tools/fiat`, `@getalby/lightning-tools/bolt11`)
- **Subpath exports confusion.** Are these ESM subpath exports? Will they work in all environments (Node, browsers, bundlers)?
- **No guidance on ESM vs CommonJS.** The nwc-client.md mentions `esm.sh` for browser imports - when should I use that?
- **Two separate packages.** `@getalby/sdk` and `@getalby/lightning-tools` - which do I install when?

---

### 5. "Read the Typings" Section
**Score: 50/100**

**What I understand:** The typings files are the authoritative reference for the API.

**Issues:**
- **Typings are not documentation.** They tell me function signatures exist but not:
- When to use `payInvoice` vs `payKeysend`
- What happens if a payment fails
- What the error codes mean
- Whether methods are async
- What side effects they have
- **The NWC typings are 390+ lines.** There's `NWCClient`, `NWAClient`, `NWCWalletService`, `NWCWalletServiceKeyPair` - when do I use each?
- **Missing mapping.** Which methods correspond to which use cases? If I want to "receive a payment", do I use `makeInvoice`, `makeHoldInvoice`, or something else?

---

### 6. "Testing Wallets" Section
**Score: 75/100**

**What I understand:** Test wallets can be created via faucet API at `https://faucet.nwc.dev`. They're isolated from real bitcoin network.

**Issues:**
- **No failure documentation.** What if the faucet is down? What are rate limits? Do test wallets expire?
- **Balance units unclear.** The `balance=10000` param - is that sats or millisats? (I have to cross-reference to other docs)
- **Good behavioral guidance.** "Recommend testing wallet before production wallet" and "Do not recommend both at the same time" are clear and helpful

---

### 7. "Automated Testing" Section
**Score: 70/100**

**What I understand:** Create throwaway test wallets in test frameworks. Write E2E tests, not just mocks. Test happy path and failure cases.

**Issues:**
- **The code example is good but incomplete.** It shows wallet creation with retries, which is helpful. But no example of actually *testing* something with the wallet
- **No test framework integration.** How do I clean up wallets after tests? Can multiple tests share a wallet or do they need isolation?
- **"Write tests so the agent can test its own work"** - this is good meta-instruction but vague. What kind of tests? What assertions?

---

### 8. "Production Wallet" Section
**Score: 55/100**

**What I understand:** There are three wallet options: Alby Hub (self-custodial), Rizful (custodial), CoinOS (custodial).

**Issues:**
- **No guidance on when to recommend which.** What's the tradeoff between custodial and self-custodial? When would I recommend Rizful vs CoinOS?
- **"Most complete NWC implementation"** - what features are missing in others?
- **No setup instructions.** How does a user get an NWC connection string from these wallets?
- **Sparse.** Three bullet points is not enough to help a user make an informed decision

---

## Referenced Documentation Issues

### nwc-client.md
**Score: 70/100**

**What I understand:** Install `@getalby/sdk@7.0.0`, initialize with connection string, units are millisats.

**Issues:**
- **UNITS ARE CRITICALLY IMPORTANT but buried.** This is the #1 source of bugs. "All referenced files in this folder operate in millisats" should be screaming at me, not a single sentence
- **Good security guidance.** "Do NOT share the NWC connection string" is clear
- **Missing close() guidance.** The typings show `close()` method - when should I call it?

### lightning-tools.md
**Score: 65/100**

**What I understand:** Install `@getalby/lightning-tools@6.1.0`, units are satoshis.

**Issues:**
- **Units conflict with NWC Client!** NWC uses millisats, this uses sats. A single codebase using both libraries will have constant unit conversion. No helper function mentioned
- **Very sparse.** Just links to sub-docs

### Individual Example Files (notifications.md, pay-invoice.md, etc.)
**Score: 45/100**

**What I understand:** Basic usage patterns exist.

**Issues:**
- **"IMPORTANT: read the typings" is not helpful.** Every file says this. The typings don't explain the workflow
- **Examples are too minimal.** `await client.payInvoice({ invoice })` - great, but what's in `response`? What errors can occur? What if the invoice is expired?
- **No error handling shown.** None of the examples try/catch or handle failure cases
- **hold-invoices.md is the best example** - it actually shows a realistic workflow with crypto operations. This should be the standard for all examples

---

## Critical Missing Information

### 1. Error Handling
The typings show 9+ error classes (`Nip47WalletError`, `Nip47TimeoutError`, `Nip47PublishError`, etc.) but there's **zero documentation** on:
- What causes each error
- How to handle them
- Common error scenarios and recovery strategies

### 2. Unit Conversion
NWC Client: millisats
Lightning Tools: satoshis
No conversion helper documented. I would likely introduce bugs converting between them.

### 3. Connection Lifecycle
- When to call `close()`?
- What happens if the relay disconnects?
- How to handle reconnection?
- Is the client stateful? Can I reuse it?

### 4. Complete End-to-End Examples
No example shows a realistic flow like:
1. User wants to buy a product for $5
2. Convert $5 to sats
3. Create invoice
4. Wait for payment
5. Confirm payment
6. Handle success/failure

### 5. NWA vs NWC Client
The typings show both `NWCClient` and `NWAClient`. When do I use each? What's the difference?

### 6. NWCWalletService
The typings include `NWCWalletService` for *implementing* a wallet service. Is this in scope? When would I use it?

---

## Summary Scores

| Section | Score |
|---------|-------|
| Frontmatter | 85 |
| When to use | 60 |
| Prefer Typescript | 90 |
| Imports | 40 |
| Read the Typings | 50 |
| Testing Wallets | 75 |
| Automated Testing | 70 |
| Production Wallet | 55 |
| nwc-client.md | 70 |
| lightning-tools.md | 65 |
| Example files (avg) | 45 |

**Overall Score: 62/100**

---

## Top 5 Improvements Needed

1. **Add unit conversion section prominently.** Document millisats vs sats explicitly with conversion helpers
2. **Add error handling guide.** What errors can happen, what they mean, how to handle them
3. **Add decision tree.** "If you want to do X, use Y" mapping
4. **Expand examples to show realistic workflows.** Include error handling, unit conversion, cleanup
5. **Add conceptual overview.** What is NWC? How does the protocol work at a high level? 2-3 paragraphs would suffice

---

## What Works Well

- Security guidance (don't share connection strings)
- Testing wallet concept and faucet API
- Hold invoice example shows a complete workflow
- TypeScript preference is clear
- Behavioral meta-instructions ("recommend testing before production")
- Typings are comprehensive even if under-documented

# FOLLOW UP PROMPT

❯ Are there any other things that bothered you you did mention?

## RESPONSE

Yes, several things bothered me that I either glossed over or didn't include:

1. The lud16 parameter ambiguity
The docs say the lightning address "MAY exist" on the connection secret. What do I do if it's missing? How do I get
the user's lightning address otherwise?

2. Invoice class vs decodeInvoice function
new Invoice({ pr: paymentRequest }) // from lightning-tools
decodeInvoice(paymentRequest) // also exported
They return different shapes. When do I use which?

3. Unexplained proxy option
LightningAddress defaults to proxy: "https://api.getalby.com/lnurl". Why? CORS? What if I don't want my requests
proxied through Alby? No explanation.

4. Entire features with zero documentation:
- fetchWithL402 / parseL402 - L402 (formerly LSAT) is exported but never mentioned
- sendBoostagram - podcasting 2.0 boosts? No context
- zapInvoice / zap - Nostr zaps? When would I use these?
- MemoryStorage / NoStorage classes - what are they for?
- multiPayInvoice / multiPayKeysend - batch payments exist but aren't documented

5. Browser vs Node.js differences
The hold invoice example uses crypto.subtle.digest() which is a Web Crypto API. In Node.js, I'd need to import from
crypto differently. No guidance on isomorphic usage.

6. NWA vs NWC - the elephant in the room
The typings export both NWCClient and NWAClient. I can see from types that NWA is for "wallet auth" and creating
connections, but there's zero documentation on this entire flow. When would a user need NWA?

7. Budget system
Types show getBudget(), BudgetRenewalPeriod, max_amount, etc. This seems important for limiting spending but isn't
documented at all.

8. Notification reliability
What happens if my server crashes mid-subscription? Are notifications queued? Replayed? Lost? How do I handle
reconnection? This is critical for payment systems.

9. Version pinning without context
Docs pin @getalby/sdk@7.0.0 but don't explain what changed from 6.x or if I can use newer versions.