Repository navigation
feat: create skills during sync, and read again after a 409 on create - #940
Conversation
7258eff to
b9a3396
Compare
b9a3396 to
f3f372d
Compare
f3f372d to
027c99c
Compare
027c99c to
fcdc2af
Compare
b0f6757 to
4c40cdf
Compare
| body := skillMutationRequest{Description: attachment.Skill.Description, Markdown: attachment.Skill.Markdown} | ||
| request.body = body |
There was a problem hiding this comment.
not sure if this added anything lol but it didn't hurt anything
There was a problem hiding this comment.
body is shared by both paths. The create branch below wraps it in skillCreateRequest with the key and name, so the description and markdown are built once.
|
|
||
| // displayName converts a key such as "support-agent" to "Support agent". | ||
| func displayName(key string) string { | ||
| runes := []rune(strings.NewReplacer("-", " ", "_", " ").Replace(key)) | ||
| if len(runes) != 0 { | ||
| runes[0] = unicode.ToUpper(runes[0]) | ||
| } | ||
| return string(runes) | ||
| } |
|
|
||
| func (resolver *attachmentResolver) publish(projectKey string, local syncdomain.Attachment) (syncdomain.Attachment, error) { | ||
| remote, err := resolver.client.ReadAttachment(projectKey, local.Kind, local.Key()) | ||
| remote, readErr := resolver.client.ReadAttachment(projectKey, local.Kind, local.Key()) |
There was a problem hiding this comment.
I like this more specific rename of the error. makes it more readable.
| ) | ||
| case syncapi.IsNotFound(readErr): | ||
| return syncdomain.Attachment{}, fmt.Errorf( | ||
| "skill %q does not exist in LaunchDarkly; add upsert: true to its front matter to create it during sync", |
There was a problem hiding this comment.
might be good to include the command to create it?
There was a problem hiding this comment.
There is no create command in this stack yet, so upsert: true followed by the sync that is already running is the only create path. A later PR in this series adds ldcli sync skills add, which creates a skill from flags, and that PR can name the command in this message and in the tool version.
| return remote, nil | ||
| case err == nil: | ||
| case readErr == nil: | ||
| writeErr = resolver.client.UpdateAttachment(projectKey, local) |
There was a problem hiding this comment.
oh nice, read error aligns well to the writeError
| create = `To create it during sync, add "upsert": true to its file. ` | ||
| consent := `add "upsert": true to its file` | ||
| if id.Kind == syncdomain.KindSkill { | ||
| consent = "add upsert: true to its front matter" |
There was a problem hiding this comment.
the exact command might be helpful here
There was a problem hiding this comment.
Same as the other thread: in this stack the file edit is the only way to give consent, and the next sync then creates the resource. The --create flag and ldcli sync skills add come in later PRs, and those can add the exact command to this message.
There was a problem hiding this comment.
Good call, i will make sure we capture adding the commands to later PRs that actually create the skill
e36972a to
fd6a92d
Compare
A skill file can now set upsert: true and name:. A variation push that uses a missing skill with upsert creates it with the name from the file, or a name made from the key. Neither key is part of the fingerprint, and a pull keeps both lines. LaunchDarkly keeps the name after the create. The error for a missing skill now also gives the upsert step. A 409 on the create of a tool, a skill, or a variation now reads the resource again: equal content succeeds, and different content fails.
fd6a92d to
dc3d694
Compare
Context
ldcli sync promptskeeps local prompt files in step with the variations of LaunchDarkly AI Configs. A variation can use tools and skills. Locally, a skill is a Markdown file with YAML front matter.Sync can create a missing tool when its file has
"upsert": true, but it cannot create a skill.api.Client.CreateAttachmentreturnscreating skills is not supported, and a skill file has noupsertkey. So a variation that uses a new local skill fails with a 404 on apply.The work to apply tool and skill rows was too large for one PR. This PR adds skill creation, and #941 applies the rows.
What changes
upsert: true, which lets sync create a missing skill, andname:. Sync still rejects every other key.CreateAttachmentcreates a skill withPOST /api/v2/projects/{projectKey}/ai-configs/skills. It sendskey,name,description, andmarkdown.name:, sync makes a name from the key. For example,incident-summarybecomesIncident summary.PATCHsends no name, a rename in LaunchDarkly makes no plan row, and a pull never writes the LaunchDarkly name into a file.name:line andupsert:line in a skill file.upsertfails withskill "handoff" does not exist in LaunchDarkly; add upsert: true to its front matter to create it during sync.new, name "Human handoff".A skill file that sync can create, at
.launchdarkly/production/skills/handoff.md:With this file,
ldcli sync prompts --yescreates the skillHuman handoffin LaunchDarkly. The variation pins version 1, and the file keeps both new lines.Design notes
CanonicalAttachmentalready clears the skill name andUpsert, so existing fingerprints andsync.lockrows do not change.syncdomain.DisplayNamemakes the name from the key. This PR moved it without changes fromlink.displayName.preserveToolUpsertis nowpreserveLocalFields, and it handles skill files too. It parses the existing skill file, as it already did for tool files.watch, can create the same key first. So a 409 on create reads the resource again. Different content fails, and the next run shows a conflict.renderAttachmentnever writesname:, because the rendered content comes from LaunchDarkly in every path today. A later PR adds a create throughadd, and that create writesname:.Review focus
name:andupsert:in a skill file?Verification
go test ./...golangci-lint run(v2.9.0)TestPromptCreatesMissingSkillWhenUpsertIsEnabledrunssync prompts --yeswith the skill file above. LaunchDarkly gets the skill with that name, the variation pins version 1, and the local file keeps both lines.TestPublishReadsAgainAfterACreateConflictandTestApplyServerChangeReadsAgainAfterAVariationCreateConflictcover the 409 read-back for a tool and for a variation.linktests pass without edits after theDisplayNamemove.ldcliand ran the same flow with a skill file withoutname:against a local stand-in for the LaunchDarkly API. The request log showsPOST ai-configs/skillswith the nameIncident summary, and then the variationPATCHthat pins version 1.Related changes
--diffflagNote
Overview
Enables creating missing AI skills during
ldcli sync, mirroring the existing toolupsertflow. Skill Markdown front matter can set optionalupsert: trueandname:; create requests sendkey,name(from the file orDisplayNameon the key),description, andmarkdown, while updates still omitnamebecause LaunchDarkly owns it after create. Planning ignores remote skill renames, and pulls keep localname/upsertvia generalizedpreserveLocalFields.Hardens ambiguous creates: a 409 on create for tools, skills, or variations triggers a follow-up read—success if content matches, otherwise the existing concurrent-change error. User-facing plan and error text now describe skill upsert and show the resolved create name (e.g.
new, name "Human handoff").Reviewed by Cursor Bugbot for commit b0f6757. Bugbot is set up for automated code reviews on this repo. Configure here.