Skip to content

docs(instructions): document AZL component-modification workflow conventions#17411

Draft
PawelWMS wants to merge 1 commit into
4.0from
pawelwi/copilot-instructions-azl-workflows
Draft

docs(instructions): document AZL component-modification workflow conventions#17411
PawelWMS wants to merge 1 commit into
4.0from
pawelwi/copilot-instructions-azl-workflows

Conversation

@PawelWMS
Copy link
Copy Markdown
Contributor

This PR captures the conventions developed during a long-running multi-day session of remediating package-signing/scan failures at the component level (no allow-listing). The goal is to make those conventions discoverable to future Copilot sessions and agents, not to introduce new tooling.

What this PR adds

1. replace-upstream source-override pattern (comp-toml.instructions.md)

Documents the single-step pattern for serving a locally-modified upstream tarball under the same filename: one [[components.<name>.source-files]] block with replace-upstream = true and a replace-reason = "..." string. No separate file-remove overlay is needed; azldev's render step emits an audit WARN log naming the override and the from/to SHA-512 pair.

Covers field semantics, the lookaside URL pattern, the requirement that hash and the $hash segment of origin.uri stay in sync, replace-reason style rules, and the file-organization rules (delete inline components.toml entries when moving to a dedicated *.comp.toml; no tombstone comments).

2. skill-modify-source skill

New skill at .github/skills/skill-modify-source/SKILL.md documenting the canonical modify_source.sh pattern. The lookaside URL embeds the SHA-512 of the served file, so the repack must be byte-deterministic. Covers:

  • Required tar flags (--sort=name, --owner=0, --group=0, --numeric-owner, --mtime='@<epoch>', --format=gnu, LC_ALL=C).
  • Required xz flags (-T 1, -9).
  • Mandatory upstream SHA-512 verification before extract.
  • Strip-list vs keep-list strategies with list-discipline rules.
  • Output reporting contract (path, hash, ready-to-paste blob-upload command).
  • Determinism self-test recipe (two runs must produce byte-identical output).
  • Anti-patterns to avoid (multi-threaded xz without --block-size, mktemp for output filename, gzip without -n, find -newer, etc.).
  • Canonical ~40-line skeleton showing the whole pattern end-to-end.

3. skill-signing-failure-remediation skill

New skill at .github/skills/skill-signing-failure-remediation/SKILL.md documenting the decision tree:

  • (a) No reverse dependencies → drop the component outright (separate PR).
  • (b) Offending artefact lives in a test-only or otherwise-optional subpackage → use spec-remove-subpackage (or build.without if a clean bcond exists).
  • (c) Reverse deps require the component to keep building → strip subtrees from Source0 via the replace-upstream pattern with a modify_source.sh script (see skill-modify-source).

Always run a reverse-dependency scan before choosing (a) or (b).

4. Component-modification order of operations (copilot-instructions.md, skill-update-component)

Canonical sequence: edit comp.tomlazldev comp update -p <name> → commit all working-tree changes (toml + lock + source-side scripts) → azldev comp render -p <name> → amend (rendered spec). The commit between update and render is mandatory so the %changelog / Release: derived from git log match the committed history rather than the synthetic "dirty" working-tree state.

5. GPG signing (copilot-instructions.md)

Every commit MUST be GPG-signed. Verify with git log -1 --show-signature.

6. Public-content hygiene (copilot-instructions.md, comp-toml.instructions.md)

Overlay descriptions, replace-reason strings, comp.toml comments, spec files, modify_source.sh echo lines, commit messages, and PR descriptions describe motivations technically and neutrally — no specific internal infrastructure names (signing services, scanner brand names, internal pipeline/tooling names, internal Azure tenants, etc.). Use neutral phrasing.

7. Descriptions over comments (copilot-instructions.md rule 1, comp-toml.instructions.md)

Prefer schema-provided fields (description, replace-reason, skip_reason, …) over TOML comments. Every overlay MUST set description, every rationale string MUST be brief (one short line, motivational, no implementation detail). Includes DO / DO-NOT examples for both descriptions and the (rare) cases where a TOML comment is still appropriate.

8. Single-commit PRs (copilot-instructions.md rule 9)

Each PR lands as a single commit per CONTRIBUTING.md — Pull Request Workflow. If intermediate commits exist during development, squash them with git reset --soft $(git merge-base origin/<base> HEAD) && git commit -S -F <msg> (NOT git merge --squash) so commit metadata stays clean and no upstream changes get inverted.

9. Reviewer-facing-only commit/PR descriptions (copilot-instructions.md rule 10)

Commit messages and PR descriptions MUST describe only changes visible in the diff. Investigations, side-observations, or rejected approaches go to internal notes or follow-up issues, with a link back if needed.

