Skip to content

Commit ff08616

Browse files
samarthya-gupta1pramodh-ayyappanclaudeanshulsao
authored
Feat/praxis onboarding embedded skill (#13)
* refactor(render): lift executionPreamble into internal/render Centralises the execution-preamble constant in internal/render/preamble.go as the exported render.ExecutionPreamble so the upcoming agentcatalog package can share it without duplication or circular imports. Wording lightly generalised ("this" instead of "this skill", praxis login instead of install-skill/refresh-skills). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(harness): add AgentDir per-host for subagent install * feat(agentcatalog): Agent wire shape + PrefixedName Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(agentcatalog): Fetch from /custom-agents + /subagents with filtering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(agentcatalog): per-harness rendering (YAML for Claude/Gemini, TOML for Codex) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(skillinstall): Receipt.Agents field with backward-compat load * feat(agentinstall): install/uninstall/list/upsert with shared receipt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(login): step 3.5 fetch and install agents alongside skills Wire agentcatalog.Fetch + agentinstall.Install into runPostAuthSetup between the catalog-skills step and MCP snapshot refresh. Same fail-safe contract as skills: fetch failure leaves existing agents on disk. Adds four seams (fetchAgents, installAgents, uninstallAgentsByPrefix, listInstalledAgents) plus two new tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(login): include agents in --json envelope * feat(cmd): praxis agents listing command Add `praxis agents` — a read-only command that lists all Praxis-sourced agent/subagent files from ~/.praxis/installed.json, with pretty table and auto-JSON (non-TTY) output modes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(agents): preserve JSON contract on empty receipt * feat(status): include agents_installed in JSON + pretty output * feat(meta-skill): teach the AI host about installed subagents * docs: README + CHANGELOG for agent sourcing * test(agentinstall): cover empty-prefix, upsert, corrupt-receipt, normalize-kind paths * fix(agents,status): nil-to-[] normalize + UninstallByPrefix batch-continue Two follow-up fixes from final code review: - cmd/status.go: normalize nil slices to []Installation{} / []AgentInstallation{} before they reach the JSON map so the envelope marshals as [] instead of null. Matches the contract praxis agents --json already upholds; AI host parsers no longer have to handle two empty shapes. - internal/agentinstall.UninstallByPrefix: aggregate per-file remove errors instead of short-circuiting on the first failure. The batch now always completes, the receipt is always saved (with un-removable entries kept so a retry can finish the job), and aggregated errors surface at the end. * fix(agentcatalog): tolerate 404 from one endpoint Older Praxis deployments expose /custom-agents but not /subagents. Treating 404 as a hard failure aborted the entire agent install on those deployments, leaving users with zero agents even when the catalog had active custom agents to install. Now fetchOne returns (nil, nil) on 404 so the merge step continues with whatever the other endpoint returned. Auth (401/403) and server errors (5xx) still fail hard. Verified live against a deployment where /custom-agents has one agent and /subagents 404s — the agent installs across all three detected hosts (.md for Claude/Gemini, .toml for Codex). * chore: gitignore pre-existing cmd/praxis* test leak A test in cmd/ writes catalog skill files relative to its working directory instead of to t.TempDir(), polluting the repo with /cmd/praxis*/ folders that contain copies of the real installed skills. Ignore the leak so it doesn't show up in 'git status' on every test run. Tracking the underlying test-isolation bug separately. * test(login_setup): genuinely verify agent preservation on fetch failure TestRunPostAuthSetupAgentFetchFailureLeavesExistingInPlace previously only asserted that state.agents was empty after a failed fetch — which is trivially true when no agent was on disk to begin with. The test was named for the fail-safe contract ("leaves existing in place") but never actually checked the contract. Now the test: - Pre-seeds a praxis-preserve-me agent via agentinstall.Install (writes the receipt + the on-disk file) - Calls runPostAuthSetup with the fetch stub returning an error - Asserts state.agents stays empty (no new install) - Asserts agentinstall.List() still contains the seeded entry - Asserts the on-disk file still exists Existing code already passes — this is a test-only strengthening. * test(status): parse JSON in agents_installed assertion TestStatusCmd_IncludesAgentsInstalled was matching the literal substring `"agents_installed"` in the raw output buffer. That would false-pass if the string ever appeared in a log line, error message, or any other quoted context — not actually verifying the JSON envelope's shape. Now the test json.Unmarshals the output into a map and asserts the key exists. Also fatals if the output isn't valid JSON at all, which the original substring match would have silently allowed. * test(agentcatalog): pin specific error contents in Fetch failure cases Three Fetch failure assertions only checked err != nil, which would pass even if the wrap routing was wrong or the inner cause got swallowed: - TestFetchPartialFailureFailsHard — also pin "fetch subagents" (which endpoint surfaced the error — catches a regression that mis-routes a subagents-side failure as a custom-agents wrap) and "HTTP 500" (catches a regression that drops the inner status from the wrapped chain). - TestFetchRequiresBaseURLAndToken — pin "baseURL is required" and "token is required" so each guard's error names the missing field. Substring match (not errors.Is) because the implementation uses fmt.Errorf with literal messages, not sentinel errors. * test(agentcatalog): assert Render success in subagent-prefix test TestRenderSubagentUsesSubPrefix discarded the error from a.Render("claude-code") with `_`, then asserted on the returned string. A regression making Render return an empty string and a non-nil error would still pass the strings.Contains check on the positive side and t.Errorf on the negative side, but never surface the actual cause. Now the test captures the error, t.Fatalf-s if Render fails, and only asserts on out when Render succeeded — so any future regression in Render's success path surfaces with a clear message. * test(agentinstall): assert Install success in TestListReturnsReceiptEntries The seed Install in this test discarded both return values with _, _ = Install(...). A regression that broke Install entirely would have surfaced as the downstream "want 3 entries, got 0" assertion, pointing at the wrong cause. Now t.Fatalf on Install error so a setup failure halts the test immediately with the actual error message. * fix(agentinstall): atomic write for agent files (temp + sync + rename) Install used os.WriteFile, which opens with O_CREATE|O_TRUNC. Truncation happens before any bytes are written, so a crash or interruption mid-write would leave behind an empty or partial file — and a concurrent reader (e.g. the AI host's subagent loader scanning the directory at the same moment) could observe that intermediate state. Replaced with a small atomicWriteFile helper that mirrors the temp+chmod+rename pattern saveReceipt already uses in the same file. Sequence: CreateTemp in the same dir (so rename is on the same filesystem), Write + Sync + Close, Chmod, then atomic Rename into place. A deferred cleanup removes the temp file on any error before the rename. No behavior change on the happy path. The only observable difference is on failure paths: previously a half-written file could be left on disk; now it cannot. saveReceipt is still called only after all per-host writes succeed (existing structure — Install returns early without saving the receipt on any write failure), so the receipt and disk stay consistent. * test(harness): use withIsolatedHome in TestAllHarnessesHaveAgentDir The test was the only one in the file calling os.UserHomeDir() directly, making it host-dependent: a developer with a non-default HOME, or CI in a sandbox that sets HOME unexpectedly, would see spurious mismatches. Every other test in the file already uses the withIsolatedHome(t) helper which sets HOME to a t.TempDir(). Switched to the same helper. Test logic is unchanged — iterates All(), compares h.AgentDir against the expected per-host paths under the isolated home. * test(render): cap preamble preview slice to avoid out-of-bounds panic The failure-message preview used p[:80] which would panic when ExecutionPreamble is shorter than 80 bytes — exactly the case where the HasPrefix check fails (e.g. constant accidentally emptied/truncated). Losing the real test failure to a slicing panic inside t.Fatalf would bury the actual problem. Cap the preview via min(len(p), 80) — built-in on Go 1.21+ (go.mod says 1.24.2). * style(skillinstall): gofmt — align AgentInstallation field tags CI's gofmt check flagged extra spaces around the `json:"kind"` struct tag. Original author wrote it aligned with the `json:"installed_at"` block below it, but gofmt strips that alignment because the comment ` // "agent" | "subagent"` follows. Equivalent fix shipped previously in a docs commit that got dropped during the v0.9.0 -> v0.10.0 rename; resurfacing it here as a standalone style fix. * refactor(agentcatalog): drop /ai-api/subagents consumption Subagents are being removed server-side; shipping the second endpoint and the prefix/Kind machinery just to delete it later isn't worth the surface area. Stripped now. Removed: - Second goroutine fetching /ai-api/subagents, the WaitGroup, the subagentsOut/subErr locals — Fetch is now a single synchronous call to fetchOne(customAgentsPath). - KindSubagent, PrefixSubagent, subagentsPath constants. - PrefixedName's subagent branch — always returns "praxis-" + name. - The parent-bound subagent filter (ParentAgentName != ""). - Agent.ParentAgentName field (only ever read by the filter). - TestRenderSubagentUsesSubPrefix and TestInstallSubagentUsesSubPrefix. - The subagent half of the old TestFetchMergesAndFilters — replaced by TestFetchFiltersInactive over a single endpoint. - The "praxis-sub-" prefix half of TestUninstallByPrefix — renamed to TestUninstallByPrefixRemovesAllAgents and now installs two agents. Kept: - Agent.Kind field (value always "agent" today) for forward-compat with the receipt schema if another resource type is ever introduced. - AgentInstallation.Kind in the receipt for the same reason. - The KIND column in praxis agents output. Updated: - TestFetchPartialFailureFailsHard renamed to TestFetchServerErrorFails; now tests a single endpoint returning 500 and pins the wrapped error to "fetch custom-agents" + "HTTP 500". - TestFetchTolerates404FromOneEndpoint renamed to TestFetchTolerates404; /custom-agents returning 404 now means "no agents installed" rather than "use the other endpoint's response." - README and meta-skill body: drop subagent paragraph; bullet for praxis agents now just mentions custom agents and the praxis- prefix. - CHANGELOG v0.10.0: drop subagent mentions; add atomic-write note. Validated: go build clean, go test -race ./... all 13 packages pass, gofmt clean. * feat(agentinstall): gate agents to Claude Code only for v1 Runtime smoke against Gemini CLI and Codex showed neither host actually loads what their docs say they load: - Codex: praxis-<n>.toml files written to ~/.codex/agents/ matched the documented schema (name, description, developer_instructions = """...""" — exact form from the Codex subagents page) but the Codex CLI did not surface the agents at all. (The docs do warn: "the format may evolve as authoring and sharing mature.") - Gemini CLI: ~/.gemini/agents/*.md with YAML frontmatter matches the documented loader path, but installed praxis agents did not appear in `/agents` during runtime smoke. Until those host loaders catch up, writing files no loader picks up is worse than not writing them — silent no-ops mislead users who think `praxis agents` listing reflects available dispatch. Added supportsAgentInstall(harnessName) gate in agentinstall.Install. Returns true only for claude-code today. The renderer and extensionFor logic still understand all three hosts; flipping supportsAgentInstall re-enables full 3-host fan-out once verified. Updated tests: - TestInstallFansOutToAllHarnesses renamed to TestInstallOnlyClaudeCodeV1. Asserts only 1 install (claude), plus negative assertions that gemini/codex agent dirs were NOT written to. - TestUninstallByPrefixRemovesAllAgents: 2 entries (2 agents × 1 host) instead of 6. - TestListReturnsReceiptEntries / TestInstallUpsertsExistingEntry: 1 entry instead of 3. Updated docs: - CHANGELOG v0.10.0: drop 3-host claim; add "Host scope — v1 ships Claude Code only" section citing the runtime evidence. - README: drop Gemini/Codex from the agents bullet; note they're gated until their loaders consume the files. - Meta-skill body (dummy.go): drop the Gemini/Codex bullets from the Agents section. - `praxis agents --help` Long description: same. No server changes. No credential changes. UninstallByPrefix is unchanged — it works against receipt entries, so any older install that had written to gemini/codex would still get cleaned up on profile switch. * feat(agentinstall): also enable Gemini CLI (Codex still gated) Correction to 3f7b880. After that commit's runtime-only-Claude narrative, follow-up smoke against Gemini CLI confirmed it DOES load agents from ~/.gemini/agents/*.md — installed praxis agents surface via @<name> invocation and the agent's tool calls fire end-to-end. The earlier Gemini "unverified" reading misattributed unrelated skill-conflict warnings to the agent path; those warnings come from skills getting scanned from both ~/.gemini/skills/ AND ~/.agents/skills/, a pre-existing dupe that has nothing to do with agents. supportsAgentInstall now returns true for both claude-code and gemini-cli. Codex stays gated (its runtime still doesn't surface ~/.codex/agents/*.toml despite the TOML matching what the Codex subagents docs prescribe). Updated: - TestInstallSupportedHostsOnly (was TestInstallOnlyClaudeCodeV1): asserts 2 installs, both .md, plus negative assertion that Codex's .toml was NOT written. - TestUninstallByPrefixRemovesAllAgents: 4 removed (2 agents × 2 supported hosts) instead of 2. - TestListReturnsReceiptEntries / TestInstallUpsertsExistingEntry: 2 entries instead of 1. - README, CHANGELOG v0.10.0, meta-skill body, `praxis agents` Long description: Claude Code + Gemini CLI as supported targets, Codex called out as gated with a one-line flip path. * feat(agentinstall): logout cleans agents + orphan sweep on refresh Two follow-ups for full agent lifecycle parity with skills, both surfaced during live testing: 1. `praxis logout` was only calling `skillinstall.UninstallByPrefix` — agents added by this PR were left orphaned in the receipt + on disk after logout. Now logout also calls `agentinstall.UninstallByPrefix("praxis-")` and reports `removed_agents` in the JSON envelope. 2. `agentinstall.RemoveOrphanedByPrefix(prefix, hosts, keep)` is the agent analog of `skillinstall.RemoveOrphanedByPrefix`. Wired into the runPostAuthSetup agent step after the install: any praxis-* file in a host's AgentDir that isn't in the freshly-installed keep set is removed. Cleans up: - Files left by older praxis-cli versions - Files at hosts that are now gated (e.g. existing Codex ~/.codex/agents/praxis-*.toml from pre-gate installs) Tests: - TestRemoveOrphanedByPrefix in installer_test.go verifies the keep/receipt logic and per-host file removal across all 3 detected hosts including Codex. No behavior change for existing agents on Claude Code or Gemini CLI — both still install via the supported-host gate. Codex files left over from earlier installs get swept automatically next time the agent fetch succeeds. * test: pin error contents + isolate pretty formatter + fsync via shared helper Six review fixes — all minimal, all touch only the surfaces flagged. cmd/agents_test.go - TestAgentsCommandPretty was named "Pretty" but ran through cobra RunE with a bytes.Buffer, which UseJSON correctly resolves to non-TTY → JSON. Substring assertions on "praxis-alpha" / "agent" passed against JSON too — couldn't catch a pretty→JSON regression. - Replaced with TestPrintAgentsPretty that calls printAgentsPretty directly. Asserts non-JSON prefix + column headers + data rows. Bypasses TTY detection by testing the formatter in isolation. internal/agentcatalog/agentcatalog_test.go - TestRenderTOMLRejectsTripleQuoteInPrompt: also pin "triple-quote sentinel" in error to catch a regression that returned a different error for this input. - TestRenderUnknownHarness: also pin "unsupported harness". internal/agentinstall/installer_test.go - TestUninstallByPrefixRejectsEmptyPrefix: also pin "non-empty" in the error. - TestListRejectsCorruptReceipt: also pin the receipt path AND "parse" so the wrap is verified to name the file and preserve the underlying cause. cmd/agents.go - printAgentsPretty dropped its always-nil error return. Caller in agentsCmd.RunE updated. fmt.Fprintf write failures aren't actionable for a terminal-only list command; doc comment notes the precedent (list-skills behaves identically). cmd/logout.go - Skill + agent UninstallByPrefix errors were suppressed when asJSON was true — JSON consumers couldn't tell "no agents" from "removal failed". Both branches (--all and active-profile) now collect errors into a `warnings []string` slice and surface them via a `warnings` field on the JSON envelope. internal/agentinstall/installer.go - saveReceipt was hand-rolling temp+rename WITHOUT fsync — a crash between Write and Rename could persist metadata without data. The atomicWriteFile helper already has the correct pattern (Write + Sync + Close + Chmod + Rename with defer-cleanup). saveReceipt now delegates to atomicWriteFile; the docstring claim about "same pattern" is now accurate. Validated: go test -race ./... clean across 13 packages; gofmt clean. * feat(skillinstall): embedded multi-file skills + praxis-onboarding Adds support for binary-embedded multi-file (tree) meta-skills via a new InstallTree path over a go:embed fs.FS, then ships praxis-onboarding as the first such skill: an engine + flow registry whose first flow walks a new Praxis adopter through one end-to-end Facets loop (modules → tweak → project → env → real release → teardown), with tiered safety (HARD GATE for billable/destructive/sandbox; SOFT for free/reversible). skillinstall changes are additive — the single-file ContentFor path is untouched. IsMetaSkill / MetaSkillNames now include tree skills so the existing login / refresh-skills loop installs them and UninstallByPrefix preserves them on profile switches. Uninstall does RemoveAll for trees; Refresh rewrites the tree from embed. New embedded_test.go covers tree install, uninstall, preservation, and refresh. Package coverage 77.3%. Skill content authored + verified against the live MCP gateway: real raptor verbs from `raptor --help`, two-gateway execution model made explicit (MCP=Praxis, raptor=Facets CP), conditional import (skip when catalog populated), clouds checked via `raptor get accounts` (not Praxis cloud_cli). Vanilla-CP "link Facets integration" parked — Stop 0 treats a linked CP as a prerequisite until a first-class capability exists. Design + handover docs under docs/superpowers/specs/. * fix(praxis-onboarding): vanilla CP QA findings remediation Skill content (flows/first-deployment.md + SKILL.md): - B5: Capture canonical project name from `create project -o json` and use it in every later stop. Handles SaaS `<name>-<cpuid>` suffix vs PaaS unchanged-name behavior uniformly. - B6: Add the two-layer cloud_account dance — CP-level `create account` + project-level `apply resource cloud_account/<provider>_provider/1.0`. - B8: Insert `launch environment` step between create-env and plan, with STOPPED-state verify before launching. - B9: Apply env-level overrides for x-ui-overrides-only fields (cloud_account spec.cloud_account + spec.region) before launch. - B10 L1: SKILL.md credential-safety invariant — never Read/cat credential files into the transcript; use single-value extraction. - B11 (partial): Drop `-w`; switch to until-loop polling on cloud-link, launch, and release. Avoids MCP wrapper 2-min HTTP timeout. - Top-of-flow: trust the Stop 0 sandbox confirmation; recommend cloud-provider browser shells (CloudShell / Cloud Shell) over local CLI probing. praxis CLI (cmd/mcp.go + cmd/mcp_test.go): - B7: `--arg` now `StringArrayVar` (was `StringSliceVar`). Old binding CSV-parsed values and broke on embedded quotes/commas (e.g. raptor command containing `--description "..."`). Regression tests cover embedded quotes, embedded commas, and repeated flags. Harness deny rules (.claude/settings.json + .gitignore + README.md): - B10 L2: Project-level Read-deny on credential paths (~/.aws, ~/.praxis, ~/.facets, ~/.config/gcloud, ~/.azure, *.pem, *.key, id_*, *.token). .gitignore un-ignores .claude/settings.json so deny rules ship with the repo while transcripts/state stay ignored. README's new Security section documents the rules and recommends copying them into ~/.claude/settings.json globally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: address CodeRabbit nitpicks (#4 table-driven tests, #5 jq example) - cmd/mcp_test.go: collapse the three TestMcpArgFlag_* functions into a single table-driven TestMcpArgFlag_LiteralValueParsing. Repo CLAUDE.md states "Table-driven tests are the default pattern in Go"; this consolidates the three quote/comma/repeated-flag cases into one fixture table with t.Run subtests. - flows/first-deployment.md Stop 4b: add a concrete `jq -r '.name'` example for capturing the canonical project name. The previous text said "parse the .name field" without showing how, leaving the skill to guess the parsing step. Body text also tightened to call out the PaaS vs SaaS expectation per-customer-type. Both are CodeRabbit nitpicks tagged "Quick win" on PR #12. No behavioral change in tests (same three cases, same assertions); doc change is additive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(praxis-onboarding): tighten gating + naming per QA P1 findings Five doc-side fixes from the second-pass vanilla CP QA run: - U10: HARD-GATE rule refined from "anything billable" to "introduces NEW billable infra". Spec-only changes on already-deployed resources (e.g. flipping `versioning_enabled` on a bucket that already exists) are SOFT — announce-and-proceed under autonomy, normal confirm otherwise. Stops the over-gating where the QA run hit 5 HARD GATEs including one for a $0 incremental change. - U11: Added "Gate verbosity scales down" rule — the first HARD GATE in a flow carries the full rationale; subsequent ones are brief (one-line action + explicit yes). Don't re-explain cost models the user already accepted at an earlier gate. - U12: Stop 4b now mandates `hello-world` as the canonical project name — universal "sample project" reading. The skill must not improvise alternates ("onboarding-hello", "my-app", etc.). The B5 canonical-name capture still handles SaaS suffix, so the user doesn't need to think about what gets appended. - U13: Added a fourth "rules in both tiers" entry — before retrying a submission verb (plan/release/launch/destroy) on a CLI error, check `get releases` first. Distinguish "CLI couldn't parse the response" from "server rejected the request"; only the latter warrants retry. Prevents duplicate-release storms observed when the raptor plan empty-release-id bug fires. - U14: Engine loop step 3 now skips the per-stop "Got it?" check when the previous step had a HARD GATE — the explicit yes IS the check. Cuts the per-run yes-click count from ~14 to ~9. Doc-only changes (SKILL.md + flows/first-deployment.md). All existing embedded-content tests still pass; the binary rebuilds; `praxis refresh-skills` installs the new content cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(flow): reword Stop 5c to remove "first cloud-touching" contradiction with 4a CR-flagged: Stop 4a (IAM-role bootstrap script) is already a cloud-touching action, so claiming Stop 5c launch is the "first cloud-touching action" contradicts the earlier text. Reworded to "first action that provisions billable cloud infrastructure" — distinguishes from 4a's free IAM-role creation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(readme): address CodeRabbit P2 nitpicks on Security section - Align `id_*` claim with actual config: text now says `id_rsa*` and `id_ed25519*` (matching .claude/settings.json). - Spell out global-adoption: separate "file doesn't exist yet" path (copy whole file) from "file exists" path (merge permissions.deny entries, preserve everything else). - Name the exact transcript path (~/.claude/projects/<encoded-project>/<session-uuid>.jsonl) instead of the vague `<...>/<...>.jsonl`. - Define what "scrub" means concretely: locate via grep, then either delete the whole session file or use sed-redact and verify JSONL integrity per-line. Doc-only. No code or config change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(skillinstall): prune stale files on tree re-install/refresh writeTree only wrote/overwrote the embedded files; it never removed files already on disk that the embedded tree no longer carries. A tree skill (praxis-onboarding) whose layout changes across binary versions — e.g. a renamed or dropped flow file — would accumulate orphans after Install (re-install) or Refresh. Clear the destination dir before rewriting so the on-disk tree always matches the embedded source. Adds regression tests for orphan pruning on both re-install and Refresh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(praxis-onboarding): correct timeout/CP-link/deny-scope; drop handover note - first-deployment flow cited a "2-min HTTP timeout" in three places; the praxis mcp wrapper default is 60s (cmd/mcp.go). Correct all three and point at the configurable --timeout flag. - SKILL.md flow registry advertised "link CP", but the flow only detects a linked CP (a prerequisite) and bails to `praxis login`; guided CP linking is not built. Reword the registry cell. (cloud-account linking in Stop 4a is separate and unchanged.) - README security section claimed the deny list covers all of ~/.praxis/; settings.json only denies ~/.praxis/credentials. Correct the wording and note mcp-tools.json stays readable on purpose. - Remove the personal handover note from docs/specs; keep the design spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: pramodh-ayyappan <pramodh.ayyappan@facets.cloud> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Anshul Sao <anshul@facets.cloud>
1 parent 3b03fae commit ff08616

12 files changed

Lines changed: 1442 additions & 20 deletions

File tree

.claude/settings.json

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
{
2+
"$schema": "https://json.schemastore.org/claude-code-settings.json",
3+
"_comment": "Project-level Claude Code settings for praxis-cli. The deny list below prevents Claude from reading credential / secret files into the conversation transcript. See SKILL.md 'Credential safety' rule and the praxis-onboarding QA findings for context.",
4+
"permissions": {
5+
"deny": [
6+
"Read(~/.aws/**)",
7+
"Read(~/.praxis/credentials)",
8+
"Read(~/.praxis/credentials.*)",
9+
"Read(~/.facets/**)",
10+
"Read(~/.config/gcloud/**)",
11+
"Read(~/.azure/**)",
12+
"Read(**/*.pem)",
13+
"Read(**/*.key)",
14+
"Read(**/*.p12)",
15+
"Read(**/id_rsa)",
16+
"Read(**/id_rsa.*)",
17+
"Read(**/id_ed25519)",
18+
"Read(**/id_ed25519.*)",
19+
"Read(**/*.token)",
20+
"Bash(cat ~/.aws/*)",
21+
"Bash(cat ~/.praxis/credentials*)",
22+
"Bash(cat ~/.facets/*)",
23+
"Bash(cat ~/.config/gcloud/*)",
24+
"Bash(cat ~/.azure/*)"
25+
]
26+
}
27+
}

.gitignore

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@ coverage.txt
3131
.env
3232
.env.local
3333

34-
# Claude Code state — agent transcripts, plans, worktrees
35-
/.claude/
34+
# Claude Code state — agent transcripts, plans, worktrees.
35+
# DO commit project-level settings.json (defines deny rules for
36+
# credential files — see .claude/settings.json comment).
37+
/.claude/*
38+
!/.claude/settings.json
3639

3740
# codegraph index
3841
/.codegraph/

README.md

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,63 @@ praxis logout --all
279279
~/.gemini/skills/... same shape for Gemini CLI
280280
```
281281

282+
## Security — credential-file deny rules
283+
284+
This repo ships a project-level `.claude/settings.json` that **denies
285+
Claude Code from reading credential / secret files** into the
286+
conversation transcript. The deny list covers `~/.praxis/credentials`
287+
plus the whole of `~/.aws/`, `~/.facets/`, `~/.config/gcloud/`,
288+
`~/.azure/`, and common key file patterns (`*.pem`, `*.key`, `id_rsa*`,
289+
`id_ed25519*`, `*.token`). The rest of `~/.praxis/` stays readable on
290+
purpose — the praxis skill greps `~/.praxis/mcp-tools.json`.
291+
Project-level settings apply to anyone working inside this repo.
292+
293+
**Recommended for all users:** adopt the same deny rules globally in
294+
`~/.claude/settings.json` so they apply to *every* Claude Code session
295+
on your machine, not just sessions opened inside praxis-cli.
296+
297+
If `~/.claude/settings.json` **does not exist yet**, copy the entire
298+
[`.claude/settings.json`](.claude/settings.json) from this repo as a
299+
starting point:
300+
301+
```bash
302+
mkdir -p ~/.claude
303+
cp .claude/settings.json ~/.claude/settings.json
304+
```
305+
306+
If you **already have** a `~/.claude/settings.json` with other
307+
permissions or settings, merge in the `permissions.deny` entries
308+
from this repo's file — don't replace your whole settings object.
309+
Append entries to your existing `permissions.deny` array (deduping
310+
any already present), preserving everything else.
311+
312+
Why this matters: the praxis CLI stores PAT tokens in
313+
`~/.praxis/credentials`. The conversation transcript is persisted to
314+
`~/.claude/projects/<encoded-project>/<session-uuid>.jsonl` and may
315+
be synced, shared, or pasted. A `Read` or `cat` of a credentials
316+
file would dump tokens into that transcript — a new exposure surface
317+
beyond the file itself. The deny rules prevent the tool call before
318+
it executes.
319+
320+
**If you find a token in a transcript:**
321+
322+
1. **Rotate the affected PAT immediately** via the Facets UI
323+
(Users → API tokens → revoke + regenerate).
324+
2. **Clean the transcript.** Transcript files live at
325+
`~/.claude/projects/<encoded-project>/<session-uuid>.jsonl` (one
326+
JSONL file per session). Find the affected file:
327+
```bash
328+
grep -rl '<the-leaked-token-prefix>' ~/.claude/projects/
329+
```
330+
Then either **delete the whole session file** (simplest; loses
331+
conversation history) or **scrub the specific lines** containing
332+
the token while preserving the rest. For a quick redact:
333+
```bash
334+
sed -i.bak 's/<token-value>/REDACTED/g' <the-file>
335+
```
336+
After scrubbing, verify each line of the JSONL is still valid
337+
JSON (`python3 -c 'import sys,json; [json.loads(l) for l in sys.stdin]' < <the-file>`).
338+
282339
## Why a CLI
283340

284341
CLIs run anywhere: any AI host, CI, cron, shell pipelines. MCP

cmd/mcp.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ var (
2727

2828
func init() {
2929
mcpCmd.Flags().BoolVar(&mcpJSON, "json", false, "JSON output (default when stdout is non-TTY)")
30-
mcpCmd.Flags().StringSliceVar(&mcpArgs, "arg", nil, "key=value pair (repeatable); merged into request body")
30+
// Use StringArrayVar (not StringSliceVar) so each --arg value is taken as a
31+
// literal string with no CSV-style splitting. StringSlice trips on embedded
32+
// commas and double quotes in the value — see TestMcpArgFlag_* regression
33+
// tests covering raptor commands like `--arg command='... "quoted" ...'`.
34+
mcpCmd.Flags().StringArrayVar(&mcpArgs, "arg", nil, "key=value pair (repeatable); merged into request body")
3135
mcpCmd.Flags().StringVar(&mcpBody, "body", "", "raw JSON body (use '-' for stdin); overrides --arg")
3236
mcpCmd.Flags().DurationVar(&mcpTimeout, "timeout", 60*time.Second, "request timeout")
3337
rootCmd.AddCommand(mcpCmd)

cmd/mcp_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,64 @@ func TestBuildMCPBody_BodyNotObject_Rejected(t *testing.T) {
9292
}
9393
}
9494

95+
// Regression tests for B7 — `--arg` value parsing.
96+
//
97+
// The bug: pflag's StringSlice does CSV-style parsing on each --arg value,
98+
// which trips on embedded double quotes (`bare " in non-quoted-field`) and
99+
// also splits on commas inside a single value. Real-world callers pass
100+
// raptor commands like `--arg command='create project foo --description
101+
// "Bar"'` — those quotes / commas must reach buildMCPBody intact.
102+
//
103+
// Fix: bind --arg with StringArrayVar (literal per-flag value, no CSV
104+
// splitting). The table cases below ParseFlags on mcpCmd and assert the
105+
// literal value comes through.
106+
107+
func TestMcpArgFlag_LiteralValueParsing(t *testing.T) {
108+
tests := []struct {
109+
name string
110+
flagArgs []string
111+
want []string
112+
}{
113+
{
114+
name: "embedded quotes preserved",
115+
flagArgs: []string{"--arg", `command=create project praxis-hello --description "Onboarding sample project"`},
116+
want: []string{`command=create project praxis-hello --description "Onboarding sample project"`},
117+
},
118+
{
119+
name: "embedded commas preserved (no CSV splitting)",
120+
flagArgs: []string{"--arg", `tags=a,b,c`},
121+
want: []string{`tags=a,b,c`},
122+
},
123+
{
124+
name: "repeated flags produce multiple entries",
125+
flagArgs: []string{
126+
"--arg", `command=foo "bar"`,
127+
"--arg", `integration_name=aws-prod`,
128+
},
129+
want: []string{`command=foo "bar"`, `integration_name=aws-prod`},
130+
},
131+
}
132+
133+
for _, tt := range tests {
134+
t.Run(tt.name, func(t *testing.T) {
135+
resetMcpFlags()
136+
defer resetMcpFlags()
137+
138+
if err := mcpCmd.ParseFlags(tt.flagArgs); err != nil {
139+
t.Fatalf("ParseFlags() error = %v", err)
140+
}
141+
if len(mcpArgs) != len(tt.want) {
142+
t.Fatalf("got %d args, want %d: %q", len(mcpArgs), len(tt.want), mcpArgs)
143+
}
144+
for i, want := range tt.want {
145+
if mcpArgs[i] != want {
146+
t.Errorf("arg[%d] = %q, want %q", i, mcpArgs[i], want)
147+
}
148+
}
149+
})
150+
}
151+
}
152+
95153
func TestExtractDetail(t *testing.T) {
96154
got := extractDetail([]byte(`{"detail":"missing key"}`), "fallback")
97155
if got != "missing key" {

0 commit comments

Comments
 (0)