diff --git a/CHANGELOG.md b/CHANGELOG.md index 1776ad9..3163bad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `docs endpoint` fetch `https://www.c1.ai/api/openapi.yaml` and use a source-specific cache at `~/.c1i/cache/api-openapi.yaml`. +- **Old cache files stayed on disk forever.** A cache file renamed or dropped + by a release, such as the `openapi.yaml` the entry above replaces, was never + removed. Refreshing the OpenAPI spec now deletes any file in `~/.c1i/cache/` + that the running version doesn't use. It leaves subdirectories alone, and + skips the sweep when the cache dir is a symlink or, with no home dir, a path + relative to the working directory. + - **`requests create grant --duration` documented a format the server refuses.** Its help and the README suggested `24h` or `7d`, which the API rejects with `invalid google.protobuf.Duration value` (exit 2). They now say diff --git a/README.md b/README.md index 49653a8..1e4b148 100644 --- a/README.md +++ b/README.md @@ -878,6 +878,8 @@ c1i docs guide register-mcp-server `docs skill` is kept as an alias of `docs agents` for backward compatibility; both print identical output. +`~/.c1i/cache/` belongs to c1i: when `docs openapi`, `docs endpoints` or `docs endpoint` refreshes the spec, any file there that the running version doesn't use (such as one left by an older release) is deleted. Don't keep your own files in it. + ## Output Conventions - **List commands** (`users list`, `apps list`, etc.) output NDJSON (one JSON object per line). diff --git a/cmd/agents.md b/cmd/agents.md index 24191e4..54e10c9 100644 --- a/cmd/agents.md +++ b/cmd/agents.md @@ -121,6 +121,9 @@ threshold, so it always returns up to 10 plausible hits even for a nonsense query: never read a result as proof a concept exists, or an unexpected result as proof it is absent. `docs endpoints --filter` DOES have a true no-match. +Don't store files in `~/.c1i/cache/`: a spec refresh deletes any file there +that the running version doesn't use. + ## Choosing a command Prefer a first-class command (`users get`, `mcp servers register`, `grants diff --git a/cmd/cache.go b/cmd/cache.go new file mode 100644 index 0000000..1af3f3b --- /dev/null +++ b/cmd/cache.go @@ -0,0 +1,35 @@ +package cmd + +import ( + "os" + "path/filepath" +) + +// cacheFiles is every file c1i keeps in ~/.c1i/cache. Anything else there is +// left over from an older release and is deleted when the cache is written, so +// renaming a cache file needs no cleanup rule of its own. Add a new cache file +// here or it will be deleted on every refresh. +var cacheFiles = map[string]bool{cacheFileName: true} + +// pruneCacheDir is best-effort: cleanup must never fail the command. It skips +// subdirectories, a symlinked dir (Lstat sees a link, not a dir) whose target +// may hold files c1i doesn't own, and a relative dir, which is what an unset +// HOME yields and would sweep the working directory instead. +func pruneCacheDir(dir string) { + if !filepath.IsAbs(dir) { + return + } + if info, err := os.Lstat(dir); err != nil || !info.IsDir() { + return + } + entries, err := os.ReadDir(dir) + if err != nil { + return + } + for _, e := range entries { + if e.IsDir() || cacheFiles[e.Name()] { + continue + } + _ = os.Remove(filepath.Join(dir, e.Name())) + } +} diff --git a/cmd/cache_test.go b/cmd/cache_test.go new file mode 100644 index 0000000..21a0b72 --- /dev/null +++ b/cmd/cache_test.go @@ -0,0 +1,151 @@ +package cmd + +import ( + "bytes" + "context" + "io" + "net/http" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +type stubRoundTripper func(*http.Request) (*http.Response, error) + +func (f stubRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// refreshOpenAPICache runs `docs openapi` against an expired cache so it +// fetches a stub spec and rewrites the cache. +func refreshOpenAPICache(t *testing.T, cacheDir string) { + t.Helper() + cachePath := filepath.Join(cacheDir, cacheFileName) + if err := os.WriteFile(cachePath, []byte(stubOpenAPISpec), 0o600); err != nil { + t.Fatal(err) + } + expired := time.Now().Add(-2 * cacheMaxAge) + if err := os.Chtimes(cachePath, expired, expired); err != nil { + t.Fatal(err) + } + + orig := http.DefaultClient.Transport + http.DefaultClient.Transport = stubRoundTripper(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(stubOpenAPISpec)), + Request: r, + }, nil + }) + t.Cleanup(func() { http.DefaultClient.Transport = orig }) + + prevCtx := docsOpenapiCmd.Context() + docsOpenapiCmd.SetOut(&bytes.Buffer{}) + docsOpenapiCmd.SetContext(context.Background()) + t.Cleanup(func() { + docsOpenapiCmd.SetOut(nil) + docsOpenapiCmd.SetContext(prevCtx) + }) + if err := docsOpenapiCmd.RunE(docsOpenapiCmd, nil); err != nil { + t.Fatalf("docs openapi: %v", err) + } + info, err := os.Stat(cachePath) + if err != nil || time.Since(info.ModTime()) > time.Minute { + t.Fatalf("cache was not rewritten: %v", err) + } +} + +func TestCacheWritePrunesLeftoverFiles(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + cacheDir := filepath.Join(home, cacheDirName, "cache") + if err := os.MkdirAll(filepath.Join(cacheDir, "subdir"), 0o700); err != nil { + t.Fatal(err) + } + for _, name := range []string{"openapi.yaml", ".tmp-leftover"} { + if err := os.WriteFile(filepath.Join(cacheDir, name), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + // None of these is a cache file, so all must survive: a link's target, a + // file inside a subdirectory, and state beside the cache dir. + outside := filepath.Join(t.TempDir(), "outside.txt") + inner := filepath.Join(cacheDir, "subdir", "inner.txt") + sibling := filepath.Join(home, cacheDirName, "other-state.json") + for _, p := range []string{outside, inner, sibling} { + if err := os.WriteFile(p, []byte("keep"), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(outside, filepath.Join(cacheDir, "link")); err != nil { + t.Logf("symlinks unavailable, link entry not covered: %v", err) + } + + refreshOpenAPICache(t, cacheDir) + + entries, err := os.ReadDir(cacheDir) + if err != nil { + t.Fatal(err) + } + var got []string + for _, e := range entries { + got = append(got, e.Name()) + } + want := []string{cacheFileName, "subdir"} + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Errorf("cache dir holds %v, want %v", got, want) + } + for _, p := range []string{outside, inner, sibling} { + if b, err := os.ReadFile(p); err != nil || string(b) != "keep" { + t.Errorf("%s was removed or changed: %q, %v", p, b, err) + } + } +} + +// A cache dir the user redirected elsewhere may hold files c1i doesn't own. +func TestCachePruneSkipsSymlinkedCacheDir(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + target := t.TempDir() + stray := filepath.Join(target, "not-ours.txt") + if err := os.WriteFile(stray, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(home, cacheDirName), 0o700); err != nil { + t.Fatal(err) + } + cacheDir := filepath.Join(home, cacheDirName, "cache") + if err := os.Symlink(target, cacheDir); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + refreshOpenAPICache(t, cacheDir) + + if _, err := os.Stat(stray); err != nil { + t.Errorf("file behind a symlinked cache dir was removed: %v", err) + } +} + +// With no home dir the cache path is relative, so it lands in the working dir. +func TestCachePruneSkipsRelativeCacheDir(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("an empty HOME doesn't unset the home dir on Windows") + } + t.Setenv("HOME", "") + t.Chdir(t.TempDir()) + cacheDir := filepath.Join(cacheDirName, "cache") + if err := os.MkdirAll(cacheDir, 0o700); err != nil { + t.Fatal(err) + } + stray := filepath.Join(cacheDir, "not-ours.txt") + if err := os.WriteFile(stray, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + + refreshOpenAPICache(t, cacheDir) + + if _, err := os.Stat(stray); err != nil { + t.Errorf("file in a working-dir-relative cache dir was removed: %v", err) + } +} diff --git a/cmd/docs_openapi.go b/cmd/docs_openapi.go index 5143019..f18c41d 100644 --- a/cmd/docs_openapi.go +++ b/cmd/docs_openapi.go @@ -203,7 +203,9 @@ func fetchOpenAPISpec(cmd *cobra.Command) ([]byte, error) { } _ = os.MkdirAll(filepath.Dir(cachePath), 0o700) - _ = os.WriteFile(cachePath, data, 0o644) // #nosec G306 -- cached OpenAPI spec is public C1 API documentation, not sensitive + if os.WriteFile(cachePath, data, 0o644) == nil { // #nosec G306 -- cached OpenAPI spec is public C1 API documentation, not sensitive + pruneCacheDir(filepath.Dir(cachePath)) + } return data, nil }