feat: wire CLI commands and MCP tools to SDK#22
Merged
Conversation
Replace placeholder stubs in all 4 CLI commands (submit, status, list, get) and all 4 MCP tools (check_status, list_documents, get_document, lookup_taxpayer) with real SDK function calls. Create shared auth helpers for both CLI and MCP. Add prepublishOnly scripts and npm keywords to all packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR successfully wires CLI commands and MCP tools to the SDK, replacing all placeholder stubs with real API calls. The implementation creates shared auth helpers ( Major Changes:
Issues Found:
Test Coverage:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| packages/cli/src/utils/api-client.ts | New shared auth helper for CLI commands - creates authenticated HttpClient from profile config |
| packages/cli/src/commands/submit.ts | Wired submit command to SDK - signs XML and calls submitAndWait; redundant env var check on p12Pin |
| packages/mcp/src/tools/api-client.ts | New shared auth helper for MCP tools - similar to CLI version but returns only HttpClient |
| packages/mcp/src/tools/document-tools.ts | Replaced all placeholder responses with real SDK calls for check_status, list_documents, and get_document |
| packages/cli/src/commands/commands.spec.ts | Added basic arg-presence tests for new CLI flags; no integration tests for actual SDK calls |
| packages/mcp/src/server.spec.ts | Updated tests with SDK mocks; tools now expect auth errors instead of placeholder responses |
Sequence Diagram
sequenceDiagram
participant User
participant CLI/MCP
participant AuthClient
participant SDK
participant Hacienda API
User->>CLI/MCP: submit/status/list/get command
CLI/MCP->>AuthClient: createAuthenticatedClient(profile)
AuthClient->>SDK: loadConfig(profileName)
AuthClient->>SDK: TokenManager.authenticate(credentials)
SDK->>Hacienda API: POST /token (OAuth ROPC)
Hacienda API-->>SDK: access_token
AuthClient->>SDK: new HttpClient(envConfig, tokenManager)
AuthClient-->>CLI/MCP: { httpClient, config }
alt Submit Command
CLI/MCP->>SDK: signAndEncode(xml, p12Buffer, p12Pin)
SDK-->>CLI/MCP: signedXmlBase64
CLI/MCP->>SDK: submitAndWait(httpClient, request)
SDK->>Hacienda API: POST /recepcion
Hacienda API-->>SDK: submission response
SDK->>Hacienda API: GET /recepcion/{clave} (poll)
Hacienda API-->>SDK: status response
SDK-->>CLI/MCP: { accepted, clave, status }
else Status/List/Get Commands
CLI/MCP->>SDK: getStatus/listComprobantes/getComprobante
SDK->>Hacienda API: GET /recepcion/{clave}
Hacienda API-->>SDK: document/status data
SDK-->>CLI/MCP: parsed response
end
CLI/MCP-->>User: formatted output
Last reviewed commit: fcbc451
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+155
to
+157
| const p12Pin = (args.pin as string | undefined) ?? | ||
| config.p12Pin ?? | ||
| process.env["HACIENDA_P12_PIN"]; |
There was a problem hiding this comment.
redundant check: config.p12Pin already contains process.env["HACIENDA_P12_PIN"] (loaded in config-manager.ts:172), so the third fallback will never be used
Suggested change
| const p12Pin = (args.pin as string | undefined) ?? | |
| config.p12Pin ?? | |
| process.env["HACIENDA_P12_PIN"]; | |
| const p12Pin = (args.pin as string | undefined) ?? config.p12Pin; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/submit.ts
Line: 155-157
Comment:
redundant check: `config.p12Pin` already contains `process.env["HACIENDA_P12_PIN"]` (loaded in config-manager.ts:172), so the third fallback will never be used
```suggestion
const p12Pin = (args.pin as string | undefined) ?? config.p12Pin;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+166
to
+172
| try { | ||
| p12Buffer = await readFile(resolve(p12Path)); | ||
| } catch { | ||
| error(`Cannot read .p12 file: ${p12Path}`); | ||
| process.exitCode = 1; | ||
| return; | ||
| } |
There was a problem hiding this comment.
swallowed error prevents seeing actual file read failure (permissions, corrupted file, etc.)
Suggested change
| try { | |
| p12Buffer = await readFile(resolve(p12Path)); | |
| } catch { | |
| error(`Cannot read .p12 file: ${p12Path}`); | |
| process.exitCode = 1; | |
| return; | |
| } | |
| let p12Buffer: Buffer; | |
| try { | |
| p12Buffer = await readFile(resolve(p12Path)); | |
| } catch (err) { | |
| error(`Cannot read .p12 file: ${p12Path}: ${err instanceof Error ? err.message : String(err)}`); | |
| process.exitCode = 1; | |
| return; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/submit.ts
Line: 166-172
Comment:
swallowed error prevents seeing actual file read failure (permissions, corrupted file, etc.)
```suggestion
let p12Buffer: Buffer;
try {
p12Buffer = await readFile(resolve(p12Path));
} catch (err) {
error(`Cannot read .p12 file: ${p12Path}: ${err instanceof Error ? err.message : String(err)}`);
process.exitCode = 1;
return;
}
```
How can I resolve this? If you propose a fix, please make it concise.- Consolidate duplicated auth helpers into SDK as `bootstrapClient()` with runtime validation via Zod (replaces unsafe `as` casts) - Add MCP client caching per profile to avoid fresh ROPC auth per tool call - Fix Greptile findings: remove redundant env var fallback, include .p12 read error details, warn about PIN visibility in CLI args - Add NaN validation for CLI list --limit/--offset args - Add happy-path tests for MCP check_status, list_documents, get_document (5 new tests, total 29 MCP tests, 916 across all packages) - Fix Prettier formatting on all changed files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
submit,status,list,get) and MCP tools (check_status,list_documents,get_document,lookup_taxpayer) with real SDK function callsapi-client.ts) for both CLI and MCP packages — load profile, authenticate via TokenManager, return HttpClientprepublishOnlyscripts and npmkeywordsto all 4 packagesCLI changes
submit: signs XML with .p12, submits viasubmitAndWait(), shows accepted/rejected resultstatus: callsgetStatus(), displays status + rejection reasonlist: callslistComprobantes(), replaces hardcoded example dataget: callsgetComprobante(), displays full document details--profileflag;submitalso accepts--p12and--pinMCP changes
check_status,list_documents,get_document: authenticate via profile and call real SDK APIslookup_taxpayer: calls publiclookupTaxpayer()API (no auth needed)Tests
--profile,--p12,--pinTest plan
pnpm build— all 4 packages build cleanlypnpm test— 911 tests pass (shared: 113, SDK: 717, CLI: 57, MCP: 24)pnpm lint— cleanpnpm typecheck— clean🤖 Generated with Claude Code