Skip to content

Phase 1 (PR #2) review deferrals — 1 P1 + 8 P2 #4

Description

@Artic0din

Tracking the Codex findings deferred from #2 (merged as 86f5fe0). Fork-only — none of these are upstream-bound.

Disposition recorded at merge: P1 #1 (fork-ci validate job missing sync/* guard) was fixed in 3e08a42. The items below were consciously deferred.

P1 — address before the sync lane is first exercised

  • fork-sync-advisory.yml:15 — upstream validate.yml not guarded for sync/*
    Upstream-owned validate.yml fires HACS/hassfest on every PR to main with no sync/* guard, so sync/* PRs get red checks from it regardless of the advisory workflow's continue-on-error. Direct fix = editing upstream-owned validate.yml (out of scope for the fork-only Phase 1 PR). Mitigated in practice: auto-merge is OFF and sync policy is already "fail = investigate, typically still merge," so it's advisory-not-blocking. Decide the approach (guard upstream file vs. branch-protection exclusion vs. accept) when the first sync/upstream-* PR lands. (thread)

P2 — fix eventually, non-blocking

  • fork-claude.yml:19 — issue_comment head_ref gate. On issue_comment events the PR marker is under github.event.issue.pull_request, not github.event.pull_request; the sync-branch exclusion evaluates against an empty value, so @claude comments could run the write-permission job on sync/* PRs. Resolve the PR head ref before allowing the job. (thread)
  • .github/dependabot.yml:4 — pip ecosystem, no manifest. No root requirements.txt / pyproject.toml / Pipfile / setup.py, so the pip update entry has nothing to update and Dependabot will report a config error. Add a real manifest or drop the pip block until one exists. (thread)
  • SECURITY.md:7 — public-disclosure routing. Tells reporters to open a normal (public) issue; a report with a token/auth-bypass/PII detail would disclose before a fix. Route through GitHub private vulnerability reporting or a security email. (thread)
  • .claude/commands/sync-upstream.md:20 — sync branch base. git checkout -b sync/upstream-... from the current branch can fold unrelated fork work into the sync PR. Check out and fast-forward main before creating the sync branch. (thread)
  • .claude/commands/fix-review.md:15 — autosquash prefix. /ship uses git rebase -i --autosquash, which only folds squash!/fixup! commits; /fix-review creates fix({scope})... commits that won't be folded. Use fixup-style commits. (thread)
  • .claude/commands/self-review.md:8 — ruff --fix mutates tree. Step is labeled read-only ("do not auto-fix") but ruff check --fix rewrites files. Use ruff check . or --diff. (thread)
  • .claude/commands/sync-upstream.md:9 — upstream remote check. Greps all of git remote -v for bolagnaise; a clone whose origin already points at upstream skips adding the upstream remote, then fails at git fetch upstream main. Use git remote get-url upstream instead. (thread)
  • CLAUDE.md:2 — missing ENGINEERING_CONSTITUTION.md. @ENGINEERING_CONSTITUTION.md is imported and referenced but the file doesn't exist; governance guidance is unresolvable in sessions using this entry point. Add the file or remove the reference. (thread)

Source: Codex review on #2.


Migration source: PLA-75
Linear status at cutover: Backlog
Cut over to GitHub on 2026-07-22.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions