vrr/qwen-code
2
0
Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2026-05-14 08:14:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 8

fix(attribution): submodule leak, PR body nesting, shallow-clone bail, schema default

Browse source 
- attachCommitAttribution: when HEAD didn't move in our cwd, leave
  pending attributions alone instead of dropping them. The case can be
  a failed commit, `git reset HEAD~1`, OR `cd submodule && git commit`
  (inner repo's HEAD moves, ours doesn't). Dropping was overly
  aggressive and silently lost outer-repo edits in the submodule case.
- addAttributionToPR: mirror addCoAuthorToGitCommit's nested-match
  rejection so `gh pr create --body "docs mention -b 'flag'"` picks the
  outer `--body`, not the inner literal `-b`. Splicing into the inner
  match would corrupt the body. Regression test added.
- getCommittedFileInfo: when `rev-parse --verify HEAD~1` fails, also
  check `rev-list --count HEAD === 1` to confirm HEAD is the true
  root commit. In a shallow clone, HEAD~1 is unreadable but the commit
  has a parent recorded — falling back to `diff-tree --root` would
  diff against the empty tree and over-attribute the entire commit.
  Bail with a debug warning instead.
- generate-settings-schema: lift `default` (and `description`) out of
  the inner `anyOf[N]` schema to the outer level when wrapping with
  `legacyTypes`. Most JSON-schema-driven editors only surface
  top-level defaults; burying the default under `anyOf` lost the
  "enabled by default" hint. Also extend the default filter to
  publish non-empty plain objects (so `gitCoAuthor`'s default can
  appear). gitCoAuthor's source default updated to the runtime shape
  `{commit: true, pr: true}` to match `normalizeGitCoAuthor`.
This commit is contained in:
wenshao 2026-05-05 14:16:19 +08:00
parent 0103af5296
commit 9e731698ae
5 changed files with 128 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

6
packages/cli/src/config/settingsSchema.ts
View file

