fix(skill_manage): batch success rows carry the linter findings and org-sharing note
Since operations[] became the only call shape (#97295) the batch's success path rebuilt each row as {name, action, file_path, success} and dropped every advisory key the per-op handler attached — lint_warnings/lint_hint from the create linter and the org_sharing note. The model never saw a finding. Carry those keys onto the row, mirroring what the failure path already does for teaching payloads.
This commit is contained in:
@@ -54,6 +54,17 @@ class TestSkillManageBatch(unittest.TestCase):
|
||||
for rel in ("SKILL.md", "references/a.md", "scripts/r.py"):
|
||||
self.assertTrue(os.path.exists(os.path.join(base, rel)), rel)
|
||||
|
||||
def test_advisory_findings_ride_on_the_op_result(self):
|
||||
# operations[] is the only call shape, so a finding the per-op handler attaches must
|
||||
# survive into the batch row or the linter is silent for every model call.
|
||||
from tools.skill_linter import _BODY_SOFT_BUDGET_CHARS
|
||||
self._call("probe", [{"action": "create", "content": SK.format(n="probe")}])
|
||||
r = self._call("probe", [{"action": "patch", "old_string": "Step 1.",
|
||||
"new_string": "- rule; why.\n" * (_BODY_SOFT_BUDGET_CHARS // 12 + 1)}])
|
||||
self.assertTrue(r["success"], r)
|
||||
rules = {w["rule"] for w in r["results"][0]["lint_warnings"]}
|
||||
self.assertIn("oversized-body", rules)
|
||||
|
||||
def test_midbatch_failure_rolls_back_existing_skill(self):
|
||||
self._call("probe", [{"action": "create", "content": SK.format(n="probe")}])
|
||||
r = self._call("probe", [
|
||||
|
||||
@@ -181,6 +181,9 @@ def _rollback(snapshots, find_skill):
|
||||
return ("; ".join(notes) if notes else "all touched skills rolled back"), bool(notes)
|
||||
|
||||
|
||||
_ADVISORY_KEYS = ("lint_warnings", "lint_hint", "org_sharing")
|
||||
|
||||
|
||||
def _skill_manage_batch(operations, default_name: str = None, task_id: str = None,
|
||||
session_id: str = None) -> str:
|
||||
"""Apply operations atomically: every touched skill is snapshotted first and any
|
||||
@@ -249,8 +252,12 @@ def _skill_manage_batch(operations, default_name: str = None, task_id: str = Non
|
||||
if k not in ("success", "error") and v is not None:
|
||||
fail.setdefault(k, v)
|
||||
return json.dumps(fail, ensure_ascii=False)
|
||||
results.append({"name": names[i], "action": op["action"],
|
||||
"file_path": op.get("file_path"), "success": True})
|
||||
entry = {"name": names[i], "action": op["action"],
|
||||
"file_path": op.get("file_path"), "success": True}
|
||||
# Advisory payloads (linter findings, org-sharing note) ride on the op result; the
|
||||
# compact success row otherwise hides them and the model never sees a finding.
|
||||
entry.update({k: parsed[k] for k in _ADVISORY_KEYS if parsed.get(k) is not None})
|
||||
results.append(entry)
|
||||
finally:
|
||||
_smt._skill_gate_bypass.reset(token)
|
||||
if rollback_failed:
|
||||
|
||||
Reference in New Issue
Block a user