mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-10 12:07:09 +02:00
fix(skills): ship the completion helper with the installed skill (#15554)
## Thinking Path > - Paperclip manages work for AI agents. > - Legacy agents use the Paperclip skill to save task status and comments. > - The skill names a script relative to the task workspace. > - That script exists only in the Paperclip source repository. > - Agents in other workspaces can hit a missing command or search for it. > - This PR ships the helper inside the skill and uses the installed skill path. > - The repository command remains available through a forwarding wrapper. ## Linked Issues or Issue Description Fixes #9527. Refs #15548 for the preceding runtime checkout guidance. Related: #6052 addresses LF line endings for the repository helper; this change addresses helper delivery and path resolution. ## What Changed - Bundle the existing issue update helper with the Paperclip skill. Preserve its HTTP checks, echoed-status check and two-attempt limit. - Resolve the command from the installed skill directory. Use a verified PATCH when that path is unavailable, without searching the filesystem. - Keep the repository command as a wrapper that works from any directory. - Test shell execution and exact status/comment payloads through both provider skill-home layouts, including paths with spaces. - Add helper sources and existing verification tests to stock-harness admission. Record an absent historical helper explicitly. Add the missing declaration for the admission fingerprint export. ## Verification - Complete directly affected source suites: 30 tests pass. They cover skill delivery, preserved multiline comments and links, authentication headers, empty responses, mismatched status, transient retries and definitive rejections. - Product E2E typecheck passes. Support suites: 1,835 Vitest tests pass, one is skipped; 128 Node tests pass. - Full local build and workspace typecheck pass. - Full local repository tests are not claimed as passed. Embedded PostgreSQL was unavailable in this worktree during the preceding task; Linux CI will run the repository gates. - The authorized matched Codex/Claude comparison is pending. It uses the existing assigned-skill case and original oracle, one initial attempt per profile and variant. - CI and a fresh Greptile review are pending. Keep this PR in draft until readiness gates complete. ## Risks - Correct path resolution depends on the harness supplying the installed skill path. The instructions use verified PATCH when that path is unavailable. - The helper still requires Bash, curl and jq. Its existing retry and response-verification behavior is unchanged. - Tests use the shared skill-directory symlink mechanism and an HTTP fixture. Real provider completion behavior still requires the bounded live comparison. - This fix does not redesign native completion, legacy recovery or ambiguous transport handling. ## Model Used OpenAI Codex, GPT-6 family. The exact model build and context window are not exposed in this session. Used code editing, shell tools and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
71cd0a2621
commit
2f0c485dec
9 files changed
+271
-176
No files matched your search
@@ -1,166 +1,7 @@
|
||||
#!/usr/bin/env bash
|
||||
|
||||
# Keep the repository command compatible; the deployable implementation belongs
|
||||
# to the skill so agents do not need a checkout of this repository.
|
||||
set -euo pipefail
|
||||
|
||||
usage() {
|
||||
cat <<'EOF'
|
||||
Usage:
|
||||
scripts/paperclip-issue-update.sh [--issue-id ID] [--status STATUS] [--comment TEXT] [--dry-run]
|
||||
|
||||
Reads a multiline markdown comment from stdin when stdin is piped. This preserves
|
||||
newlines when building the JSON payload for PATCH /api/issues/{issueId}.
|
||||
|
||||
Examples:
|
||||
scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status in_progress <<'MD'
|
||||
Investigating formatting
|
||||
|
||||
- Pulled the raw comment body
|
||||
- Comparing it with the run transcript
|
||||
MD
|
||||
|
||||
scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status done --dry-run <<'MD'
|
||||
Done
|
||||
|
||||
- Fixed the issue update helper
|
||||
MD
|
||||
EOF
|
||||
}
|
||||
|
||||
require_command() {
|
||||
if ! command -v "$1" >/dev/null 2>&1; then
|
||||
printf 'Missing required command: %s\n' "$1" >&2
|
||||
exit 1
|
||||
fi
|
||||
}
|
||||
|
||||
issue_id="${PAPERCLIP_TASK_ID:-}"
|
||||
status=""
|
||||
comment_arg=""
|
||||
dry_run=0
|
||||
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case "$1" in
|
||||
--issue-id)
|
||||
issue_id="${2:-}"
|
||||
shift 2
|
||||
;;
|
||||
--status)
|
||||
status="${2:-}"
|
||||
shift 2
|
||||
;;
|
||||
--comment)
|
||||
comment_arg="${2:-}"
|
||||
shift 2
|
||||
;;
|
||||
--dry-run)
|
||||
dry_run=1
|
||||
shift
|
||||
;;
|
||||
--help|-h)
|
||||
usage
|
||||
exit 0
|
||||
;;
|
||||
*)
|
||||
printf 'Unknown argument: %s\n' "$1" >&2
|
||||
usage >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
done
|
||||
|
||||
if [[ -z "$issue_id" ]]; then
|
||||
printf 'Missing issue id. Pass --issue-id or set PAPERCLIP_TASK_ID.\n' >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
comment=""
|
||||
if [[ -n "$comment_arg" ]]; then
|
||||
comment="$comment_arg"
|
||||
elif [[ ! -t 0 ]]; then
|
||||
comment="$(cat)"
|
||||
fi
|
||||
|
||||
require_command jq
|
||||
|
||||
payload="$(
|
||||
jq -nc \
|
||||
--arg status "$status" \
|
||||
--arg comment "$comment" \
|
||||
'
|
||||
(if $status == "" then {} else {status: $status} end) +
|
||||
(if $comment == "" then {} else {comment: $comment} end)
|
||||
'
|
||||
)"
|
||||
|
||||
if [[ "$dry_run" == "1" ]]; then
|
||||
printf '%s\n' "$payload"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if [[ -z "${PAPERCLIP_API_URL:-}" || -z "${PAPERCLIP_API_KEY:-}" || -z "${PAPERCLIP_RUN_ID:-}" ]]; then
|
||||
printf 'Missing PAPERCLIP_API_URL, PAPERCLIP_API_KEY, or PAPERCLIP_RUN_ID.\n' >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# A successful PATCH always returns the updated issue JSON. An empty body or a
|
||||
# connection-level failure means the write did NOT land, even when a pipeline
|
||||
# exit code says otherwise, so verify the response instead of inferring success.
|
||||
# Two attempts total: the shared heartbeat policy stops a control-plane write
|
||||
# after two consecutive failures, so the helper must not send a third.
|
||||
max_attempts=2
|
||||
attempt=1
|
||||
while :; do
|
||||
http_code=""
|
||||
body=""
|
||||
set +e
|
||||
response="$(
|
||||
curl -sS -m 30 -X PATCH \
|
||||
"$PAPERCLIP_API_URL/api/issues/$issue_id" \
|
||||
-H "Authorization: Bearer $PAPERCLIP_API_KEY" \
|
||||
-H "X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID" \
|
||||
-H 'Content-Type: application/json' \
|
||||
--data-binary "$payload" \
|
||||
-w '\n%{http_code}'
|
||||
)"
|
||||
curl_exit=$?
|
||||
set -e
|
||||
|
||||
if [[ "$curl_exit" -eq 0 ]]; then
|
||||
http_code="${response##*$'\n'}"
|
||||
body="${response%$'\n'*}"
|
||||
fi
|
||||
|
||||
if [[ "$curl_exit" -eq 0 && "$http_code" == 2* ]]; then
|
||||
if [[ -z "$body" ]]; then
|
||||
printf 'Issue update FAILED: HTTP %s with an empty response body. A real update echoes the issue JSON; treat this write as not saved.\n' "$http_code" >&2
|
||||
exit 1
|
||||
fi
|
||||
if [[ -n "$status" ]]; then
|
||||
returned_status="$(jq -r '.status // empty' <<<"$body" 2>/dev/null || true)"
|
||||
if [[ "$returned_status" != "$status" ]]; then
|
||||
printf 'Issue update FAILED: server echoed status %s instead of requested %s.\n' "${returned_status:-<none>}" "$status" >&2
|
||||
printf '%s\n' "$body" >&2
|
||||
exit 1
|
||||
fi
|
||||
fi
|
||||
printf '%s\n' "$body"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# 4xx (other than 429) is a definitive rejection; retrying cannot change it.
|
||||
if [[ "$curl_exit" -eq 0 && "$http_code" == 4* && "$http_code" != "429" ]]; then
|
||||
printf 'Issue update rejected (HTTP %s).\n' "$http_code" >&2
|
||||
[[ -n "$body" ]] && printf '%s\n' "$body" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if (( attempt >= max_attempts )); then
|
||||
printf 'Issue update FAILED after %d attempts (curl exit %s, HTTP %s). The status/comment was NOT saved — report this write as failed, do not assume it landed.\n' "$max_attempts" "$curl_exit" "${http_code:-000}" >&2
|
||||
[[ -n "$body" ]] && printf '%s\n' "$body" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
printf 'Issue update attempt %d/%d failed (curl exit %s, HTTP %s); retrying...\n' "$attempt" "$max_attempts" "$curl_exit" "${http_code:-000}" >&2
|
||||
sleep $((attempt * 2))
|
||||
attempt=$((attempt + 1))
|
||||
done
|
||||
paperclip_repo_scripts="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
|
||||
exec bash "$paperclip_repo_scripts/../skills/paperclip/scripts/paperclip-issue-update.sh" "$@"
|
||||
Reference in new issue
Block a user