test(cli): cover API path construction (#9254)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The CLI is one of the main operator and agent-facing control surfaces. > - CLI commands build API paths from dynamic company, issue, project, agent, and other resource identifiers. > - Dynamic path segments need to be encoded so reserved characters cannot reshape the request URL. > - Empty dynamic path segments should fail locally instead of creating malformed API routes. > - The shared `apiPath` helper already implements those safeguards, but its behavior was not directly covered in the common CLI tests. > - This pull request adds focused coverage for path segment encoding and empty-segment rejection. > - The benefit is stronger regression coverage around a small but security-relevant CLI routing helper. ## Linked Issues or Issue Description - Bug: `apiPath` is the shared CLI helper for constructing API paths with dynamic identifiers, but the common CLI tests did not directly assert that dynamic segments are URL-encoded or that empty segments are rejected before a request is made. ## What Changed - Imported `apiPath` into `cli/src/__tests__/common.test.ts`. - Added coverage that verifies reserved characters in dynamic path segments are encoded. - Added coverage that verifies empty and undefined dynamic path segments throw before producing a malformed path. ## Verification - `./node_modules/.bin/vitest run cli/src/__tests__/common.test.ts --config cli/vitest.config.ts` passed (10 tests). - `git diff --check` passed. ## Risks - Low risk. This is test-only coverage for existing helper behavior. - If future code intentionally wants query-string construction through this helper, it should use static template text for the query string and keep dynamic values as path segments or use a dedicated query builder. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI GPT-5 via Codex, with code editing and local command execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: 馨冉 <xinxincui239@gmail.com>
This commit is contained in:
parent
c81a089c12
commit
876ac7596c
|
|
@ -4,7 +4,7 @@ import path from "node:path";
|
|||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { writeContext } from "../client/context.js";
|
||||
import { setStoredBoardCredential } from "../client/board-auth.js";
|
||||
import { inferContentTypeFromPath, resolveApiBase, resolveCommandContext } from "../commands/client/common.js";
|
||||
import { apiPath, inferContentTypeFromPath, resolveApiBase, resolveCommandContext } from "../commands/client/common.js";
|
||||
|
||||
const ORIGINAL_ENV = { ...process.env };
|
||||
|
||||
|
|
@ -185,3 +185,17 @@ describe("inferContentTypeFromPath", () => {
|
|||
expect(inferContentTypeFromPath("noextension")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("apiPath", () => {
|
||||
it("encodes dynamic path segments", () => {
|
||||
expect(apiPath`/api/issues/${"PAP-1/child"}/comments/${"needs review?"}`)
|
||||
.toBe("/api/issues/PAP-1%2Fchild/comments/needs%20review%3F");
|
||||
});
|
||||
|
||||
it("rejects empty dynamic path segments", () => {
|
||||
expect(() => apiPath`/api/issues/${""}`).toThrow("Cannot build API path with an empty path segment.");
|
||||
expect(() => apiPath`/api/issues/${undefined}`).toThrow("Cannot build API path with an empty path segment.");
|
||||
expect(() => apiPath`/api/issues/${null}`).toThrow("Cannot build API path with an empty path segment.");
|
||||
expect(() => apiPath`/api/issues/${" "}`).toThrow("Cannot build API path with an empty path segment.");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue