refactor(hooks): consume normalized skill activations - #5086
Conversation
|
|
|
||||||||||||||||
|
|
||||||||||||||||
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/relay/skills.go">
<violation number="1" location="hooks/relay/skills.go:35">
P1: Implicit Codex/Cursor skill completions never reach the registry or uploader: their content is available only on `ToolPost`, but that event has no `payload.Data.Skill` and this resolver exits before consuming the activation. Propagate a skill block/hash for content-bearing normalized completions while retaining `tool.completed`, and cover a post-read delivery.</violation>
<violation number="2" location="hooks/relay/skills.go:88">
P2: `$name` fallback no longer recognizes bundled skills under `/etc/codex/skills/.system` or `/opt/codex/skills/.system`, so those prompt activations are silently omitted. Include each standard root’s `.system` child in this lookup.</violation>
</file>
<file name="hooks/relay/drain.go">
<violation number="1" location="hooks/relay/drain.go:208">
P1: Existing v1 offline skill entries lose their content upload after upgrade: legacy entries have source path/root but no tool output, so this reconstruction returns nil and drain removes them after ingest acceptance. Add a legacy migration/replay path (or version and explicitly preserve legacy entries) before treating all v1 entries as output-backed.</violation>
</file>
<file name="hooks/relay/runner.go">
<violation number="1" location="hooks/relay/runner.go:236">
P2: The live skill-content upload is now synchronous on the relay handler, extending the gating path. Previously this line called `startSkillContentUpload`, which launched a detached child process (`cmd.Start` + `Process.Release`) and returned immediately — the code explicitly stated "content upload never extends the hook's gating path." Now `uploadSkillContent` runs inline against the network with a 30s `skillUploadBudget`, an internal per-attempt retry loop, and SDK backoff up to `retryMaxElapsedMS` (30s). Every tool event that captures a skill therefore blocks the agent's hook round-trip for up to ~30s whenever the upload endpoint is slow or unreachable (the failure is only `debugf`-logged afterward). Consider bounding this delivery well below the gating budget, or deferring it, so a content-upload stall cannot stall the agent's tool loop.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| func captureResolvedSkill(result *resolvedSkill, location skillLocation) *resolvedSkill { | ||
| file, authorizedRoot, ok := openValidatedSkill(location) | ||
| if !ok { | ||
| activation := agenthooks.SkillActivationOf(typed) |
There was a problem hiding this comment.
P1: Implicit Codex/Cursor skill completions never reach the registry or uploader: their content is available only on ToolPost, but that event has no payload.Data.Skill and this resolver exits before consuming the activation. Propagate a skill block/hash for content-bearing normalized completions while retaining tool.completed, and cover a post-read delivery.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/relay/skills.go, line 35:
<comment>Implicit Codex/Cursor skill completions never reach the registry or uploader: their content is available only on `ToolPost`, but that event has no `payload.Data.Skill` and this resolver exits before consuming the activation. Propagate a skill block/hash for content-bearing normalized completions while retaining `tool.completed`, and cover a post-read delivery.</comment>
<file context>
@@ -18,218 +15,38 @@ import (
-func captureResolvedSkill(result *resolvedSkill, location skillLocation) *resolvedSkill {
- file, authorizedRoot, ok := openValidatedSkill(location)
- if !ok {
+ activation := agenthooks.SkillActivationOf(typed)
+ if activation == nil || !activation.ContentAvailable || len(activation.Content) > maxSkillContentBytes || !utf8.ValidString(activation.Content) {
return result
</file context>
| } | ||
| return &resolvedSkill{ | ||
| content: "", | ||
| content: content, |
There was a problem hiding this comment.
P1: Existing v1 offline skill entries lose their content upload after upgrade: legacy entries have source path/root but no tool output, so this reconstruction returns nil and drain removes them after ingest acceptance. Add a legacy migration/replay path (or version and explicitly preserve legacy entries) before treating all v1 entries as output-backed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/relay/drain.go, line 208:
<comment>Existing v1 offline skill entries lose their content upload after upgrade: legacy entries have source path/root but no tool output, so this reconstruction returns nil and drain removes them after ingest acceptance. Add a legacy migration/replay path (or version and explicitly preserve legacy entries) before treating all v1 entries as output-backed.</comment>
<file context>
@@ -190,17 +190,25 @@ func drainSpool(ctx context.Context, dir string) DrainSummary {
}
return &resolvedSkill{
- content: "",
+ content: content,
rawSHA256: *entry.Envelope.Data.Skill.RawSha256,
- sourcePath: *entry.Envelope.Data.Skill.SourcePath,
</file context>
| return false | ||
| } | ||
| home, _ := os.UserHomeDir() | ||
| roots := []string{"/etc/codex/skills", "/opt/codex/skills"} |
There was a problem hiding this comment.
P2: $name fallback no longer recognizes bundled skills under /etc/codex/skills/.system or /opt/codex/skills/.system, so those prompt activations are silently omitted. Include each standard root’s .system child in this lookup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/relay/skills.go, line 88:
<comment>`$name` fallback no longer recognizes bundled skills under `/etc/codex/skills/.system` or `/opt/codex/skills/.system`, so those prompt activations are silently omitted. Include each standard root’s `.system` child in this lookup.</comment>
<file context>
@@ -249,327 +66,52 @@ func codexPromptSkill(prompt, cwd string) (string, skillLocation) {
+ return false
}
home, _ := os.UserHomeDir()
+ roots := []string{"/etc/codex/skills", "/opt/codex/skills"}
if home != "" {
- root := filepath.Join(home, ".agents", "skills")
</file context>
| roots := []string{"/etc/codex/skills", "/opt/codex/skills"} | |
| roots := []string{ | |
| "/etc/codex/skills", filepath.Join("/etc/codex/skills", ".system"), | |
| "/opt/codex/skills", filepath.Join("/opt/codex/skills", ".system"), | |
| } |
| commitPromptAttachmentHighWater(promptAttachmentAdvance) | ||
| } | ||
| if err := startSkillContentUpload(finalCreds, res, resolvedSkill); err != nil { | ||
| if err := uploadSkillContent(ctx, finalCreds, res, resolvedSkill); err != nil { |
There was a problem hiding this comment.
P2: The live skill-content upload is now synchronous on the relay handler, extending the gating path. Previously this line called startSkillContentUpload, which launched a detached child process (cmd.Start + Process.Release) and returned immediately — the code explicitly stated "content upload never extends the hook's gating path." Now uploadSkillContent runs inline against the network with a 30s skillUploadBudget, an internal per-attempt retry loop, and SDK backoff up to retryMaxElapsedMS (30s). Every tool event that captures a skill therefore blocks the agent's hook round-trip for up to ~30s whenever the upload endpoint is slow or unreachable (the failure is only debugf-logged afterward). Consider bounding this delivery well below the gating budget, or deferring it, so a content-upload stall cannot stall the agent's tool loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/relay/runner.go, line 236:
<comment>The live skill-content upload is now synchronous on the relay handler, extending the gating path. Previously this line called `startSkillContentUpload`, which launched a detached child process (`cmd.Start` + `Process.Release`) and returned immediately — the code explicitly stated "content upload never extends the hook's gating path." Now `uploadSkillContent` runs inline against the network with a 30s `skillUploadBudget`, an internal per-attempt retry loop, and SDK backoff up to `retryMaxElapsedMS` (30s). Every tool event that captures a skill therefore blocks the agent's hook round-trip for up to ~30s whenever the upload endpoint is slow or unreachable (the failure is only `debugf`-logged afterward). Consider bounding this delivery well below the gating budget, or deferring it, so a content-upload stall cannot stall the agent's tool loop.</comment>
<file context>
@@ -235,11 +229,11 @@ func (r *Relay) deliver(ctx context.Context, typed any) (ingestResult, authState
commitPromptAttachmentHighWater(promptAttachmentAdvance)
}
- if err := startSkillContentUpload(finalCreds, res, resolvedSkill); err != nil {
+ if err := uploadSkillContent(ctx, finalCreds, res, resolvedSkill); err != nil {
r.debugf("skill upload: %v", err)
}
</file context>
1 participant
Depends on speakeasy-api/agenthooks#13.
Consume
agenthooks.SkillActivationOfas the single projection for tool-based skill activations. The normalized tool output continues through ordinary hook ingestion, and the same event content supplies the hash and payload used by the skill-content registry and offline replay.Delete Gram’s Claude/Codex/Cursor manifest readers, path classification, symlink authorization, detached file-reopen uploader, and internal
upload-skillcommand. Keep only the separate Codex$skill-nameprompt fallback because it has no tool event.Summary by cubic
Consume normalized skill activations via
agenthooks.SkillActivationOfand route tool output through standard hook ingestion. This removes path-based skill detection and the detached upload flow, simplifying skill capture and replay.Refactors
agenthooks.SkillActivationOfto populatedata.skilland emitskill.activatedwhen explicit.rawSHA256from that.SourcePathandSourceLevelare now nil.upload-skillcommand.$skill-nameprompt fallback.finishExchange,spoolUnsent) to the new flow.github.com/speakeasy-api/agenthooksto a version that provides normalized activations.Migration
upload-skillCLI, remove those calls; skill uploads now use captured tool output directly.SourcePath/SourceLevelin payloads; they are unset with the normalized model. Ensure downstream consumers accept this.Written for commit 5a2bd00. Summary will update on new commits.