10. Fix drift locally, not via CI patch artifacts (copilot-instructions.md rule 11, skill-update-component drift-fix discipline)

When Render + drift check or Update locks + drift check fails and the bot offers a rendered-specs-patch / locks-patch artifact, re-run the relevant azldev command locally and amend with that output — do NOT gh run download the artifact and git apply it.

11. spec-remove-subpackage / spec-remove-section failure modes (skill-fix-overlay)

Two failure modes documented: %define / %global inside the removed body vanishing (tracked in microsoft/azure-linux-dev-tools#203) and stray buildroot files. %if / %endif balancing around the removed section is handled automatically by the engine.

Files

File Change
.github/copilot-instructions.md Hygiene-rules expansion (descriptions-over-comments, signing, render order, single-commit, reviewer-facing descriptions, drift-fix discipline).
.github/instructions/comp-toml.instructions.md replace-upstream pattern, descriptions-over-comments, public-content hygiene, file-organization rules.
.github/skills/skill-modify-source/SKILL.md New skill — byte-deterministic Source0 repack.
.github/skills/skill-signing-failure-remediation/SKILL.md New skill — decision tree for signing/scan failures.
.github/skills/skill-fix-overlay/SKILL.md Refresh spec-remove-subpackage failure-mode list.
.github/skills/skill-update-component/SKILL.md Commit-order example + drift-fix discipline.
AGENTS.md Skill-index entries for the two new skills.
base/comps/AGENTS.md Directory-level restatement of components.toml hygiene rule.

Single GPG-signed commit per CONTRIBUTING.md — Pull Request Workflow.

…entions

Captures the conventions developed during the multi-day session of
remediating package-signing/scan failures at the component level
(no allow-listing) and folds them into permanent Copilot guidance.
No new tooling -- this is purely instructional / documentation
content to make future agent sessions discover and follow the
patterns we now use across the distro.

Changes
-------

1. `.github/instructions/comp-toml.instructions.md` (+69 lines).
   New "replace-upstream source override" section documenting the
   single-step pattern for serving a locally-modified upstream
   tarball under the same filename: one
   `[[components.<name>.source-files]]` block with
   `replace-upstream = true` and a `replace-reason = "..."` string.
   No separate `file-remove` overlay against `sources` is needed;
   `azldev`'s render step emits an audit WARN log naming the
   override and the from/to SHA-512 pair.

   Covers: field semantics (`filename`, `hash`, `hash-type`,
   `replace-upstream`, `replace-reason`, `origin.uri`); the
   lookaside URL pattern; the requirement that `hash` and the
   `$hash` segment of `origin.uri` stay in sync; the
   `replace-reason` style rules (single TOML string, no
   triple-quoted strings, self-contained motivation); and a
   "Public-content hygiene" subsection establishing that
   motivations be described technically and neutrally (no specific
   internal infrastructure names).

   Also documents the `components.toml` file-organization rule:
   when moving a component to a dedicated `*.comp.toml`, delete
   the inline entry; when removing a component, delete the inline
   entry. No tombstone comments.

2. `.github/skills/skill-modify-source/SKILL.md` (new, 267 lines).
   New skill documenting the canonical `modify_source.sh` pattern
   for byte-deterministic Source0 repacks. The lookaside URL
   embeds the SHA-512 of the served file, so the repack MUST be
   byte-deterministic: same upstream input ⇒ byte-identical
   output ⇒ identical SHA-512, across machines and re-runs.

   Sections:
     (a) Anatomy of `modify_source.sh` (scratch dir under
         `base/build/work/scratch/<name>/`, `BASH_SOURCE[0]`
         root resolution, `set -euo pipefail`).
     (b) The byte-deterministic repack contract -- required `tar`
         flags (`--sort=name`, `--owner=0 --group=0`,
         `--numeric-owner`, `--mtime='@<epoch>'`, `--format=gnu`,
         `LC_ALL=C`) and `xz` flags (`-T 1`, `-9`), plus the
         forbidden set.
     (c) Mandatory upstream SHA-512 verification before extract.
     (d) Strip-list vs keep-list strategies with list-discipline
         rules.
     (e) Output-and-reporting contract (path, hash, ready-to-paste
         `az storage blob upload` command, comp.toml update
         reminder).
     (f) Determinism self-test recipe (two consecutive runs must
         produce byte-identical output).
     (g) Anti-patterns (`mktemp` for output filename, `$$` / `$RANDOM`
         / `$(date)` intermediates, `gzip` without `-n`, multi-
         threaded `xz` without `--block-size`, `find -newer`, etc.).
     (h) Canonical ~40-line bash skeleton showing the whole pattern
         end-to-end.

3. `.github/skills/skill-signing-failure-remediation/SKILL.md`
   (new, 98 lines). New skill documenting the decision tree for
   handling pipeline scan failures at the component level:
     (a) No reverse dependencies → drop the component outright
         (separate PR).
     (b) Offending artefact lives in a test-only / optional
         subpackage → use `spec-remove-subpackage` (or
         `build.without` if a clean bcond exists).
     (c) Reverse deps require the component to keep building →
         strip subtrees from Source0 via the `replace-upstream`
         pattern with a `modify_source.sh` script
         (see `skill-modify-source`).
   Always run a reverse-dependency scan before choosing (a) or (b).

4. `.github/copilot-instructions.md` (+3 lines). Three new
   repository-wide rules at the bottom of "Repository Hygiene
   Rules":
     6. Commit signing -- every commit MUST be GPG-signed;
        `git log -1 --show-signature` must show "Good signature".
     7. Public-content hygiene -- describe motivations in overlay
        descriptions, replace-reason strings, comp.toml comments,
        spec files, modify_source.sh echo lines, commit messages,
        and PR descriptions *technically and neutrally*. Use
        neutral phrasing such as "automated package-signing
        pipeline", "FS-aware deep scanner", "automated malware
        scan".
     8. Component-modification order of operations -- canonical
        sequence is `edit comp.toml → azldev comp update -p <name>
        → commit (toml + lock) → azldev comp render -p <name> →
        amend (rendered spec)`. Keeps the lock fingerprint,
        %changelog, and Release: in sync within a single commit.

5. `.github/skills/skill-update-component/SKILL.md` (+20/-4 lines).
   Clarified the commit ordering with a worked example. The
   canonical sequence is: edit `comp.toml`, `azldev comp update -p
   <name>` to refresh the lock, commit the comp.toml + lock,
   `azldev comp render -p <name>`, amend in the rendered spec.
   Spelled out *why* the second render-and-amend is needed
   (`%changelog` / `Release:` are derived from `git log`, not the
   working tree).

6. `AGENTS.md` (+2 lines). Added two rows to the skill index:
     | Byte-deterministic Source0 repack (`modify_source.sh` +
       `replace-upstream`) | `skill-modify-source` |
     | Remediate component-level signing/scan failures           |
       `skill-signing-failure-remediation` |

7. `base/comps/AGENTS.md` (+4 lines). Mirrored the
   `components.toml` hygiene rule at the directory-level guide:
   when moving a component to a dedicated file, delete the inline
   entry (no tombstone comment); when removing a component,
   delete the inline entry.

8. `.github/copilot-instructions.md` (+1 line, follow-up). Added
   rule 11 to "Repository Hygiene Rules": fix render / lock
   drift locally by re-running `azldev comp render -p <name>` /
   `azldev comp update -p <name>` and amending, never by
   `gh run download`-ing the `rendered-specs-patch` /
   `locks-patch` CI artifact and `git apply`-ing it. The
   artifact is a reviewer convenience; using it as a fix path
   skips local reproduction and masks any local/CI environment
   skew.

9. `.github/skills/skill-update-component/SKILL.md` (+2 lines,
   follow-up). Added a "Drift-fix discipline" paragraph under the
   existing "CI gotcha" section, cross-referencing rule 11.

10. `.github/copilot-instructions.md` + `.github/instructions/comp-toml.instructions.md`
    (follow-up). Encode the "descriptions over comments" policy that
    emerged from the TOML cleanup work in this session:
      - Rule 1 of "Repository Hygiene Rules" reworded from "Overlay
        descriptions" to "Descriptions over comments". Establishes that
        schema-provided fields (`description`, `replace-reason`,
        `skip_reason`, etc.) are preferred over TOML comments, and that
        every rationale string MUST be brief -- one short line,
        motivational (the *why*), not implementation detail.
      - Rule 9 updated to cite `CONTRIBUTING.md` -- Pull Request
        Workflow as the authoritative source for the
        single-commit-per-PR requirement.
      - `comp-toml.instructions.md` section renamed from "Adding
        Description Comments" to "Descriptions over comments".
        Rewritten to lead with the structured-field preference and to
        give DO / DO NOT examples for both descriptions and (sparing)
        TOML comments; the "References" subsection is retained.

11. `.github/skills/skill-fix-overlay/SKILL.md` (follow-up). Refresh
    the `spec-remove-subpackage` / `spec-remove-section` failure-mode
    list. The engine now balances surrounding `%if` / `%endif`
    wrappers automatically (microsoft/azure-linux-dev-tools#190), so
    the "greedy consumption of `%if` guards" entry was dropped along
    with its manual `spec-append-lines` workaround. The two remaining
    failure modes — `%define` / `%global` inside the removed body
    vanishes (tracked in microsoft/azure-linux-dev-tools#203) and
    stray buildroot files — stay as-is, with a cross-link to the
    upstream tracking issue for the macro case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant