Repository navigation
Conversation
…repo #820 🐛 fix(prompt): prompt push --owner - fails when it creates a new repo Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix satisfies issue #820 and includes appropriate unit and integration coverage.
0 open findings
What changed in this PR
Fixes prompt repository creation when --owner - is used by sending the bare repository name to LangSmith.
Changes:
- Normalizes
-/namehandles during repository creation. - Adds unit and end-to-end integration coverage for create-on-push behavior.
| File | Description |
|---|---|
sdk/src/prompts.rs |
Normalizes current-workspace handles and adds unit tests. |
cli/tests/prompt_structured_test.rs |
Verifies push, pull, and cleanup for a newly created repository. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
codekiln
left a comment
There was a problem hiding this comment.
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)
Three findings on the title and body, which have no line to sit on, and one inline.
1. Title. Quoted: "send the repo name, not -/, when push creates a repo". It makes the reader hold -/<name> as an option nobody proposed in order to learn one fact. Rule: My/AI/Rule/State the Positive. Rewrite: 🐛 fix(prompt): create the repo as <name> when prompt push uses --owner -. Use the same subject as the squash-merge subject.
2. Heading. Quoted: "## Decisions for codekiln: leave owner/name creation as it was". Read alone, "owner/name creation" and "as it was" each send the reader into the section to learn what is being left alone. The heading of a decision should be the decision. Rule: My/AI/Rule/How to Communicate Effectively With Me/Use Headings to Serve a Purpose. Rewrite: ## Handles with a named owner are still sent as given.
3. Pointer with no link or title. Quoted: "It pushes with --schema because the plain push is fixed separately in #823; this branch does not depend on that PR." A bare #823 says nothing about the request, and the second clause answers a question nobody asked. Rule: My/AI/Rule/How to Communicate Effectively With Me/A pointer carries a link, an id with a slug, and a reason. Rewrite: It pushes with --schemabecause a push without--schema has its own bug, fixed in [🐛 fix(prompt): send a ChatPromptTemplate manifest when pushing without --schema · Pull Request #823](https://github.com/codekiln/langstar/pull/823).
Checked and found accurate: the three pinned links and line anchors in the body (parse_prompt_identifier at utils.py L711-L743, "repo_handle": prompt_name at client.py L9904, RepoNewParams at repo.go L310-L320) land on the code the body describes.
#820 🐛 fix(prompt): prompt push --owner - fails when it creates a new repo Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)AI (codekiln-review): Edited the title and body for the three findings.
|
codekiln
left a comment
There was a problem hiding this comment.
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)
Two findings on the body, which has no line to sit on. Everything else read clean: the title, the doc comments, the test and the earlier rounds' rewrites. The three pinned upstream links and their line anchors land on the code the body describes.
1. Decision paragraph. Quoted: "This change strips only a leading -/. A named owner (acme/name) is still sent as given, and the API's error message suggests it rejects a slash too. Upstream drops every owner, so sending the bare name for any owner is probably right. I could not test a named owner because the workspace has no handle; upstream accepts only - or the tenant's own handle and sends only the bare name. Recommendation: a follow-up issue." Cost: the reader meets "upstream" twice saying the same thing, has to work out what "it" and "a follow-up issue" are (no link, no title, no statement of what the issue would change), and the reason for not testing is followed by a clause that has nothing to do with testing. The heading above it already says the first sentence. Rule: My/AI/Rule/How to Communicate Effectively With Me/Never make the reader resolve a reference and My/AI/Rule/Prune useless commandments. Rewrite, to replace the whole paragraph: A handle with a named owner, such as acme/name, still goes to the API unchanged, and the API's error message lists no slash among the allowed characters, so the API probably rejects it. langsmith-sdk sends only the bare name whatever the owner, so I recommend a follow-up issue that does the same here. I could not test a named owner because this workspace has no handle.
2. Heading and first bullet of the upstream section. Quoted: "## How langsmith-sdk and langsmith-go send the handle" and "- langsmith-sdk splits the identifier with parse_prompt_identifier, accepts an owner of - or the tenant's own handle, and posts "repo_handle": prompt_name, so the owner never reaches the API." Read alone, the heading names two packages the reader has not been told are LangChain's Python and Go clients, and "the handle" could be any handle; the bullet repeats the unintroduced name, then "the identifier" and "the tenant's own handle". The point of the section is that both clients send only the repo name. Rule: My/AI/Rule/How to Communicate Effectively With Me/Use Headings to Serve a Purpose and My/Pref/Writing/Be specific and explicit. Rewrite: heading ## LangChain's Python and Go clients send only the repo name, first bullet The Python client, langsmith-sdk, splits owner/name with [parse_prompt_identifier](<the existing pinned link>), accepts an owner of - or the workspace's own handle, and posts only the name as ["repo_handle": prompt_name](<the existing pinned link>), so the owner never reaches the API.
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)AI (codekiln-review): I rewrote the two body passages the review named. I checked the facts first: the API's 400 message lists lowercase alphanumeric, hyphen, underscore and a UUID, with no slash.
|
codekiln
left a comment
There was a problem hiding this comment.
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)
One finding on the body, which has no line to sit on. The title, the code, the doc comments, the test, the earlier rounds' rewrites and the three pinned upstream links with their line anchors read clean.
1. Decision paragraph. Quoted: "langsmith-sdk sends only the bare name whatever the owner, so I recommend a follow-up issue that does the same here." Cost: an issue cannot send a name, so the reader works out that the subject should be a change to create_repo; and "whatever the owner" is broader than the code the body pins, because create_prompt calls _current_tenant_is_owner and raises an error for any owner other than - or the workspace's own handle, which the section above states itself. The first sentence of the paragraph also lists what the error message leaves out ("lists no slash") instead of what it allows. Rules: My/Pref/Writing/Give every sentence a subject that can do its verb, My/Pref/Writing/Be specific and explicit and My/AI/Rule/State the Positive. Rewrite of the paragraph: A handle with a named owner, such as acme/name, still goes to the API unchanged. The API's error message allows only lowercase letters, digits, hyphens, underscores or a UUID, so it probably rejects that handle. langsmith-sdk sends only the bare name when the owner is -or the workspace's own handle, so I recommend filing a follow-up issue to makecreate_repo do the same for the workspace's own handle. I could not test a named owner because this workspace has no handle.
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)AI (codekiln-review): I rewrote the Decisions paragraph as the review asked. I checked it against
The paragraph's first two sentences now say what the API allows ("lowercase letters, digits, hyphens, underscores or a UUID") instead of what it leaves out. |
codekiln
left a comment
There was a problem hiding this comment.
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)
Two findings on the body, which has no line to sit on. The title, the code, the doc comments, the test, the earlier rounds' rewrites, the pinned upstream links and their line anchors (checked against the pinned commits with gh api) and the claims about _current_tenant_is_owner and the bare repo_handle read clean.
1. Decision paragraph, second sentence. Quoted: "The API's error message allows only lowercase letters, digits, hyphens, underscores or a UUID, so it probably rejects that handle." Cost: an error message cannot allow anything, so the reader works out that the API allows and the message only reports it, and "that handle" sends them back one sentence. Rule: My/Pref/Writing/Give every sentence a subject that can do its verb. Rewrite: According to its 400 message, the API accepts only a handle of lowercase letters, digits, hyphens and underscores, or a UUID, so it probably rejects acme/name.
2. Test section, fourth sentence. Quoted: "Without the fix in this pull request, it fails with the 404 above." Cost: "the 404 above" sends the reader back past a heading to the first paragraph to learn which 404. Rule: My/AI/Rule/How to Communicate Effectively With Me/Never make the reader resolve a reference. Rewrite: Without the fix, the push fails with 404 "Repository not found".
from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)AI (codekiln-review): Both findings on the body are fixed.
The 404 text and the 400 wording match what the first paragraph of the body already states. |
| push_cmd.env("HOME", home::empty_home()); | ||
| push_cmd.args([ | ||
| "prompt", | ||
| "push", | ||
| "--owner", | ||
| "-", | ||
| "--repo", | ||
| &repo_name, | ||
| "--template", | ||
| "Owner dash push: {input}", | ||
| "--input-variables", | ||
| "input", | ||
| "--schema", | ||
| schema_path, | ||
| ]); | ||
| push_cmd.assert().success(); | ||
|
|
||
| // VERIFY: the pulled manifest has the pushed template and schema | ||
| let mut pull_cmd = Command::new(&bin); | ||
| pull_cmd.env("HOME", home::empty_home()); |

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)
Fixes #820
langstar prompt push --owner - --repo <name>to a repo that does not exist yet sent-/<name>as the new repo'srepo_handle, and the API answered 400 "handle must be lowercase alphanumeric, hyphen, or underscore, starting with a-z, or a valid UUID". The CLI printed that as a warning, pushed anyway, and failed with 404 "Repository not found".create_repoinsdk/src/prompts.rsnow sends only<name>for a-/<name>handle.Test
test_cli_push_creates_new_repo_with_owner_dashincli/tests/prompt_structured_test.rspushes a structured prompt with--owner -to a repo that does not exist, pulls it back as JSON and asserts on the template and schema title. It deletes the repo afterwards. Without the fix, the push fails with 404 "Repository not found". It pushes with--schemabecause a push without--schemahas its own bug, fixed in 🐛 fix(prompt): send a ChatPromptTemplate manifest when pushing without --schema · Pull Request #823.LangChain's Python and Go clients send only the repo name
owner/namewithparse_prompt_identifier, accepts an owner of-or the workspace's own handle, and posts only the name as"repo_handle": prompt_name, so the owner never reaches the API.RepoNewParamshas arepo_handlefield and no owner field, so its caller passes a bare name.Decisions for codekiln
Handles with a named owner are still sent as given
A handle with a named owner, such as
acme/name, still goes to the API unchanged. According to its 400 message, the API accepts only a handle of lowercase letters, digits, hyphens and underscores, or a UUID, so it probably rejectsacme/name. langsmith-sdk sends only the bare name when the owner is-or the workspace's own handle, so I recommend filing a follow-up issue to makecreate_repodo the same for the workspace's own handle. I could not test a named owner because this workspace has no handle.🤖 Generated with Claude Code