Repository navigation
Enforce retention maturity and join safety - #6
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21d0d98700
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ";" in stripped: | ||
| raise ValueError("Only one SQL statement is allowed") | ||
| if not _ALLOWED_START.match(stripped): | ||
| raise ValueError("Only read-only SELECT or WITH statements are allowed") | ||
| match = _FORBIDDEN.search(stripped) |
There was a problem hiding this comment.
For otherwise valid read-only queries containing a semicolon or forbidden-looking word inside a string literal or comment, the raw-text checks reject the query—for example, SELECT 'delete' AS action, SELECT 'a;b' AS value, and SELECT 1 -- update stats. This contradicts the adapter's promise to accept one read-only SELECT or WITH statement and will block common predicates on action/status text; inspect SQL tokens so only actual statement separators and executable keywords are rejected.
Useful? React with 👍 / 👎.
| ids = [case.get("id") for case in value] | ||
| if any(not isinstance(item, str) or not item for item in ids) or len(ids) != len(set(ids)): |
There was a problem hiding this comment.
When a custom --cases file supplies an absolute or traversing ID, this validation accepts it and the later path joins can escape the response directories. For example, an ID derived from /tmp/shared.md makes both the ChatData and unguided conditions read that same absolute file, so prepare succeeds with two identical responses while silently ignoring both supplied condition directories and invalidating the comparison. Require IDs to be simple relative filename stems without separators or traversal components.
Useful? React with 👍 / 👎.
Claude's review identified a real gap: ChatData encoded strong analytical judgment, but too much of that judgment still depended on prose. This release turns two common failure modes into runnable checks and adds public evidence for what the package can actually enforce.
Validation:
python3 -m unittest discover -s tests -p 'test_*.py'— 189 passed, 2 optional DuckDB tests skipped in base Python/tmp/chatdata-duckdb-test/bin/python -m unittest tests.test_duckdb_query— 5 passed with DuckDB installedpython3 scripts/validate.py— passedclaude plugin validate plugins/chatdata— passedNo completed head-to-head model comparison is claimed. The release ships the blinded harness needed to run and publish one honestly.