feat(skills): add smoke health-check command to int-* skills#95
Open
mt-alarcon wants to merge 1 commit into
Open
feat(skills): add smoke health-check command to int-* skills#95mt-alarcon wants to merge 1 commit into
smoke health-check command to int-* skills#95mt-alarcon wants to merge 1 commit into
Conversation
…/youtube
Add a uniform `smoke` connectivity command to three integration skills.
The command runs auth + a representative read-only step, always exits 0,
and emits structured JSON so callers (CI, heartbeats, manual validation)
can distinguish PASS from FAIL without parsing stderr or relying on exit
codes that crash on failure.
Contract:
{"overall": "PASS"|"FAIL",
"steps": [{"step": "...", "status": "PASS"|"FAIL", "error": "...", "duration_ms": N}],
"duration_ms": N}
- Always exits 0, even on failure (missing creds report overall=FAIL, never crash)
- Each step wrapped in try/except; errors captured into the `error` field
int-stripe, int-linkedin and int-youtube are clean reference implementations.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideImplements a standardized Sequence diagram for standardized smoke health-check commandsequenceDiagram
actor Caller
participant Int_skill as int_*_script
participant External_API
Caller->>Int_skill: run smoke
activate Int_skill
Int_skill->>Int_skill: [auth step]
alt auth fails
Int_skill-->>Caller: JSON {overall: FAIL, steps: [auth FAIL], duration_ms}
Int_skill->>Caller: exit code 0
else auth passes
Int_skill->>External_API: read-only request
External_API-->>Int_skill: response or error
Int_skill->>Int_skill: [evaluate API result]
Int_skill-->>Caller: JSON {overall: PASS or FAIL, steps: [...], duration_ms}
Int_skill->>Caller: exit code 0
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
smokeimplementations are inconsistent across skills (Stripe wraps the whole routine in aBaseExceptionhandler and guarantees JSON output, whereas LinkedIn/YouTube can still raise and exit non‑zero), which can violate the stated contract thatsmokealways exits 0; consider wrapping the LinkedIn/YouTube blocks in a similar top-level try/except to normalize behavior. - The
smokecommands currently diverge slightly from the documented contract (e.g., Stripe returns extra fields likecurrency, LinkedIn/YouTube pretty-print JSON and callsys.exit(0)inline); it might be cleaner to centralize or standardize the JSON schema and exit behavior so allint-*skills behave identically. - The Stripe
smokedocstring and error messages contain Portuguese phrases while the rest of the file is in English; aligning language and style (and using a consistent time source, e.g.,time.monotonic()vstime.time()) across the newsmokeimplementations would improve readability and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `smoke` implementations are inconsistent across skills (Stripe wraps the whole routine in a `BaseException` handler and guarantees JSON output, whereas LinkedIn/YouTube can still raise and exit non‑zero), which can violate the stated contract that `smoke` always exits 0; consider wrapping the LinkedIn/YouTube blocks in a similar top-level try/except to normalize behavior.
- The `smoke` commands currently diverge slightly from the documented contract (e.g., Stripe returns extra fields like `currency`, LinkedIn/YouTube pretty-print JSON and call `sys.exit(0)` inline); it might be cleaner to centralize or standardize the JSON schema and exit behavior so all `int-*` skills behave identically.
- The Stripe `smoke` docstring and error messages contain Portuguese phrases while the rest of the file is in English; aligning language and style (and using a consistent time source, e.g., `time.monotonic()` vs `time.time()`) across the new `smoke` implementations would improve readability and maintainability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
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.
What
Adds a uniform
smokeconnectivity command to integration skills, with three clean reference implementations: int-stripe, int-linkedin, int-youtube.Why
There's currently no standard way to ask an
int-*skill "are you connected and working?" in a machine-readable way. Ad-hoc connectivity checks (where they exist) log to stderr and can crash or return non-zero, so a caller (CI step, scheduled heartbeat, or a human validating a fresh install) can't reliably distinguish "all good" from "auth failed".Contract
smokealways exits 0 and prints structured JSON:{ "overall": "PASS" | "FAIL", "steps": [ {"step": "auth", "status": "PASS", "duration_ms": 123}, {"step": "list_charges", "status": "FAIL", "error": "401 Unauthorized", "duration_ms": 45} ], "duration_ms": 168 }overall: "FAIL"but never crash and never return non-zero. This lets a caller key off the JSON, not the exit code.errorfield.auth/client init, (2) a cheap representative read-only call.Scope
Three skills as reference implementations of the convention. The pattern is intentionally simple so it can be applied incrementally to the rest of the
int-*family in follow-ups.Testing
Each command verified locally —
python3 <script> smokereturns exit 0 with valid JSON containing theoverallfield, both with valid credentials (overall: PASS) and without (overall: FAIL, no crash).🤖 Generated with Claude Code
Summary by Sourcery
Add a standardized JSON-based smoke health-check command to selected integration skills to report connectivity status without relying on exit codes.
New Features:
smokecommand for the Stripe integration skill to validate API key presence and perform a balance read-only check, always returning structured JSON and exit code 0.smokecommand to the LinkedIn integration skill that checks account auth and performs a profile read to report health via structured JSON.smokecommand to the YouTube integration skill that validates account auth and performs a lightweight channel stats read, emitting structured JSON health results.