Add Ruby as a supported output language#104
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
This is likely a transient issue. You can re-trigger a run from the dashboard. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
8f19b95 to
cdef5f0
Compare
|
@greptile review |
Follows the exact same pattern as python/javascript/typescript:
- New prompt partial (prompts/partials/_language_ruby.md): uses
net/http and json - both part of Ruby's standard library, so no
gem/Bundler dependency is needed at all, no project/dependency file
of any kind. Calls out that net/http has no built-in cookie jar
(unlike some other languages' HTTP clients), so cookie-based auth
needs manual Set-Cookie capture and replay. Same auth-hardcoding/
refresh guidance as the other languages otherwise.
- base_engineer.py: added "ruby" to _OUTPUT_LANGUAGE_EXTENSIONS (.rb)
and _get_language_name() ("Ruby"). _get_run_command() uses the full
path rather than a bare relative "ruby api_client.rb" - the same
working-directory fix already applied to Go/Java/C#/PHP, applied
proactively here too since ruby <relative-path> has the identical
shape (the agent's cwd is scripts_dir.parent.parent, not scripts_dir
where the script is actually saved).
- cli.py: added Ruby to the interactive `output_language` config
picker.
- Updated the three docstrings/comments that listed the language set
(config.py, engineer.py, prompts/__init__.py) and README.md's output
language line.
- Tests: mirrored the existing per-language test pattern in
test_base_engineer.py (extension, run command, prompt content) and
test_prompts.py (partial loading) for Ruby.
Checked the partial for the literal-brace str.format_map() bug caught
in the Java PR - no literal `{`/`}` anywhere in the new file.
Full test suite passes (49/49, run 3x) except the same pre-existing
flaky test noted on the other language PRs
(test_existing_client_language_falls_back_to_newest_file - a
mtime-resolution race condition, unrelated to this change). ruff check
is clean on every file this touches (8 pre-existing lint errors
elsewhere on main, none in the touched lines).
Manually double-quoting the path (f'ruby "{path}"') does not stop
$()/backtick command substitution inside a POSIX shell double-quoted
string. Use shlex.quote() instead, matching the fix already applied to
the Go/Java/C#/PHP run commands after bot review flagged the same
issue on the PHP PR.
Both review bots flagged the same two issues: "reuse one persistent connection" doesn't account for captured traffic spanning multiple hosts (Net::HTTP.start is bound to a single host:port), and replaying every captured cookie as one Cookie header ignores Set-Cookie's domain/path/secure scoping, which can leak a session cookie to the wrong origin. Reworded both bullets to scope per origin instead.
cubic flagged that a relative --output-dir breaks this command: scripts_dir stays relative to the original cwd, but once the agent's cwd moves to scripts_dir.parent.parent (see analyze_and_generate), embedding that same relative string re-interprets it from the new location and points at a doubly-nested, nonexistent path. Resolve scripts_dir before interpolating it, same fix as just applied to the C# run command.
cdef5f0 to
2978715
Compare
| **Authentication & credentials:** | ||
| - Hardcode all cookies, tokens, session IDs, and auth headers found in the traffic directly in the script | ||
| - The user should be able to run the script immediately with zero configuration — no env vars, no config files, no `bundle install` | ||
| - `net/http` has no built-in cookie jar, unlike some other languages' HTTP clients — if the API uses cookies, capture each `Set-Cookie` header's name/value and its domain, path, and secure attributes, and send back only the cookies matching a given request's origin and path in the `Cookie` header, rather than replaying every captured cookie on every request (a multi-origin trace can otherwise leak a session cookie to the wrong host or send an invalid header) |
There was a problem hiding this comment.
This instruction still omits Expires and Max-Age. When a refresh or logout response expires a cookie with Max-Age=0, generated code can retain and replay the old session value instead of deleting it. This can leave the script authenticated as the wrong session or cause later requests to fail. Require the cookie store to process expiration and deletion, with entries keyed by name, domain, and path.
| - `net/http` has no built-in cookie jar, unlike some other languages' HTTP clients — if the API uses cookies, capture each `Set-Cookie` header's name/value and its domain, path, and secure attributes, and send back only the cookies matching a given request's origin and path in the `Cookie` header, rather than replaying every captured cookie on every request (a multi-origin trace can otherwise leak a session cookie to the wrong host or send an invalid header) | |
| - `net/http` has no built-in cookie jar, unlike some other languages' HTTP clients — if the API uses cookies, store each `Set-Cookie` header by name, domain, and path; enforce its secure, `Expires`, and `Max-Age` attributes; remove expired or deleted entries; and send back only cookies matching the request's origin and path in the `Cookie` header (a multi-origin trace can otherwise leak a session cookie to the wrong host or send an invalid header) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/reverse_api/prompts/partials/_language_ruby.md
Line: 11
Comment:
**Handle cookie expiration too**
This instruction still omits `Expires` and `Max-Age`. When a refresh or logout response expires a cookie with `Max-Age=0`, generated code can retain and replay the old session value instead of deleting it. This can leave the script authenticated as the wrong session or cause later requests to fail. Require the cookie store to process expiration and deletion, with entries keyed by name, domain, and path.
```suggestion
- `net/http` has no built-in cookie jar, unlike some other languages' HTTP clients — if the API uses cookies, store each `Set-Cookie` header by name, domain, and path; enforce its secure, `Expires`, and `Max-Age` attributes; remove expired or deleted entries; and send back only cookies matching the request's origin and path in the `Cookie` header (a multi-origin trace can otherwise leak a session cookie to the wrong host or send an invalid header)
```
How can I resolve this? If you propose a fix, please make it concise.A Max-Age=0 or Expires deletion from a refresh/logout response must evict the stored entry, or the generated script keeps replaying a dead session value. Key entries by name, domain, and path.
Follows the exact same pattern as python/javascript/typescript:
output_languageconfig picker.Checked the partial for the literal-brace str.format_map() bug caught in the Java PR - no literal
{/}anywhere in the new file.Full test suite passes (49/49, run 3x) except the same pre-existing flaky test noted on the other language PRs
(test_existing_client_language_falls_back_to_newest_file - a mtime-resolution race condition, unrelated to this change). ruff check is clean on every file this touches (8 pre-existing lint errors elsewhere on main, none in the touched lines).
Summary by cubic
Adds Ruby as a supported output language for generated API clients. Uses stdlib
net/httpandjson, runs via a shell-quoted absolute-pathrubycommand, and adds per-origin connection reuse plus a cookie jar with domain/path/secure and expiration/deletion handling.New Features
Expires/Max-Age, and deletion; includes token refresh guidance.BaseEngineer:.rbextension, language name, andruby <absolute path to api_client.rb>usingshlex.quote.Bug Fixes
Written for commit 916f68d. Summary will update on new commits.
Greptile Summary
This PR adds Ruby as a supported generated-client language. The main changes are:
net/httpandjson.Confidence Score: 4/5
The remaining cookie behavior should be fixed before merging.
src/reverse_api/prompts/partials/_language_ruby.md
Security Review
The cookie guidance can retain and replay credentials after a server expires or deletes them.
Important Files Changed
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "Resolve scripts_dir to an absolute path ..." | Re-trigger Greptile