@ -372,7 +372,11 @@ const SETTINGS_SCHEMA = {
label: 'Attribution',
category: 'General',
requiresRestart: false,
default: {},
// Match `normalizeGitCoAuthor`'s runtime defaults so the IDE
// schema publishes the same "enabled by default" hint users see
// at runtime. The empty-object form here would silently lose
// editor-surfaced defaults.
default: { commit: true, pr: true },
description:
'Attribution added to git commits and pull requests created through Qwen Code.',
showInDialog: false,

33
packages/core/src/tools/shell.test.ts
View file

@ -2452,6 +2452,39 @@ describe('ShellTool', () => {
);
});
// A `-b 'flag'` mention literally inside the outer `--body "..."`
// text must NOT be picked as the body argument: the trailer
// would land mid-body, corrupting the user-approved command.
// Mirrors addCoAuthorToGitCommit's nested-match check.
it('should pick the OUTER --body when an inner -b appears in body text', async () => {
const command =
'gh pr create --title "x" --body "docs mention -b \'flag\' here"';
const invocation = shellTool.build({ command, is_background: false });
const promise = invocation.execute(mockAbortSignal);
resolveExecutionPromise({
rawOutput: Buffer.from(''),
output: '',
exitCode: 0,
signal: null,
error: null,
aborted: false,
pid: 12345,
executionMethod: 'child_process',
});
await promise;
const calls = mockShellExecutionService.mock.calls;
const cmd = calls[calls.length - 1]?.[0] as string;
// The trailer must appear AFTER the closing `"` of the outer
// body, not between `flag` and `here`.
expect(cmd).toMatch(
/--body "docs mention -b 'flag' here[\s\S]*Generated with Qwen Code"/,
);
expect(cmd).not.toMatch(
/-b 'flag[\s\S]*Generated with Qwen Code[\s\S]*' here"/,
);
});
it('should append attribution to gh pr create --body when pr enabled', async () => {
const command = 'gh pr create --title "x" --body "Summary"';
const invocation = shellTool.build({ command, is_background: false });

90
packages/core/src/tools/shell.ts
View file

@ -1802,14 +1802,17 @@ export class ShellToolInvocation extends BaseToolInvocation<
const attributionService = CommitAttributionService.getInstance();
if (!commitCreated) {
// No new commit landed (nothing staged, hook rejected, or user
// reset right after). Drop the per-file attributions but keep the
// "since last commit" prompt counters intact — the user's next
// attempt should still credit prompts that happened during this
// failed try.
if (attributionService.hasAttributions()) {
attributionService.clearAttributions(false);
}
// HEAD didn't move in this cwd. Possible causes:
// 1. Commit failed (hook rejected, nothing staged, etc.)
// 2. User did `git commit && git reset HEAD~1` — HEAD reverted
// 3. Submodule case (`cd submodule && git commit`) — the inner
// repo's HEAD moved, ours didn't
// We can't tell these apart reliably from here. Dropping the
// per-file attributions on (1)/(2) is fine in isolation, but on
// (3) we'd silently lose the user's outer-repo edits even though
// none of them were committed. Leave attributions intact instead:
// a later successful commit will overwrite the counters and the
// accumulated aiContribution still represents real AI work.
return;
}
@ -2015,17 +2018,38 @@ export class ShellToolInvocation extends BaseToolInvocation<
};
try {
// The two `rev-parse` calls are independent — fan out so we
// don't pay the spawn latency twice serially. Same for the
// three diff calls below once we know which form to use.
const [hasParentOutput, repoRootOutput] = await Promise.all([
runGit('rev-parse --verify HEAD~1'),
runGit('rev-parse --show-toplevel'),
]);
// hasParent fails for shallow clones where the parent was
// pruned, which is fine — diff-tree --root is a safe fallback
// that diffs against the empty tree.
// The four `rev-parse`-shaped calls are independent — fan out so
// we don't pay the spawn latency serially. Same for the three
// diff calls below once we know which form to use.
// - `rev-parse --verify HEAD~1`: probe whether the parent OBJECT
// is locally available (fails in shallow clones where the
// parent was pruned).
// - `rev-list --count HEAD`: total commits reachable. `=== 1`
// means HEAD truly is the root commit (no parent SHA recorded
// on HEAD), where `diff-tree --root` is correct. `> 1` means
// HEAD has a parent recorded — if --verify also failed, this
// is a shallow clone and we must NOT fall back to --root
// (which would diff against the empty tree and over-attribute).
const [hasParentOutput, commitCountOutput, repoRootOutput] =
await Promise.all([
runGit('rev-parse --verify HEAD~1'),
runGit('rev-list --count HEAD'),
runGit('rev-parse --show-toplevel'),
]);
const hasParent = hasParentOutput.length > 0;
const totalCommits = parseInt(commitCountOutput.trim(), 10);
const isTrueRootCommit =
Number.isFinite(totalCommits) && totalCommits === 1;
// Shallow clone: HEAD has a parent but the object isn't local.
// Bail rather than over-attribute via --root.
if (!hasParent && !isTrueRootCommit) {
debugLogger.warn(
'getCommittedFileInfo: HEAD~1 unreadable but commit is not the ' +
'true root (shallow clone?); skipping attribution to avoid ' +
'attributing the entire commit contents.',
);
return empty;
}
// Capture the repo root so the attribution service can
// reconcile paths from `git diff` (relative to the toplevel)
// against absolute paths recorded by the edit/write tools.
@ -2365,12 +2389,34 @@ export class ShellToolInvocation extends BaseToolInvocation<
};
const bodyDoubleMatch = lastMatch(segment.matchAll(bodyDoublePattern));
const bodySingleMatch = lastMatch(segment.matchAll(bodySinglePattern));
// Pick whichever match appears LAST in the segment, regardless of
// quote style — but reject any candidate that's nested inside the
// other's range. For `gh pr create --body "docs mention -b 'flag'"`
// the inner `-b 'flag'` is INSIDE the outer `--body "..."`; without
// a nesting check the inner (later) `-b` would win and the trailer
// would be spliced into the body text rather than appended after it.
const bodyMatchRange = (m: RegExpMatchArray | null) =>
m ? { start: m.index ?? 0, end: (m.index ?? 0) + m[0].length } : null;
const bodyIsInside = (
inner: RegExpMatchArray | null,
outer: RegExpMatchArray | null,
): boolean => {
const i = bodyMatchRange(inner);
const o = bodyMatchRange(outer);
return !!(i && o && i.start >= o.start && i.end <= o.end);
};
let bodyMatch: RegExpMatchArray | null;
if (bodyDoubleMatch && bodySingleMatch) {
bodyMatch =
(bodyDoubleMatch.index ?? 0) > (bodySingleMatch.index ?? 0)
? bodyDoubleMatch
: bodySingleMatch;
if (bodyIsInside(bodySingleMatch, bodyDoubleMatch)) {
bodyMatch = bodyDoubleMatch;
} else if (bodyIsInside(bodyDoubleMatch, bodySingleMatch)) {
bodyMatch = bodySingleMatch;
} else {
bodyMatch =
(bodyDoubleMatch.index ?? 0) > (bodySingleMatch.index ?? 0)
? bodyDoubleMatch
: bodySingleMatch;
}
} else {
bodyMatch = bodyDoubleMatch ?? bodySingleMatch;
}

4
packages/vscode-ide-companion/schemas/settings.schema.json
View file

@ -67,6 +67,10 @@
},
"gitCoAuthor": {
"description": "Attribution added to git commits and pull requests created through Qwen Code.",
"default": {
"commit": true,
"pr": true
},
"anyOf": [
{
"type": "boolean"

19
scripts/generate-settings-schema.ts
View file

@ -148,7 +148,7 @@ function convertSettingToJsonSchema(
break;
}
// Add default value for simple types only
// Add default value for simple and object types
if (setting.default !== undefined && setting.default !== null) {
const defaultVal = setting.default;
if (
@ -159,6 +159,14 @@ function convertSettingToJsonSchema(
schema.default = defaultVal;
} else if (Array.isArray(defaultVal) && defaultVal.length > 0) {
schema.default = defaultVal;
} else if (
typeof defaultVal === 'object' &&
!Array.isArray(defaultVal) &&
Object.keys(defaultVal).length > 0
) {
// Non-empty plain object — publish so IDE editors can surface the
// default value (e.g. `{commit: true, pr: true}` for gitCoAuthor).
schema.default = defaultVal;
}
}
@ -166,11 +174,20 @@ function convertSettingToJsonSchema(
// later expanded into an object), wrap with `anyOf` so existing values
// in users' settings.json don't trip the IDE schema validator while
// they wait for our migration to rewrite them on the next launch.
//
// Lift `description` and `default` to the outer (anyOf) level so IDE
// editors that surface schema-driven defaults / descriptions still see
// them — burying these behind `anyOf[N]` makes most validators ignore
// the `default`, which loses the "enabled by default" hint for any
// setting using `legacyTypes`.
if (setting.legacyTypes && setting.legacyTypes.length > 0) {
const description = schema.description;
const defaultVal = schema.default;
delete schema.description;
delete schema.default;
return {
...(description ? { description } : {}),
...(defaultVal !== undefined ? { default: defaultVal } : {}),
anyOf: [...setting.legacyTypes.map((t) => ({ type: t })), schema],
};
}