feat(ack): add Feishu bug review approval gate
This commit is contained in:
@@ -236,6 +236,10 @@ tasks: []
|
||||
self.assertIn("--brand feishu", content)
|
||||
self.assertTrue("scope" in content)
|
||||
self.assertIn("不要把 `lark-cli auth check`", content)
|
||||
self.assertIn("审核通过前的唯一协作区", content)
|
||||
self.assertIn("不创建或刷新 `tasks.yaml` 中的 ACK 任务", content)
|
||||
self.assertIn("base:record:write", content)
|
||||
self.assertIn("fixLogic", content)
|
||||
self.assertNotIn("--lark-cli", content)
|
||||
self.assertNotIn("--executable", content)
|
||||
|
||||
@@ -248,8 +252,12 @@ tasks: []
|
||||
self.assertEqual(empty_plan["actions"], [{
|
||||
"sourceRef": first,
|
||||
"recordId": "recBug1",
|
||||
"draftRevision": empty_plan["records"][0]["draftRevision"],
|
||||
"action": "create",
|
||||
}])
|
||||
self.assertRegex(
|
||||
empty_plan["actions"][0]["draftRevision"], r"^sha256:[0-9a-f]{64}$",
|
||||
)
|
||||
|
||||
base = self.board.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
@@ -53,9 +53,13 @@ class FeishuBugIntakeUnitTests(unittest.TestCase):
|
||||
" if '--format' in args: raise SystemExit(8)\n"
|
||||
" print(json.dumps([{'name':'tenant-a','appId':'cli_a','brand':'feishu','active':True},{'name':'tenant-b','appId':'cli_b','brand':'feishu','active':False}]))\n"
|
||||
"elif args[:2] == ['base', '+record-list']:\n"
|
||||
" print(json.dumps({'code': 0, 'data': {'fields': ['fldTitle','fldActual','fldExpected','fldSteps','fldAcceptance','fldPriority','fldAttachments','fldUpdated'], 'record_id_list': ['recA'], 'data': [[' Bug\\n title ', ' actual ', 'expected', ['one', 'two'], 'accept', 'P1', [{'file_token':'fileA','name':'shot.png','type':'image/png','size':3}], '2026-08-01']]}}))\n"
|
||||
" fields=['fldTitle','fldActual','fldExpected','fldSteps','fldAcceptance','fldPriority','fldAttachments','fldUpdated']; row=[' Bug\\n title ',' actual ','expected',['one','two'],'1. save succeeds\\n2. the crash no longer occurs','P1',[{'file_token':'fileA','name':'shot.png','type':'image/png','size':3}],'2026-08-01']\n"
|
||||
" if 'fldFixLogic' in args: fields.append('fldFixLogic'); row.append('change parser without widening input')\n"
|
||||
" print(json.dumps({'code': 0, 'data': {'fields': fields, 'record_id_list': ['recA'], 'data': [row]}}))\n"
|
||||
"elif args[:2] == ['base', '+record-download-attachment']:\n"
|
||||
" out = pathlib.Path(args[args.index('--output') + 1]); out.mkdir(parents=True, exist_ok=True); (out / 'shot.png').write_bytes(b'png'); print(json.dumps({'ok': True, 'data': {}}))\n"
|
||||
"elif args[:2] == ['base', '+record-upsert']:\n"
|
||||
" print(json.dumps({'code': 0, 'data': {}}))\n"
|
||||
"else: raise SystemExit(2)\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
@@ -210,6 +214,166 @@ class FeishuBugIntakeUnitTests(unittest.TestCase):
|
||||
}
|
||||
self.assertEqual(feishu_bug_intake.matrix_from_response(response, fields), (["recA"], [["Bug"]]))
|
||||
|
||||
def test_optional_fix_logic_field_is_normalized_for_preapproval_review(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
board_path = Path(temp_dir) / "tasks.yaml"
|
||||
board_path.write_text(
|
||||
BOARD.replace(
|
||||
" profile: tenant-b\n",
|
||||
" workflow: reviewed-writeback-v1\n profile: tenant-b\n",
|
||||
).replace(
|
||||
" acceptance: fldAcceptance\n",
|
||||
" acceptance: fldAcceptance\n fixLogic: fldFixLogic\n",
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
config = feishu_bug_intake.config_from_board(
|
||||
feishu_bug_intake.load_board(board_path)
|
||||
)
|
||||
row = [
|
||||
"Bug", "actual", "expected", "steps", "acceptance", "P1", [],
|
||||
"2026-08-01T12:00:00Z", "change parser without widening input",
|
||||
]
|
||||
with mock.patch.object(feishu_bug_intake, "profile_check"), mock.patch.object(
|
||||
feishu_bug_intake, "fetch_pages", return_value=[("recA", row)]
|
||||
):
|
||||
payload = feishu_bug_intake.fetch(config, None)
|
||||
|
||||
record = payload["records"][0]
|
||||
self.assertEqual(record["fixLogic"], "change parser without widening input")
|
||||
self.assertNotIn("fixLogic", record["enrichmentRequired"])
|
||||
|
||||
def test_write_draft_uses_trusted_cli_and_only_configured_review_fields(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp = Path(temp_dir)
|
||||
board_path = temp / "tasks.yaml"
|
||||
board_path.write_text(
|
||||
BOARD.replace(
|
||||
" profile: tenant-b\n",
|
||||
" workflow: reviewed-writeback-v1\n profile: tenant-b\n",
|
||||
).replace(
|
||||
" acceptance: fldAcceptance\n",
|
||||
" acceptance: fldAcceptance\n fixLogic: fldFixLogic\n",
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
draft_path = temp / "draft.json"
|
||||
draft_path.write_text(json.dumps({
|
||||
"fixLogic": "change parser without widening input",
|
||||
"acceptance": ["save succeeds", "the crash no longer occurs"],
|
||||
}), encoding="utf-8")
|
||||
fake, log_path = self.make_fake_cli(temp)
|
||||
|
||||
fetched_code, fetched_output, fetched_error = self.invoke([
|
||||
"fetch", str(board_path),
|
||||
], fake)
|
||||
self.assertEqual(fetched_code, 0, fetched_error)
|
||||
fetched_record = json.loads(fetched_output)["records"][0]
|
||||
|
||||
code, output, error = self.invoke([
|
||||
"write-draft", str(board_path), "--record-id", "recA",
|
||||
"--expected-source-ref", fetched_record["sourceRef"],
|
||||
"--expected-draft-revision", fetched_record["draftRevision"],
|
||||
"--input", str(draft_path),
|
||||
], fake)
|
||||
|
||||
self.assertEqual(code, 0, error)
|
||||
self.assertEqual(json.loads(output)["written"], ["fixLogic", "acceptance"])
|
||||
self.assertRegex(json.loads(output)["draftRevision"], r"^sha256:[0-9a-f]{64}$")
|
||||
calls = [json.loads(line) for line in log_path.read_text(encoding="utf-8").splitlines()]
|
||||
write_call = next(call for call in calls if call[:2] == ["base", "+record-upsert"])
|
||||
self.assertEqual(write_call[write_call.index("--profile") + 1], "tenant-b")
|
||||
patch = json.loads(write_call[write_call.index("--json") + 1])
|
||||
self.assertEqual(set(patch), {"fldFixLogic", "fldAcceptance"})
|
||||
self.assertIn("1. save succeeds", patch["fldAcceptance"])
|
||||
|
||||
imported_code, imported_output, imported_error = self.invoke([
|
||||
"import-approved", str(board_path), "--record-id", "recA",
|
||||
"--expected-source-ref", fetched_record["sourceRef"],
|
||||
"--expected-draft-revision", fetched_record["draftRevision"],
|
||||
], fake)
|
||||
self.assertEqual(imported_code, 0, imported_error)
|
||||
task_draft = json.loads(imported_output)["taskDraft"]
|
||||
self.assertEqual(task_draft["source"]["workflow"], "reviewed-writeback-v1")
|
||||
self.assertEqual(
|
||||
task_draft["acceptanceCriteria"],
|
||||
["save succeeds", "the crash no longer occurs"],
|
||||
)
|
||||
self.assertRegex(
|
||||
task_draft["source"]["approvedPayloadHash"], r"^sha256:[0-9a-f]{64}$",
|
||||
)
|
||||
|
||||
def test_write_draft_rejects_stale_revision_before_mutation(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp = Path(temp_dir)
|
||||
board_path = temp / "tasks.yaml"
|
||||
board_path.write_text(
|
||||
BOARD.replace(
|
||||
" profile: tenant-b\n",
|
||||
" workflow: reviewed-writeback-v1\n profile: tenant-b\n",
|
||||
).replace(
|
||||
" acceptance: fldAcceptance\n",
|
||||
" acceptance: fldAcceptance\n fixLogic: fldFixLogic\n",
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
draft_path = temp / "draft.json"
|
||||
draft_path.write_text(json.dumps({
|
||||
"fixLogic": "fix",
|
||||
"acceptance": ["pass"],
|
||||
}), encoding="utf-8")
|
||||
fake, log_path = self.make_fake_cli(temp)
|
||||
fetched = json.loads(self.invoke(["fetch", str(board_path)], fake)[1])["records"][0]
|
||||
|
||||
code, _, _ = self.invoke([
|
||||
"write-draft", str(board_path), "--record-id", "recA",
|
||||
"--expected-source-ref", fetched["sourceRef"],
|
||||
"--expected-draft-revision", "sha256:" + "0" * 64,
|
||||
"--input", str(draft_path),
|
||||
], fake)
|
||||
|
||||
self.assertNotEqual(code, 0)
|
||||
calls = [json.loads(line) for line in log_path.read_text(encoding="utf-8").splitlines()]
|
||||
self.assertFalse(any(call[:2] == ["base", "+record-upsert"] for call in calls))
|
||||
|
||||
def test_write_draft_rejects_symlink_and_unknown_fields(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp = Path(temp_dir)
|
||||
target = temp / "draft.json"
|
||||
target.write_text(json.dumps({
|
||||
"fixLogic": "fix",
|
||||
"acceptance": ["pass"],
|
||||
"unexpected": "do not write",
|
||||
}), encoding="utf-8")
|
||||
with self.assertRaises(feishu_bug_intake.IntakeError):
|
||||
feishu_bug_intake.load_draft(target)
|
||||
|
||||
target.write_text(json.dumps({
|
||||
"fixLogic": "fix",
|
||||
"acceptance": ["pass"],
|
||||
}), encoding="utf-8")
|
||||
linked = temp / "linked.json"
|
||||
linked.symlink_to(target)
|
||||
with self.assertRaises(feishu_bug_intake.IntakeError):
|
||||
feishu_bug_intake.load_draft(linked)
|
||||
|
||||
def test_draft_revision_binds_attachment_identity(self) -> None:
|
||||
record = {
|
||||
"sourceRef": "feishu-base:sha256:" + "a" * 64,
|
||||
"updatedAt": "2026-08-03T12:00:00+08:00",
|
||||
"title": "Bug",
|
||||
"actual": "crash",
|
||||
"expected": "save",
|
||||
"steps": "open then save",
|
||||
"fixLogic": "fix parser",
|
||||
"acceptance": "save succeeds",
|
||||
"priority": "P1",
|
||||
"attachments": [{"name": "same.png", "type": "image/png", "size": 4}],
|
||||
}
|
||||
first = feishu_bug_intake.draft_revision(record, ["fileTokenA"])
|
||||
second = feishu_bug_intake.draft_revision(record, ["fileTokenB"])
|
||||
self.assertNotEqual(first, second)
|
||||
|
||||
def test_attachments_and_required_text_are_resource_bounded_and_strict(self) -> None:
|
||||
with self.assertRaises(feishu_bug_intake.IntakeError):
|
||||
feishu_bug_intake.attachment_items([{
|
||||
|
||||
@@ -67,7 +67,7 @@ class AckSkillContentTests(unittest.TestCase):
|
||||
):
|
||||
self.assertTrue((ack_dir / relative_path).is_file(), relative_path)
|
||||
version = (ack_dir / "VERSION").read_text(encoding="utf-8").strip()
|
||||
self.assertEqual(version, "0.13.1")
|
||||
self.assertEqual(version, "0.14.0")
|
||||
self.assertIn(
|
||||
f'ackVersion: "{version}"',
|
||||
(ack_dir / "examples" / "tasks.example.yaml").read_text(encoding="utf-8"),
|
||||
|
||||
@@ -16,6 +16,8 @@ REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
VALIDATOR = REPO_ROOT / "skills" / "ack" / "scripts" / "validate_tasks.py"
|
||||
EXAMPLE = REPO_ROOT / "skills" / "ack" / "examples" / "tasks.example.yaml"
|
||||
SCHEMA = REPO_ROOT / "skills" / "ack" / "templates" / "tasks.schema.json"
|
||||
sys.path.insert(0, str(VALIDATOR.parent))
|
||||
from approval_payload import approval_payload_hash # noqa: E402
|
||||
|
||||
|
||||
def valid_knowledge_board() -> dict:
|
||||
@@ -148,6 +150,93 @@ class AckTaskValidationTests(unittest.TestCase):
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertIn("任务板校验通过", result.stdout)
|
||||
|
||||
def test_approved_feishu_plan_fields_are_validated(self) -> None:
|
||||
board = valid_knowledge_board()
|
||||
board["tasks"][0]["fixLogic"] = "change the parser and preserve legacy input"
|
||||
board["tasks"][0]["acceptanceCriteria"] = [
|
||||
"the submitted value is saved",
|
||||
"the original crash no longer occurs",
|
||||
]
|
||||
self.assert_board_accepted_in_all_modes(board)
|
||||
|
||||
invalid = copy.deepcopy(board)
|
||||
invalid["tasks"][0]["acceptanceCriteria"] = ["valid", 7]
|
||||
self.assert_board_rejected_in_all_modes(invalid, "列表项必须是字符串")
|
||||
|
||||
def test_reviewed_feishu_workflow_requires_fix_logic_and_approval_revision(self) -> None:
|
||||
fields = {
|
||||
"title": "fTitle",
|
||||
"actual": "fActual",
|
||||
"expected": "fExpected",
|
||||
"stepsToReproduce": "fSteps",
|
||||
"acceptance": "fAcceptance",
|
||||
"priority": "fPriority",
|
||||
"attachments": "fAttachments",
|
||||
"updatedAt": "fUpdated",
|
||||
}
|
||||
board = valid_knowledge_board()
|
||||
board["project"]["bugIntake"] = {
|
||||
"provider": "feishu-base",
|
||||
"workflow": "reviewed-writeback-v1",
|
||||
"profile": "tenant-b",
|
||||
"baseToken": "baseToken",
|
||||
"tableId": "tblBugs",
|
||||
"viewId": "vewReview",
|
||||
"fields": fields,
|
||||
}
|
||||
self.assert_board_rejected_in_all_modes(
|
||||
board, "reviewed-writeback-v1 必须映射",
|
||||
)
|
||||
|
||||
board["project"]["bugIntake"]["fields"]["fixLogic"] = "fFixLogic"
|
||||
board["tasks"][0].update({
|
||||
"priority": "P1",
|
||||
"actual": "save crashes",
|
||||
"expected": "save succeeds",
|
||||
"stepsToReproduce": ["open", "save"],
|
||||
"fixLogic": "preserve input while fixing the parser",
|
||||
"acceptanceCriteria": ["save succeeds", "the crash no longer occurs"],
|
||||
})
|
||||
board["tasks"][0]["source"] = {
|
||||
"kind": "feishu-base",
|
||||
"workflow": "reviewed-writeback-v1",
|
||||
"ref": "feishu-base:sha256:" + "a" * 64,
|
||||
"recordId": "recA",
|
||||
"updatedAt": "2026-08-03T12:00:00+08:00",
|
||||
"approvedRevision": "sha256:" + "b" * 64,
|
||||
}
|
||||
board["tasks"][0]["source"]["approvedPayloadHash"] = approval_payload_hash(
|
||||
board["tasks"][0]
|
||||
)
|
||||
self.assert_board_accepted_in_all_modes(board)
|
||||
|
||||
board["tasks"][0]["source"]["approvedRevision"] = "latest"
|
||||
self.assert_board_rejected_in_all_modes(board, "必须是 sha256 revision")
|
||||
|
||||
board["tasks"][0]["source"]["approvedRevision"] = "sha256:" + "b" * 64
|
||||
board["tasks"][0]["fixLogic"] = "silently changed after approval"
|
||||
self.assert_board_rejected_in_all_modes(board, "与任务审核字段不匹配")
|
||||
|
||||
board["tasks"][0]["fixLogic"] = "preserve input while fixing the parser"
|
||||
board["tasks"][0]["source"]["approvedPayloadHash"] = approval_payload_hash(
|
||||
board["tasks"][0]
|
||||
)
|
||||
board["tasks"][0]["description"] = "modify unrelated modules"
|
||||
self.assert_board_rejected_in_all_modes(board, "与任务审核字段不匹配")
|
||||
|
||||
legacy = copy.deepcopy(board)
|
||||
legacy["tasks"][0]["description"] = "validate knowledge fields"
|
||||
legacy["tasks"][0]["source"] = {
|
||||
"kind": "feishu-base",
|
||||
"ref": "feishu-base:sha256:" + "c" * 64,
|
||||
"recordId": "recLegacy",
|
||||
"updatedAt": "2026-08-01T12:00:00+08:00",
|
||||
}
|
||||
self.assert_board_rejected_in_all_modes(legacy, "必须先迁移审核")
|
||||
|
||||
legacy["tasks"][0]["status"] = "verified"
|
||||
self.assert_board_accepted_in_all_modes(legacy)
|
||||
|
||||
def test_legacy_sources_remain_open_while_feishu_sources_are_strict(self) -> None:
|
||||
for legacy_source in (
|
||||
"manual",
|
||||
|
||||
Reference in New Issue
Block a user