Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions .github/workflows/qwen-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,22 @@ jobs:
OPENAI_MODEL: '${{ vars.QWEN_TRIAGE_MODEL || vars.QWEN_PR_REVIEW_MODEL }}'
run: |-
set -euo pipefail
if [ -n "${OPENAI_MODEL:-}" ]; then
sed -i "s/qwen3\.7-max/${OPENAI_MODEL}/g" .qwen/skills/triage/references/pr-workflow.md
if [ -z "${OPENAI_MODEL:-}" ]; then
exit 0
fi
TARGET=.qwen/skills/triage/references/pr-workflow.md
# The default signature literal must still exist in the skill, or
# this injection silently no-ops and every staged comment ships the
# wrong model name. Fail loudly so a skill edit that renames the
# default surfaces here instead of in production comments.
if ! grep -q 'qwen3\.7-max' "$TARGET"; then
echo "::error::Signature literal 'qwen3.7-max' not found in $TARGET — update this step's sed target to match the skill."
exit 1
fi
# Escape sed replacement metacharacters (& / \) so a model name
# containing them cannot corrupt the skill text.
ESCAPED=$(printf '%s' "$OPENAI_MODEL" | sed -e 's/[&/\]/\\&/g')
sed -i "s/qwen3\.7-max/${ESCAPED}/g" "$TARGET"

- name: 'Run Qwen Triage'
id: 'triage'
Expand Down Expand Up @@ -626,8 +639,8 @@ jobs:
set -uo pipefail
MARKER='<!-- qwen-triage stage=status -->'
if [ "${TRIAGE_OUTCOME:-}" = 'success' ]; then
EN="✅ **Qwen Triage finished** — [view run]($RUN_URL). See the stage comments above for the result."
ZH="✅ **Qwen Triage 已完成** —— [查看运行]($RUN_URL)。结果见上方各阶段评论。"
EN="✅ **Qwen Triage finished** — [view run]($RUN_URL). See the stage comments in this thread for the result."
ZH="✅ **Qwen Triage 已完成** —— [查看运行]($RUN_URL)。结果见本线程中的各阶段评论。"
else
EN="⚠️ **Qwen Triage ended early** — [view run]($RUN_URL). It stopped before finishing; check the run log."
ZH="⚠️ **Qwen Triage 提前结束** —— [查看运行]($RUN_URL)。未跑完,请查看运行日志。"
Expand Down
13 changes: 11 additions & 2 deletions .qwen/skills/triage/references/pr-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,21 @@ or Stage 1c direction escalation), submit exactly one `CHANGES_REQUESTED`
review and stop. Do not also post or update a Stage 1 issue comment, and do not
continue to Stage 2, Stage 3, or approval.

**Re-runs:** if the triage runs again on the same PR, update each comment in place:
**Re-runs:** if the triage runs again on the same PR, update each comment in place. **Resolve the comment id by its stage marker AT PATCH TIME — never from memory, list position, or an earlier stage's bookkeeping.** On a re-run the thread holds four or more bot comments whose list order is not the stage order, and a wrong id silently overwrites another stage's comment (observed on a real re-run: the stage=3 comment clobbered with stage=1 content mid-run). The author filter matters too — the marker is public text anyone can paste into a comment, and the bot PAT may be able to edit other users' comments:

```bash
gh api -X PATCH "/repos/$REPO/issues/comments/$COMMENT_ID" -F body=@/tmp/stage-N-updated.md
BOT_LOGIN=$(gh api user --jq '.login')
stage_comment_id() { # $1 = stage number (1, 2, 3) or "status"
gh api "repos/$REPO/issues/$PR_NUMBER/comments" --method GET --paginate -F per_page=100 |
Comment thread
wenshao marked this conversation as resolved.
jq -rs --arg bot "$BOT_LOGIN" --arg m "<!-- qwen-triage stage=$1 -->" \
'[.[][] | select(.user.login == $bot) | select(.body | startswith($m))] | last | .id // empty'
}
CID=$(stage_comment_id 2) # re-resolve immediately before EACH patch
[ -n "$CID" ] && gh api -X PATCH "/repos/$REPO/issues/comments/$CID" -F body=@/tmp/stage-2-updated.md
```

`startswith`, not `contains`: stage comments carry their marker as the first line, and a substring match would also hit a comment that merely quotes the marker. An empty `CID` means that stage has no comment yet — POST a new one instead of patching.

Never create duplicates. For terminal-exit reviews (submitted via
`gh pr review --request-changes`), the GitHub API does not support editing PR
reviews. On re-run: check if a `CHANGES_REQUESTED` review from the bot already
Expand Down
56 changes: 55 additions & 1 deletion scripts/tests/qwen-triage-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@
*/

import { spawnSync } from 'node:child_process';
import { readFileSync } from 'node:fs';
import {
mkdirSync,
mkdtempSync,
readFileSync,
rmSync,
writeFileSync,
} from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { describe, expect, it } from 'vitest';

const workflow = readFileSync('.github/workflows/qwen-triage.yml', 'utf8');
Expand Down Expand Up @@ -306,4 +314,50 @@ describe('qwen-triage tmux workflow', () => {
workflow.indexOf("- name: 'Install tmux runner tools'"),
).toBeLessThan(workflow.indexOf("- name: 'Checkout PR merge ref'"));
});

it('escapes injected model names and fails loudly when the signature literal is gone', () => {
const injectStep = step('Inject model name into triage signature');
const body = injectStep.match(/run: \|-\n([\s\S]*)$/)?.[1];
expect(body).toBeTruthy();
const script = body.replace(/^ {10}/gm, '');

const run = (model, content) => {
const dir = mkdtempSync(join(tmpdir(), 'triage-inject-'));
try {
const target = join(dir, '.qwen/skills/triage/references');
mkdirSync(target, { recursive: true });
writeFileSync(join(target, 'pr-workflow.md'), content);
const proc = spawnSync('bash', ['-c', script], {
cwd: dir,
env: { ...process.env, OPENAI_MODEL: model },
encoding: 'utf8',
});
return {
status: proc.status,
out: readFileSync(join(target, 'pr-workflow.md'), 'utf8'),
log: `${proc.stdout}${proc.stderr}`,
};
} finally {
rmSync(dir, { recursive: true, force: true });
}
};

// A model name carrying every sed replacement metacharacter (/ & \) must
// land verbatim — the old unescaped sed corrupted the skill text on these.
const meta = run('m1/pre&post\\x', 'sig: qwen3.7-max end');
expect(meta.status).toBe(0);
Comment thread
wenshao marked this conversation as resolved.
expect(meta.out).toBe('sig: m1/pre&post\\x end');

// The signature literal disappearing from the skill must fail the step —
// the old silent no-op shipped the wrong model name in every comment.
const missing = run('m2', 'sig: some-other-model end');
expect(missing.status).not.toBe(0);
expect(missing.log).toContain('Signature literal');
expect(missing.out).toBe('sig: some-other-model end');

// No model configured → file untouched, step succeeds.
const empty = run('', 'sig: qwen3.7-max end');
expect(empty.status).toBe(0);
expect(empty.out).toBe('sig: qwen3.7-max end');
});
});
Loading