docs: document memory endpoints and add CI validation - #1
Conversation
📝 WalkthroughWalkthroughThe PR documents per-user memory-file APIs across OpenAPI, API references, SDK guides, and navigation. It updates OpenAPI sanitization rules, adds Python documentation-snippet validation, and runs documentation checks in GitHub Actions. ChangesMemory API documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-checks.yml:
- Around line 13-21: Update the workflow’s actions/checkout step to disable
credential persistence, and change the Mintlify CLI installation in the “Install
Mintlify CLI” step to use a specific verified mint version rather than the
unpinned package. Preserve the existing Node setup and workflow sequence.
In `@api-reference.mdx`:
- Around line 29-34: Update the request-scoping documentation in
api-reference.mdx to state that thread endpoints and all memory endpoints use
query-parameter user_id, except POST /memory, which receives user_id in the JSON
body. Preserve the existing chat-call and SDK behavior descriptions.
In `@scripts/check_snippets.py`:
- Line 21: Update FENCE_RE in scripts/check_snippets.py to match Python fences
with optional same-line metadata and either LF or CRLF line endings, while
capturing only the fenced body. Ensure the existing snippet-validation flow
dedents each captured Python block before parsing and allows top-level await.
In `@scripts/sanitize_openapi.py`:
- Around line 60-74: Remove the duplicate endpoint-by-endpoint inventory above
KEEP_PATHS in scripts/sanitize_openapi.py, retaining only a concise comment
stating that KEEP_PATHS defines the public SDK surface and should be updated
when the SDK endpoint set changes.
In `@sdk/reference.mdx`:
- Around line 59-95: Align the documentation with the currently published SDK:
in sdk/reference.mdx lines 59-95, delay or remove the client.memory section
until its methods and models are released; in api-reference.mdx lines 6-9,
remove the claim that every listed endpoint is SDK-callable until a compatible
release exists; in sdk/overview.mdx lines 90-95, remove or delay the
client.memory resource claim; and in sdk/quickstart.mdx lines 119-147, retain
the memory workflow only after shipping examples/09_memory.py with the SDK
release, then keep its link.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cf921012-57fe-4c31-9887-2baa8d9f4ceb
📒 Files selected for processing (10)
.github/workflows/docs-checks.ymlCLAUDE.mdapi-reference.mdxdocs.jsonopenapi.jsonscripts/check_snippets.pyscripts/sanitize_openapi.pysdk/overview.mdxsdk/quickstart.mdxsdk/reference.mdx
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "20" | ||
|
|
||
| - name: Install Mintlify CLI | ||
| run: npm install -g mint |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
npm view mint version dist.integrityRepository: cominty/docs
Length of output: 597
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file =="
cat -n .github/workflows/docs-checks.yml | sed -n '1,80p'
echo
echo "== package presence and lock metadata =="
fd -a '^(package.json|package-lock.json|pnpm-lock.yaml|yarn.lock)$' .GitHub workflow root 2>/dev/null || true
sed -n '1,220p' package.json 2>/dev/null || true
git ls-files | rg '(^|/)package(-lock)?\.json$' || true
echo
echo "== mint lifecycle/package metadata =="
tmp="$(mktemp -d)"
cd "$tmp"
npm pack mint@4.2.788 --silent 2>/dev/null || { echo "npm pack failed"; exit 0; }
tar -tzf mint-4.2.788.tgz | sed -n '1,120p'
python3 - <<'PY'
# Read package metadata and lifecycle fields from packed tarball without executing scripts.
import tarfile, json, os
for name in [n for n in sorted(tarfile.open('mint-4.2.788.tgz', 'r:gz').getnames())]:
if name.endswith('/package.json'):
f = tarfile.open('mint-4.2.788.tgz', 'r:gz').extractfile(name)
data = f.read()
try:
pkg = json.loads(data)
except Exception as e:
print("package.json parse failed for", name, e)
continue
print("package:", pkg.get("name"), pkg.get("version"))
print("lifecycle scripts:", {k:v for k,v in pkg.get("scripts",{}).items() if k in ("preinstall","install","postinstall","preversion","version","postversion","prepublishOnly","prepack","postpack","prepublish","prepare","prepublish","pretest","test","posttest")})
print("dependencies keys:", sorted(pkg.get("dependencies",{}).keys())[:80])
for key in ("preinstall","install","postinstall"):
print(key + ":", pkg.get("scripts",{}).get(key))
PYRepository: cominty/docs
Length of output: 1486
🌐 Web query:
npm global install lifecycle scripts dependency runs postinstall dependencies
💡 Result:
As of August 2026, npm has introduced a significant shift in how it handles dependency lifecycle scripts (such as preinstall, install, and postinstall) to mitigate supply chain security risks [1]. Lifecycle scripts in dependencies are now blocked by default during installation [1]. This means that when you run an npm install command, these scripts will no longer execute automatically unless the package is explicitly allowed in your project configuration [2][3][1]. Regarding global installations specifically: - When using npm install -g, you can control the execution of lifecycle scripts using the --allow-scripts flag [2][3]. This flag accepts a comma-separated list of packages that are permitted to run their installation scripts [2]. - Alternatively, you can use --foreground-scripts if you wish to run these scripts in the foreground process to see their output [2][4]. - You can also bypass the security restrictions entirely using --dangerously-allow-all-scripts, though this is discouraged for security reasons [2][3]. - If you prefer to disable all scripts globally, you can set the ignore-scripts configuration to true in your npmrc file [5][6]. This change reflects a move toward an "opt-in" model for script execution [3][1]. Projects are encouraged to manage these permissions within their package.json or npmrc files rather than relying on command-line overrides [2][3]. This design adjustment is intended to prevent the automatic execution of potentially malicious code that has historically exploited the trust inherent in the installation process [1][6][7].
Citations:
- 1: https://www.infoworld.com/article/4183849/github-finally-pulls-the-plug-on-automatic-install-script-execution-for-npm.html
- 2: https://docs.npmjs.com/cli/v11/commands/npm-install/?v=true
- 3: https://github.com/npm/rfcs/blob/main/accepted/0054-make-scripts-install-opt-in.md
- 4: [BUG] No "postinstall" on global packages anymore npm/cli#2194
- 5: https://cheatsheetseries.owasp.org/cheatsheets/NPM_Security_Cheat_Sheet.html
- 6: https://unit42.paloaltonetworks.com/monitoring-npm-supply-chain-attacks/
- 7: https://safeguard.sh/resources/blog/npm-lifecycle-scripts-security-risks
🌐 Web query:
actions/checkout persists credentials true default
💡 Result:
The default value for the persist-credentials input in the actions/checkout GitHub Action is true [1][2][3]. When persist-credentials is set to true (the default), the action configures the local Git repository to use the provided authentication token or SSH key [1][2][4]. This allows subsequent steps in the same job to execute authenticated Git commands without needing to manually provide credentials [1][2][5]. For security reasons, recent versions of actions/checkout have improved how this is handled by storing these credentials in a separate file under $RUNNER_TEMP rather than directly in .git/config [1][2]. The credentials are removed during post-job cleanup [1][2][4]. If you wish to disable this behavior, you must explicitly set persist-credentials: false in your workflow configuration [1][2][6].
Citations:
- 1: https://github.com/actions/checkout
- 2: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 3: https://github.com/actions/checkout/blob/cbb722410c2e876e24abbe8de2cc27693e501dcb/action.yml
- 4: https://github.com/marketplace/actions/checkout?version=v3.6.0
- 5: https://github.com/actions/checkout/blob/v2.4.0/README.md
- 6: Remove
persist-credentialsor change the default tofalseactions/checkout#485
Pin the Mintlify CLI install and avoid exposed checkout credentials.
npm install -g mint leaves the workflow vulnerable if npm script execution is enabled or pinned credentials are used elsewhere. Pin a verified mint version and disable credential persistence.
Proposed fix
+permissions:
+ contents: read
+
jobs:
validate:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
+ with:
+ persist-credentials: false
...
- name: Install Mintlify CLI
- run: npm install -g mint
+ run: npm install -g mint@4.2.788📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - name: Set up Node | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: "20" | |
| - name: Install Mintlify CLI | |
| run: npm install -g mint | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false | |
| - name: Set up Node | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: "20" | |
| - name: Install Mintlify CLI | |
| run: npm install -g mint@4.2.788 |
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 13-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 21-21: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docs-checks.yml around lines 13 - 21, Update the
workflow’s actions/checkout step to disable credential persistence, and change
the Mintlify CLI installation in the “Install Mintlify CLI” step to use a
specific verified mint version rather than the unpinned package. Preserve the
existing Node setup and workflow sequence.
Source: Linters/SAST tools
| Every request is scoped to one end user via `user_id`. Where it's sent | ||
| varies by endpoint: in the request body (`options.user_id`) for chat calls, | ||
| as a query parameter for thread and memory listings/lookups — **except** | ||
| `POST /memory`, which takes it in the JSON body alongside `path`, `purpose`, | ||
| and `content`. The SDK fills this in for you from the client's configured | ||
| user, so you never pass it explicitly. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document query user_id for all memory operations.
Line 31 limits the query-parameter description to memory listings/lookups. PUT /memory/file and DELETE /memory/file also take user_id in the query string. State that thread endpoints and all memory endpoints except POST /memory use query user_id.
Proposed fix
- as a query parameter for thread and memory listings/lookups — **except**
+ as a query parameter for thread endpoints and all memory endpoints — **except**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Every request is scoped to one end user via `user_id`. Where it's sent | |
| varies by endpoint: in the request body (`options.user_id`) for chat calls, | |
| as a query parameter for thread and memory listings/lookups — **except** | |
| `POST /memory`, which takes it in the JSON body alongside `path`, `purpose`, | |
| and `content`. The SDK fills this in for you from the client's configured | |
| user, so you never pass it explicitly. | |
| Every request is scoped to one end user via `user_id`. Where it's sent | |
| varies by endpoint: in the request body (`options.user_id`) for chat calls, | |
| as a query parameter for thread endpoints and all memory endpoints — **except** | |
| `POST /memory`, which takes it in the JSON body alongside `path`, `purpose`, | |
| and `content`. The SDK fills this in for you from the client's configured | |
| user, so you never pass it explicitly. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api-reference.mdx` around lines 29 - 34, Update the request-scoping
documentation in api-reference.mdx to state that thread endpoints and all memory
endpoints use query-parameter user_id, except POST /memory, which receives
user_id in the JSON body. Preserve the existing chat-call and SDK behavior
descriptions.
elfominty
left a comment
elfominty
left a comment
There was a problem hiding this comment.
Thanks a lot for this work @ahmeda-cominty
| await client.memory.delete("preferences/tone.md") | ||
| ``` | ||
|
|
||
| ▶ [`examples/09_memory.py`](https://github.com/cominty/python-sdk/blob/main/examples/09_memory.py) |
There was a problem hiding this comment.
Before merging the documentation, make sure the SDK has been released; otherwise, it may result in broken links.
| "responses": { | ||
| "201": { | ||
| "description": "Successful Response", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/MemoryFileOut" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "422": { | ||
| "description": "Validation Error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/HTTPValidationError" | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't you get a 409 error if you create a memo with the same path/version as an existing one ?
| "responses": { | ||
| "200": { | ||
| "description": "Successful Response", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/MemoryFileSummaryOut" | ||
| }, | ||
| "title": "Response List Memory Files Memory Get" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "422": { | ||
| "description": "Validation Error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/HTTPValidationError" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Question: If the memo via the path doesn't exist, does the endpoint return a 404 ? (Same for the other endpoints)
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores