justinmclean commented on PR #532:
URL: https://github.com/apache/airflow-steward/pull/532#issuecomment-4760123705

   Self review:
   
   ## Correctness
   No findings.
   
   Cross-checked the eval suite against the harness rather than trusting the 
fixtures at face value: all 9 expected.json parse, every step-config.json 
step_heading matches a real heading in SKILL.md (so the harness extracts the 
right step), and the grading wiring is right — step-2 declares its non-default 
prose fields in grading-schema.json (similarity_summary, closing_comment, 
cross_ref_comment, injection_warning), step-0's blockers is already in the 
harness DEFAULT_PROSE_FIELDS so it grades soft without a schema, and step-3's 
action lists are exact-matched, which is what you want for deterministic 
output. All three relay symlinks resolve.
   
   
   ## Security
   No findings.
   
   The skill makes every state change a confirmed proposal (Golden rules 1–2) 
and the adversarial eval (step-2 case-2-injection-in-body) correctly treats an 
injected HTML-comment directive as data: sets injection_warning, keeps 
proposed: true, and doesn't let it swap the kept/duplicate assignment. No 
credentials, injection, or untrusted-deserialization patterns introduced.
   Conventions
   
   [advisory] docs/mode-economics.md (not in diff) — new skill not added to the 
Triage cost table.
   
   AGENTS.md → "Keeping evals and mode-economics in sync" lists "a new skill" 
as a trigger; the table already carries security-issue-deduplicate.
   
   Rule: AGENTS.md#keeping-evals-and-mode-economics-in-sync. Note: the 
preceding sibling issue-stale-sweep (#509) also omitted it and the table is 
non-exhaustive, so this is precedent-consistent — optional.
   
   
   [advisory] skills/issue-deduplicate/SKILL.md:397 — reference link text 
doesn't match its target.
   
   - [docs/triage/spec.md](../../tools/spec-loop/specs/triage-mode.md)
   
   Rule: link label should name the real path; docs/triage/spec.md doesn't 
exist (target tools/spec-loop/specs/triage-mode.md does, so lychee passes but 
the label misleads).
   
   
   [advisory] skills/issue-deduplicate/SKILL.md:405 — link points at the 
directory, not the file.
   
   [<project-config>/issue-tracker-config.md](../../projects/_template/)
   
   Rule: link the file (.../projects/_template/issue-tracker-config.md).
   
   
   [advisory] commit 81cc0f5 "fix tests" — vague subject and no Generated-by: 
trailer.
   
   AGENTS.md commit conventions: keep the message focused on the user-visible 
change; agent-generated commits carry a Generated-by: trailer. Moot if you 
squash-merge (the lead commit has the trailer and no Co-Authored-By:, which is 
the rule that matters).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to