You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: skills/code-review/SKILL.md
+49-3Lines changed: 49 additions & 3 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -18,19 +18,65 @@ description: >-
18
18
19
19
Provide consistent **security**, **correctness**, and **maintainability** review for this repository. The plugin handles stack API keys in error messages, opens **HTML** diffs in a browser, and writes **diagram** files via Graphviz.
20
20
21
+
Use **Critical** / **Important** / **Suggestion** when leaving feedback.
22
+
21
23
### Highlights
22
24
23
25
-**Secrets**: Never approve logging of tokens, `authtoken` / `authorization` values, or raw management tokens.
24
26
-**Compare / diagram**: Changes to [src/core/content-type/compare.ts](../../src/core/content-type/compare.ts) or [diagram.ts](../../src/core/content-type/diagram.ts) deserve extra scrutiny (temp files, browser open, paths, binary dependency).
25
27
-**Dependencies**: axios, diff2html, git-diff, node-graphviz, tmp, cli-ux—review changelog and supply-chain for version bumps.
26
28
-**Quality**: TypeScript and **eslint-config-oclif-typescript** ([.eslintrc](../../.eslintrc)); behavioral changes should include or update **Jest** tests where appropriate.
27
29
28
-
### Full checklist
30
+
### Security and privacy
31
+
32
+
| Severity | Item |
33
+
|----------|------|
34
+
| Critical | No logging or serializing of **access tokens**, **management tokens**, or **Bearer** strings. |
35
+
| Critical | No new `console.log` of full API responses that may contain secrets. |
36
+
| Important | Stack API keys appear in user-facing errors only in line with [src/core/contentstack/client.ts](../../src/core/contentstack/client.ts) (`buildError` + optional key suffix). |
37
+
38
+
### Correctness
39
+
40
+
| Severity | Item |
41
+
|----------|------|
42
+
| Critical | Command flags and `setup(flags)` behavior remain consistent; **compare-remote** still resolves origin vs remote stacks correctly. |
43
+
| Important |**Compare**: left/right version logic and warning when versions are equal; HTML output path and browser open behavior unchanged unless intentionally redesigned. |
44
+
| Important |**Diagram**: output path validation; Graphviz / DOT paths; orientation and file type flags. |
45
+
| Suggestion | Edge cases for empty audit logs, missing references, or single-version content types. |
46
+
47
+
### Compare and diagram (touching core)
48
+
49
+
| Severity | Item |
50
+
|----------|------|
51
+
| Critical |[compare.ts](../../src/core/content-type/compare.ts): temp HTML creation does not write sensitive data beyond the diff; file handling is safe on failure paths. |
52
+
| Important |[diagram.ts](../../src/core/content-type/diagram.ts): `sanitizePath` / path usage; large stack models do not cause unbounded memory without consideration. |
53
+
| Suggestion | User messaging when Graphviz is missing or SVG generation fails. |
54
+
55
+
### Dependencies
56
+
57
+
| Severity | Item |
58
+
|----------|------|
59
+
| Important |**axios**: security advisories; upgrade notes. |
Formatting helpers live under `src/core/content-type/formatting.ts` where imported by core modules.
50
+
51
+
### Auth flow (high level)
52
+
53
+
```mermaid
54
+
flowchart LR
55
+
subgraph setup [ContentTypeCommand.setup]
56
+
A[getAuthDetails]
57
+
B{accessToken?}
58
+
C[error auth:login]
59
+
D{alias or stack key?}
60
+
E[exit 1 missing stack]
61
+
F[getToken or use stack API key]
62
+
G[ContentstackClient]
63
+
end
64
+
A --> B
65
+
B -->|no| C
66
+
B -->|yes| D
67
+
D -->|neither| E
68
+
D -->|ok| F
69
+
F --> G
70
+
```
71
+
72
+
-**`compare-remote`**: `setup` is called with `{ alias: undefined, stack: flags["origin-stack"] }` so `apiKey` is the **origin** stack API key; remote stack is passed only in `getStack` / `getContentType` calls.
-**Axios `ContentstackClient`**: `GET https://{cmaHost}/v3/...` with default headers `authorization` (if Bearer) or `authtoken`, plus per-request `headers: { api_key }`. Used for audit logs and references. Errors → `ContentstackError` via `buildError`.
85
+
-**Axios `ContentstackClient`**: `GET https://{cmaHost}/v3/...` with default headers `authorization` (if Bearer) or `authtoken`, plus per-request `headers: { api_key }`. Used for audit logs and CT references. Errors → `ContentstackError` via `buildError`.
50
86
-**Management SDK** (`managementSDKClient({ host, 'X-CS-CLI': ... })`): stack fetch, content types, global fields, content type by version—see `src/utils/index.ts`.
51
87
88
+
**CMA request shape (`ContentstackClient`)**
89
+
90
+
-**Base URL**: `https://{cmaHost}/v3/` (`cmaHost` from command context).
91
+
-**Default axios headers**: `authorization: <token>` if token string includes `Bearer`, else `authtoken: <token>`.
92
+
-**Per-request**: `headers: { api_key: <stack API key> }` for stack-scoped routes.
93
+
94
+
| Method | HTTP | Path / params |
95
+
|--------|------|----------------|
96
+
|`getContentTypeAuditLogs`| GET |`/audit-logs` — `params.query.$and`: `module: content_type`, `metadata.uid`|
97
+
|`getContentTypeReferences`| GET |`/content_types/{uid}/references` — `include_global_fields: true`|
98
+
99
+
Errors: response `data.errors` → `ContentstackError`; optional suffix with stack API key when `data.errors.api_key` and context `api_key` are set.
100
+
101
+
### Compare and diagram pipelines
102
+
103
+
-**Compare**: `core/content-type/compare.ts` builds a unified diff from two JSON snapshots (`git-diff`), parses with **diff2html**, writes a **temporary HTML** file, opens it in the browser (`cli-ux` / `cli.open`). Not a terminal table.
104
+
-**Diagram**: `core/content-type/diagram.ts` builds a DOT graph, runs **node-graphviz** (`graphviz` binary must be available on the system for SVG rendering). Output path is sanitized where utilities apply.
105
+
106
+
### Commands (flags and behavior)
107
+
108
+
Primary sources: `README.md` and `src/commands/content-type/*.ts`.
-**Behavior**: Audit logs via `getContentTypeAuditLogs`; README notes **audit log retention** (e.g. 90 days) per Contentstack docs.
127
+
128
+
#### `content-type:compare`
129
+
130
+
-**Flags**: stack identity + `--content-type` (`-c`); optional `--left` (`-l`) / `--right` (`-r`) **integers** (both required if either set). If omitted, command infers latest version vs previous from discovery fetch.
-**Behavior**: Side-by-side diff in **HTML** in a browser; not stdout-only. Warns if left === right.
133
+
134
+
#### `content-type:compare-remote`
135
+
136
+
-**Flags**: `--origin-stack` (`-o`) and `--remote-stack` (`-r`) **required** (stack API keys); `--content-type` (`-c`) required. No token-alias flow for two stacks—setup uses **origin** stack key for session.
137
+
-**Files**: `src/commands/content-type/compare-remote.ts`, same `core/content-type/compare.ts` as same-stack compare.
138
+
-**Behavior**: Same HTML diff pipeline; compares CT JSON from two stacks. Warns if origin === remote API key.
-**Behavior**: Loads all content types + global fields; renders graph. **Graphviz** must be installed for typical SVG generation; DOT export available. README documents `-t dot` for raw DOT language.
145
+
146
+
### Editing checklist
147
+
148
+
| Change | Touch first |
149
+
|--------|-------------|
150
+
| New flag / description | Command file under `src/commands/content-type/`, then `oclif readme`|
151
+
| Output format / table |`src/core/content-type/*.ts`, `formatting.ts`|
`package.json` scripts **`prepack`** and **`version`** drive `tsc`, `oclif manifest`, and `oclif readme`. After changing commands, flags, or descriptions, keep **README.md** and **oclif.manifest.json** in sync—see [dev-workflow/SKILL.md](../dev-workflow/SKILL.md) for commands and workflow.
0 commit comments