refactor(workflows): inline scripts, eliminate checkout dependency

[lml2468]

Problem

Three reusable notify workflows used a two-step pattern:

1. actions/checkout to fetch scripts/*.py from this repo (required contents: read)
2. run: python3 _shared/scripts/xxx.py

This forced callee permissions: contents: read, which in turn required every caller across all sub-repos to also declare contents: read — because GitHub Actions intersects caller × callee permissions. A caller with permissions: {} would silently 403 on checkout.

Solution

Inline the Python script content directly into the run: step as a heredoc. No checkout needed, no external dependency.

- name: Notify Octo IM
  env: ...
  run: |
    python3 - << 'PYEOF'
    # ... script content inline ...
    PYEOF

Result

Workflow	Before	After
octo-issue-feed.yml	permissions: contents: read	permissions: {}
octo-pr-feed.yml	permissions: contents: read	permissions: {}
octo-ci-status.yml	permissions: actions: read + contents: read	permissions: actions: read

Callers across all 8 sub-repos need zero changes. Their existing permissions: {} is now fully compatible.

After Merge

Tag v1.1 (or v2 if preferred). Sub-repo callers referencing @v1 will need a one-line bump to @v1.1 — or keep @v1 and accept the known permissions issue until they update. Recommend tagging v2 to make the upgrade intentional.

[yujiawei]

Code Review — PR #15 (.github)

Summary

Clean, well-scoped refactor that correctly addresses the reusable-workflow permissions intersection problem. The three inlined Python heredocs are byte-for-byte identical to the corresponding scripts/*.py at this SHA (verified by extracting the heredoc bodies and diff-ing against scripts/octo_ci_status_notify.py, scripts/octo_issue_feed_notify.py, scripts/octo_pr_feed_notify.py). All three compile under python3 -m py_compile. No behavior change beyond the (intended) elimination of the actions/checkout step.

Verdict: COMMENTED — no blockers; two non-blocking cleanup suggestions and one release-process note below.

1. Verification

Check	Result
Inlined script == scripts/octo_ci_status_notify.py at HEAD	✅ identical (178 lines)
Inlined script == scripts/octo_issue_feed_notify.py at HEAD	✅ identical (79 lines)
Inlined script == scripts/octo_pr_feed_notify.py at HEAD	✅ identical (84 lines)
Heredoc quoting prevents shell expansion	✅ << 'PYEOF' (single-quoted) on all 3 files (octo-ci-status.yml:56, octo-issue-feed.yml:55, octo-pr-feed.yml:67)
YAML literal-block indentation consistent	✅ uniform 10-space indent on all heredoc body lines, PYEOF terminator at column 0 after YAML strip
env: block in each step passes every variable the script reads	✅ verified per workflow (CONCLUSION, REPO_NAME, etc.)
All Checkout reusable-workflow scripts steps removed	✅ no remaining scripts/ references in the three workflow YAML files

2. Permissions Analysis

Workflow	Final permissions	API surface used by inlined script	Correct?
octo-ci-status.yml	actions: read	GET /repos/{org}/{repo}/actions/runs/{id} and …/actions/workflows/{id}/runs (lines 71-73, 89-91 of inlined script)	✅ actions: read is the exact scope these endpoints require
octo-issue-feed.yml	permissions: {}	None (only POSTs to im.deepminer.com.cn)	✅
octo-pr-feed.yml	permissions: {}	None (only POSTs to im.deepminer.com.cn)	✅

The reusable-workflow rule the PR body cites — the GITHUB_TOKEN issued to a called workflow is bounded by the caller's permissions: declaration — is correctly applied. After this lands, octo-issue-feed.yml and octo-pr-feed.yml are usable from a caller with permissions: {} (no API surface left to gate). octo-ci-status.yml still needs actions: read from the caller, but that is intrinsic to what the script does (read run history) and unchanged from before this PR.

A minor accuracy note on the PR description: the table says callers' "existing permissions: {} is now fully compatible." Spot-checking actual callers (Mininglamp-OSS/octo-server/.github/workflows/octo-issue-feed.yml, Mininglamp-OSS/octo-admin/.github/workflows/octo-ci-status.yml, etc.) shows they don't declare a top-level permissions: at all — they inherit repo default permissions. That's still compatible (and arguably better, since the new callees clamp the token down regardless), but the description slightly mis-states the pre-state.

3. Findings

P2 — scripts/ directory is now unreferenced but still on disk

After this PR, no workflow refers to scripts/octo_*.py, but the three files remain in the repo at the same SHA:

scripts/octo_ci_status_notify.py
scripts/octo_issue_feed_notify.py
scripts/octo_pr_feed_notify.py

Two future readers will be confused: one will edit scripts/octo_ci_status_notify.py thinking it's the source of truth, and one will edit the inlined heredoc and forget to keep scripts/*.py in sync (or vice-versa). Recommend either:

• (preferred) delete scripts/ in this PR or as an immediate follow-up; or
• add a short scripts/README.md explaining "kept for reference; the live source lives inline in .github/workflows/octo-*.yml".

This is intentionally non-blocking — the PR title is a refactor, not a cleanup, and shipping it now is fine.

P2 — Editing inlined Python in YAML is more painful than editing scripts/*.py

You lose Python syntax highlighting/formatter on the inline body (it's a YAML string), and mypy / ruff / pyflakes won't run against it without an extraction step. For 84-184 lines × 3 files this is acceptable, but if either script grows, consider switching to a pure-actions/github-script step (no Python, no checkout) or accepting the checkout cost for that one workflow. Not a request for change here — just a forward note.

Note — v1 tag retag vs. cutting v2

v1 is the only tag today, and all sub-repo callers (verified in Mininglamp-OSS/octo-server, octo-admin, octo-web, octo-deployment, octo-adapters, octo-smart-summary, octo-matter, octo-lib) pin to @v1. Moving v1 to this commit would silently push the new behavior to every caller — fine because behavior is unchanged, but it sets a precedent that v1 is mutable. The PR description's recommendation to cut v2 (callers opt in by explicit bump) is the right call. If you do retag v1, please document it in the release notes so future drift is traceable. (No code change needed — this is a release-process note.)

4. Additional Findings (Outside Task Scope)

• Send-failure behavior unchanged but worth re-confirming intent: each workflow exits with sys.exit(1) if any group push fails (after retries). That means a transient outage on one of the two channels marks the whole reusable-workflow run as failed. Reasonable, and it matches the prior behavior — flagging it because someone reading the inlined code in YAML for the first time might miss that the failed list tail is the failure gate.
• Hardcoded org Mininglamp-OSS in octo-ci-status.yml API URLs (e.g. f'https://api.github.com/repos/Mininglamp-OSS/{repo}/...'): pre-existing, not introduced by this PR. If the workflow is ever published for use outside the org, it'd need to take an owner input. No action needed for this PR.

5. Recommendation

Land it. The faithful inlining + correct permission scope reduction is a real ergonomic win for the eight downstream callers. The scripts/ cleanup can follow in a one-line PR.

[Jerry-Xin]

Review Summary

Clean, well-scoped refactor. The permissions intersection problem is real and the fix is correct.

✅ Verified

• Script fidelity: All 3 inlined Python scripts are byte-identical to their scripts/*.py originals on main. No logic drift.
• Heredoc safety: << 'PYEOF' (single-quoted delimiter) correctly prevents shell expansion of $ inside Python f-strings.
• Permission reduction is accurate:
  • octo-issue-feed.yml: contents: read → permissions: {} ✓
  • octo-pr-feed.yml: contents: read → permissions: {} ✓
  • octo-ci-status.yml: drops contents: read, keeps actions: read (still needed for GitHub Actions API history fetch) ✓
• Backward compatibility: Permissions are relaxed, so existing callers with permissions: {} will work correctly instead of silently 403-ing on checkout. Zero caller changes needed.
• CI: add-to-project failure is pre-existing (missing github-token secret), unrelated to this PR.

💬 Non-blocking

1. Dead scripts/ directory — The 3 original .py files under scripts/ are now completely unreferenced. Consider deleting them in this PR (or a follow-up) to avoid confusion.
2. send() duplication — The retry-aware send() helper is now copy-pasted across all 3 workflows (~30 lines × 3). Acceptable trade-off for eliminating the checkout dependency, but worth noting for future maintainability.
3. Tagging strategy — PR body suggests v1.1 or v2. Since this is a transparent improvement (callers don't need changes), v1.1 with a moving v1 tag update seems cleaner than a major bump.

LGTM. Ship it.

[Jerry-Xin]

Re-review after commit c8a00be.

Previous APPROVE concern resolved: scripts/ directory (3 dead .py files) is now properly deleted in the new commit. Inlined scripts remain byte-identical to the removed originals.

Changes verified (6 files):

• 3 workflows: checkout step removed → heredoc inline, permissions narrowed ✅
• 3 scripts: deleted (no longer referenced) ✅
• CI failure (add-to-project missing github-token) is pre-existing, unrelated ✅

Clean and complete. LGTM.

[yujiawei]

Code Review — PR #15 (.github)

Summary

Faithful, well-scoped refactor that correctly addresses the reusable-workflow permissions-intersection problem. Three Python helper scripts are inlined into their workflow YAMLs as python3 - << 'PYEOF' heredocs; the standalone scripts/*.py files and the actions/checkout step that fetched them are deleted. Behavior is unchanged; the change is purely structural.

The current head c8a00be is one commit ahead of an earlier reviewed revision (18c2c12) — the only delta is removal of the now-unreferenced scripts/ directory, which was the main non-blocking nit on the previous pass. With that cleaned up, the PR is in good shape.

Verdict: COMMENTED — no blockers; a couple of forward-looking notes below.

1. Verification

Check	Result
Inlined octo-ci-status.yml heredoc body == previously-deleted scripts/octo_ci_status_notify.py	✅ identical (178 lines)
Inlined octo-issue-feed.yml heredoc body == previously-deleted scripts/octo_issue_feed_notify.py	✅ identical (79 lines)
Inlined octo-pr-feed.yml heredoc body == previously-deleted scripts/octo_pr_feed_notify.py	✅ identical (84 lines)
Heredoc delimiter is single-quoted (<< 'PYEOF') on all three files	✅ — prevents shell expansion of $, f-strings, etc.
YAML literal-block indentation consistent	✅ uniform 10-space body indent; PYEOF terminator at the same column so YAML strip yields a column-0 sentinel
env: block in each step still passes every variable the inlined script reads	✅ verified per workflow (CONCLUSION, REPO_NAME, WORKFLOW_NAME, RUN_ID, WORKFLOW_ID, RUN_URL, PROJECT_GROUP_ID, GITHUB_TOKEN, OCTO_BOT_TOKEN for ci-status; analogous sets for the other two)
scripts/ directory completely removed at HEAD	✅ (404 from GET /contents/scripts at this SHA — confirmed)
No remaining scripts/ references anywhere in .github/workflows/	✅

2. Permissions Analysis

Workflow	Final permissions	API surface used by inlined script	Correct?
octo-ci-status.yml	actions: read	GET /repos/{org}/{repo}/actions/runs/{id} and …/actions/workflows/{id}/runs?...	✅ actions: read is the exact scope these endpoints require — and the only one
octo-issue-feed.yml	permissions: {}	None (only POSTs to im.deepminer.com.cn)	✅
octo-pr-feed.yml	permissions: {}	None (only POSTs to im.deepminer.com.cn)	✅

The PR body's premise — that the GITHUB_TOKEN issued to a called workflow is bounded by the caller's permissions: declaration, so a callee declaring contents: read forced every caller to also grant contents: read — is correctly applied here. After this lands, the issue/PR feed workflows have no GitHub API surface left to gate, and octo-ci-status.yml still asks the caller for actions: read (intrinsic to reading run history; unchanged from before this PR for that workflow's purposes).

Minor accuracy note on the PR description: it claims callers' "existing permissions: {} is now fully compatible." Spot-checking actual sub-repo callers (e.g. Mininglamp-OSS/octo-server, octo-deployment, etc.) shows they don't declare a top-level permissions: at all — they inherit repo defaults. That is still compatible (and arguably better, since the new callees clamp the token regardless), but the table's pre-state is slightly mis-stated.

3. Findings

Note 1 — Inlined Python is harder to edit and lint than scripts/*.py

You lose Python syntax highlighting, formatting, and mypy / ruff / pyflakes coverage on the inline body — it's now a YAML string. For 79–178 lines × 3 files this is acceptable, but if any of these scripts grow, two paths to consider:

• Switch to a pure actions/github-script step (no Python, no checkout) for the GitHub API path in octo-ci-status.yml; or
• Accept the checkout cost for that one workflow and keep the other two inline.

This is a forward note, not a request for change here.

Note 2 — send() is now duplicated three times in YAML rather than three times in .py

The retry-aware send() helper (~40 lines) is copy-pasted across all three workflow files. This existed before too (it was duplicated across the three scripts/*.py), so the refactor doesn't add duplication, but it's a forward note: any bug fix to retry logic, Retry-After handling, etc. now needs to be applied in three YAML heredocs instead of three Python files. Not blocking.

Note 3 — Versioning / release strategy

v1 is the only tag today, and all observed sub-repo callers pin to @v1. Two paths from the PR description, both fine:

• Cut v2 (callers opt in by explicit bump) — safer signal, matches semver-style intent.
• Move v1 to this commit — also acceptable here because behavior is unchanged, but it sets a precedent that v1 is mutable. If you take this route, please document the retag in release notes so future drift is traceable.

Either way, no code change in this PR.

4. Additional Findings (Outside Task Scope)

• Send-failure semantics: each workflow exits with sys.exit(1) if any group push fails after retries. That marks the whole reusable-workflow run as failed when only one of two channels is degraded. Reasonable, matches prior behavior — flagging it because someone reading the inlined code in YAML for the first time might miss that the trailing if failed: sys.exit(1) is the failure gate.
• Hardcoded Mininglamp-OSS org in octo-ci-status.yml API URLs (e.g. f'https://api.github.com/repos/Mininglamp-OSS/{repo}/...'): pre-existing, not introduced by this PR. If the workflow is ever published outside the org, an owner input would be needed. No action for this PR.
• First-run guard: when no prior completed run exists, the script silently skips. Correct, but the prose comment ("First run detected") will also fire when the workflow's history was just cleared (e.g. after retention pruning) — minor edge case worth being aware of operationally; not a fix request.

5. Recommendation

Land it. Faithful inlining + correct permission-scope reduction is a real ergonomic win for the eight downstream callers, and the trailing commit on this branch already tidied up the now-dead scripts/ directory. The two forward notes above can be revisited only if/when these scripts grow.