feat(manifest): add fork-identity fields for Phase C / P4 (no forks yet)

[tonythethompson]

Summary

Tooling-only groundwork for the selective fork policy (P4). Does not fork anything — see numan-registry PR (companion to this one) and docs/adr/0001-... in numan for why real forks are explicitly out of scope here.

• validate_manifest.py: optional upstream_repo field (owner/name of the original upstream). Requires owner == "numan-maintained" on any entry that sets it — a fork manifest entry can't claim the original author's identity.
• gen_spec.py: when upstream_repo is present, emits source.upstream in the generated spec (consumed by numan-registry's add-package.py, which now requires it for owner: "numan-maintained") and appends a fork-attribution note to the description.
• manifest.json: schema_notes.upstream_repo documents the field and points at ADR 0001 for the eligibility/stewardship requirements this field doesn't itself enforce.

Test plan

• python -m pytest scripts/ — 47 passed
• python scripts/validate_manifest.py --manifest manifest.json — OK, 22 active plugins, unaffected
• New tests: upstream_repo requires numan-maintained owner / accepted with it / rejects blank, gen_spec.py emits source.upstream + description note for fork entries, omits both for normal entries

Ref: numan/docs/plans/2026-08-09-intake-process-reform.md (P4), numan PR #113.

🤖 Generated with Claude Code

---

Summary by cubic

Adds fork identity support for P4 selective forks and hardens manifest/spec validation, including source.upstream and a fork note for valid numan-maintained forks. Tooling-only; no forks created.

• New Features

  • validate_manifest.py: require upstream_repo for owner numan-maintained; enforce non-empty owner/name; reject values equal to repo.
  • gen_spec.py: for fork entries, emit source.upstream and append a fork-attribution note; omit both for non-forks (needed by numan-registry).
  • manifest.json: add schema_notes.upstream_repo with ADR link.

• Bug Fixes

  • gen_spec.py: build_spec validates upstream_repo via the shared helper, rejecting blank, malformed, wrong-owner, or self-referential values before emitting provenance.

^{Written for commit dcafab1. Summary will update on new commits.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The manifest now documents and validates optional upstream_repo metadata. build_spec adds fork attribution to descriptions and source metadata. Tests cover fork, non-fork, ownership, missing values, and malformed repositories.

Changes

Upstream repository provenance

Layer / File(s)	Summary
Upstream metadata contract and validation manifest.json, scripts/validate_manifest.py, scripts/test_validate_manifest.py	The manifest documents upstream_repo. Validation checks GitHub repository slugs, ownership rules, required values, self-reference, and malformed inputs for active entries.
Generated specification provenance scripts/gen_spec.py, scripts/test_gen_spec_source.py	Fork entries add an upstream URL to source and a fork note to the description. Non-fork entries omit both fields. Tests cover valid forks, self-reference errors, and non-fork output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 8

✅ Passed checks (8 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Pipeline Stage Enum Ordering	✅ Passed	The PR changes only manifest tooling and tests; no SessionWorkflowStage enum, stage members, comparisons, or converters exist in the tracked solution.
Gpu/Cpu Runtime Boundary	✅ Passed	The PR changes only Python files under scripts/; no inference/, CPU/GPU requirements, main.py, or C# files are modified, so this runtime-boundary check does not apply.
Managed Host Restart Safety	✅ Passed	The PR modifies manifest/spec validation code and tests only; no target host-manager symbols or restart/lease/busy-state paths exist in the repository.
Linked Issues check	✅ Passed	The description references the related intake plan, ADR, and companion pull request.
Out of Scope Changes check	✅ Passed	The changes remain within scope by adding tooling support and tests without creating forks.
Title check	✅ Passed	The title clearly identifies the manifest fork-identity fields and states that the change does not create forks.
Description check	✅ Passed	The description accurately explains the validation, specification-generation, documentation, and test changes in the pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/p4-fork-identity-tooling

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch feat/p4-fork-identity-tooling

Warning

Review ran into problems

🔥 Problems

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. Analyzed tonythethompson/QuickShell, tonythethompson/numan, tonythethompson/dependency-chain-substrate, skipped Trackdubllc/Trackdub.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Sorry @tonythethompson, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[qodo-code-review]

PR Summary by Qodo

Add fork identity support via optional upstream_repo in manifest + spec output

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Add optional manifest upstream_repo field for fork attribution under numan-maintained
• Emit source.upstream and a fork attribution note in generated registry specs
• Add validation + tests to enforce owner rules and reject blank upstream_repo

Diagram

graph TD
  M[("manifest.json")] --> V["validate_manifest.py"] --> G["gen_spec.py"] --> S["generated spec"] --> R{{"numan-registry"}}
  M --> G
  subgraph Legend
    direction LR
    _db[(Data file)] ~~~ _tool[Tool script] ~~~ _ext{{External}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Store upstream as full URL (upstream_url)

• ➕ Avoids implicit GitHub-only assumption and string concatenation
• ➕ More explicit/ready for non-GitHub upstreams
• ➖ More verbose in the manifest; harder to validate consistently
• ➖ Doesn't match existing repo fields that use owner/name

2. Introduce explicit fork marker (is_fork + upstream_repo)

• ➕ Clearer semantics than inferring fork-ness from upstream_repo presence
• ➕ Could support future fork metadata without overloading one field
• ➖ Adds additional schema surface area for minimal immediate benefit
• ➖ Still needs the owner==numan-maintained enforcement logic

Recommendation: Current approach is a good minimal-change foundation: upstream_repo stays optional, preserves original identity without claiming it, and cleanly propagates to the generated spec as source.upstream for downstream enforcement. The owner==numan-maintained gate in validate_manifest.py is a pragmatic safeguard consistent with the stated “tooling-only” scope.

Files changed (5) +105 / -7

Enhancement (2) +25 / -7

gen_spec.pyEmit source.upstream and fork attribution note when upstream_repo is set +16/-7

Emit source.upstream and fork attribution note when upstream_repo is set

• Refactors spec construction to build description/source objects first, then conditionally appends a fork attribution suffix and emits source.upstream as a GitHub URL when upstream_repo is present. Leaves non-fork entries unchanged.

scripts/gen_spec.py

validate_manifest.pyValidate upstream_repo presence and enforce numan-maintained owner +9/-0

Validate upstream_repo presence and enforce numan-maintained owner

• Introduces optional upstream_repo validation: must be a non-empty string when present and requires owner to be exactly 'numan-maintained' to prevent identity confusion. Keeps existing manifest and target validation behavior intact.

scripts/validate_manifest.py

Tests (2) +77 / -0

test_gen_spec_source.pyTest fork vs non-fork spec emission for source.upstream +62/-0

Test fork vs non-fork spec emission for source.upstream

• Adds tests ensuring fork entries emit source.upstream and include a fork attribution note in the description, while non-fork entries omit both. Retains existing provenance emission coverage.

scripts/test_gen_spec_source.py

test_validate_manifest.pyAdd validation tests for upstream_repo owner/blank rules +15/-0

Add validation tests for upstream_repo owner/blank rules

• Adds unit tests verifying that upstream_repo is only allowed for owner=='numan-maintained', is preserved when valid, and is rejected when blank.

scripts/test_validate_manifest.py

Documentation (1) +3 / -0

manifest.jsonDocument upstream_repo schema notes for fork attribution +3/-0

Document upstream_repo schema notes for fork attribution

• Adds schema_notes.upstream_repo describing the new optional per-entry field and its intended use for numan-maintained forks. Includes a pointer to the ADR covering eligibility/stewardship requirements (not enforced by this field).

manifest.json

[greptile-apps]

Greptile Summary

The PR adds fork-identity validation and generated-spec provenance for future numan-maintained forks.

• Requires maintained-fork entries to provide a distinct upstream_repo.
• Emits the upstream repository URL and fork attribution in generated specifications.
• Documents the new manifest field and adds validation/spec-generation coverage.

Confidence Score: 4/5

The PR is not yet safe to merge because malformed repository identities can still pass validation and produce incorrect upstream URLs.

The new slug regex excludes slashes and whitespace inside each segment but still accepts URL delimiter characters; https:/github.com reaches spec generation and becomes https://github.com/https:/github.com.

Files Needing Attention: scripts/validate_manifest.py

Important Files Changed

Filename	Overview
scripts/validate_manifest.py	Adds centralized validation for maintained-fork ownership, required upstream identity, slug shape, and self-reference.
scripts/gen_spec.py	Reuses manifest validation and emits upstream provenance plus a fork-attribution description for maintained forks.
scripts/test_validate_manifest.py	Covers ownership, presence, blank values, common malformed forms, and self-referential upstream identities.
scripts/test_gen_spec_source.py	Covers generated provenance, attribution text, validation failures, and unchanged non-fork output.
manifest.json	Documents the optional fork-identity field and its intended stewardship constraints.

_{Reviews (7): Last reviewed commit: "Potential fix for pull request finding" | Re-trigger Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3de7acfd42

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Weak upstream_repo validation ✓ Resolved 🐞 Bug ≡ Correctness

Description

validate_manifest() accepts whitespace-only or malformed upstream_repo values (it only checks
non-empty, without stripping or enforcing the documented owner/name shape), which can lead
gen_spec.py to emit invalid source.upstream URLs and misleading description text. This can break
downstream consumers and undermines the provenance metadata this field is meant to capture.

Code

scripts/validate_manifest.py[R107-110]

+        upstream_repo = entry.get("upstream_repo")
+        if upstream_repo is not None:
+            if not isinstance(upstream_repo, str) or not upstream_repo:
+                raise ValueError(f"{name}: upstream_repo must be a non-empty string when present")

Relevance

●●● Strong

Stricter input validation (strip/format checks) is a small, deterministic correctness fix they
usually take.

PR-#3
PR-#12

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The validator only checks not upstream_repo, which does not reject whitespace-only strings, and it
does not enforce the documented owner/name format. gen_spec then uses the raw value to build both
the fork attribution note and the https://github.com/{upstream_repo} URL.

scripts/validate_manifest.py[105-116]
scripts/gen_spec.py[151-164]
manifest.json[4-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`scripts/validate_manifest.py` currently treats any non-empty string as a valid `upstream_repo`. This allows whitespace-only values (e.g., `'   '`) and other malformed strings that are not in the documented `owner/name` format, which can then produce invalid GitHub URLs and incorrect fork attribution in `scripts/gen_spec.py`.

### Issue Context
`manifest.json` documents `upstream_repo` as an `owner/name` identifier, and `gen_spec.py` converts it into `https://github.com/{upstream_repo}`.

### Fix Focus Areas
- scripts/validate_manifest.py[107-116]
- scripts/gen_spec.py[151-164]
- scripts/test_validate_manifest.py[103-117]

### What to change
1. In `validate_manifest()`, if `upstream_repo` is present:
  - Require it to be a `str`.
  - `strip()` it and reject if blank after stripping.
  - Validate it matches the intended `owner/name` shape (e.g., exactly one `/`, no spaces; optionally a conservative regex like `^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`).
  - Optionally normalize by storing the stripped value back into the entry.
2. Add unit tests:
  - Reject whitespace-only values.
  - Reject obvious malformed values (e.g., missing `/`, multiple `/`, includes `http://`).
3. (Optional) In `gen_spec.py`, consider also stripping before rendering to be robust even if `build_spec()` is called with unvalidated data.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Missing fork invariant check ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

build_spec() will emit “numan-maintained fork” attribution and source.upstream for any truthy
upstream_repo without verifying owner=='numan-maintained', so direct use of gen_spec.py/build_spec()
on unvalidated input can generate policy-inconsistent specs. This is a defense-in-depth gap given
validate_manifest enforces the owner constraint separately.

Code

scripts/gen_spec.py[R161-164]

+    if upstream_repo:
+        description += f" (numan-maintained fork; upstream: {upstream_repo})"
+        source["upstream"] = f"https://github.com/{upstream_repo}"
+

Relevance

●●● Strong

Repo often accepts defense-in-depth hardening in scripts; local owner-guard in build_spec is
low-risk.

PR-#3
PR-#12

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

gen_spec’s fork attribution path is keyed only on truthiness of upstream_repo, while the owner
restriction is enforced only in validate_manifest, not in build_spec itself.

scripts/gen_spec.py[151-164]
scripts/validate_manifest.py[107-115]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`build_spec()` adds a “numan-maintained fork” note and emits `source.upstream` whenever `upstream_repo` is truthy, but it does not enforce the corresponding owner invariant (`owner == 'numan-maintained'`). This relies on callers always running `validate_manifest()` first.

### Issue Context
CI may validate manifests elsewhere, but `gen_spec.py` itself loads entries and generates specs without calling `validate_manifest()`.

### Fix Focus Areas
- scripts/gen_spec.py[151-176]

### What to change
- In `build_spec()`, before emitting fork attribution:
 - If `upstream_repo` is set and `entry['owner'] != 'numan-maintained'`, raise `ValueError` (or omit fork attribution entirely).
 - Consider also requiring `upstream_repo` to be in the expected format (or rely on `validate_manifest()` after it is hardened).
- Add a unit test that passes `owner != 'numan-maintained'` with `upstream_repo` set and asserts `build_spec()` fails closed or does not label it as a numan-maintained fork.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 5 rules

✅ REVIEW.md

Review mode: ⚖️ Balanced: This is a behavioral manifest/schema and spec-generation change spanning validation, output contracts, and tests; it has several edit sites and merits a complete single-pass review, but not the defect density requiring redundant extended passes.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

Qodo Fixer

No findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page.

[tonythethompson]

Fixed in ee44791: numan-maintained entries now require upstream_repo, and upstream_repo must match the documented owner/name form (rejects whitespace, URLs, and extra path segments). Added regression tests.

[tonythethompson]

cde847e: upstream_repo now requires numan-maintained owner in gen_spec (defense in depth); manifest ADR link points to tonythethompson/numan on GitHub. upstream_repo validation landed in ee44791/92bda94.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/validate_manifest.py`:
- Line 73: Remove the behavior-only docstring from the manifest validation
function associated with optional fork upstream_repo metadata. Do not replace it
with another comment unless it documents a non-obvious rationale or invariant.
- Around line 72-89: Reject self-referential upstream provenance by updating
validate_upstream_repo in scripts/validate_manifest.py to compare upstream_repo
with entry["repo"] case-insensitively and raise a validation error when they
identify the same GitHub repository. In scripts/gen_spec.py lines 151-167, call
the shared validation helper before emitting source.upstream so direct
build_spec callers enforce the same invariant. Add matching rejection coverage
in scripts/test_validate_manifest.py lines 123-135 and
scripts/test_gen_spec_source.py lines 67-99, including case-only repository
variations.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 944e6c09-4ede-43f5-a062-f6042a18f2c7

📥 Commits

Reviewing files that changed from the base of the PR and between 4d4c475 and cde847e.

📒 Files selected for processing (5)

• manifest.json
• scripts/gen_spec.py
• scripts/test_gen_spec_source.py
• scripts/test_validate_manifest.py
• scripts/validate_manifest.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• tonythethompson/QuickShell (manual)
• tonythethompson/numan (manual) → reviewed against open PR #116 feat/p4-fork-identity-tooling instead of the default branch
• tonythethompson/dependency-chain-substrate (manual)

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (7)

**/*.{cs,py}

📄 CodeRabbit inference engine (Custom checks)

**/*.{cs,py}: Ensure managed host restarts are safe: modifications involving ManagedVenvHostManager, ContainerizedServiceProbe, ContainerizedInferenceClient, or ContainerizedProviderReadiness must not call StopTrackedHost, RestartAsync, or equivalent host-kill methods unless active request leases are zero. Hosts reporting busy_reason or active_requests > 0 must not be restarted, and readiness probing must not downgrade a known-available host merely because a background refresh is in flight.
Maintain SessionWorkflowStage ordering as Foundation < MediaLoaded < Transcribed < Diarized < Translated < TtsGenerated. Comparisons must use enum members rather than raw integer literals. New or renumbered members require a legacy-compatible JSON converter for old numeric values, and reordered enums require checking every inequality comparison across the solution for preserved semantic meaning.

Files:

• scripts/test_gen_spec_source.py
• scripts/gen_spec.py
• scripts/validate_manifest.py
• scripts/test_validate_manifest.py

**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Use Serena's semantic, symbol-aware tools as the primary tools for code work; use built-in Read, Glob, Grep, and Edit only when Serena cannot perform the task or an explicitly listed exception applies.
Use the appropriate Serena symbol tool for code operations: get_symbols_overview for structure, find_symbol for symbol bodies, find_referencing_symbols for callers, find_declaration or implementations for declarations, replace_symbol_body for edits, insertion tools for nearby additions, replace_content for pattern replacement, and rename, move, safe-delete, or inline tools for refactoring.
Before editing code, obtain a symbol overview of the target file, inspect the specific symbols with bodies included, and edit using Serena's symbol-aware tools.
Understand existing code before changing it and make the smallest change that satisfies the request; avoid unrelated cleanup, premature abstractions, speculative error handling, feature flags, and compatibility shims unless requested.
Prefer editing existing files over creating new ones, and never create Markdown or README files unless explicitly requested.
For exploratory questions, provide a recommendation and the main tradeoff in two or three sentences without implementing changes until the user agrees.
For UI or frontend changes that cannot be tested in a browser, explicitly state that browser testing was not performed rather than claiming success.
Identify and fix security issues when discovered, including injection, XSS, SQL injection, path traversal, and secret leaks.
Pause and obtain confirmation before destructive, hard-to-reverse, externally visible, or third-party-upload actions, including deleting files or branches, dropping tables, killing processes, force-pushing, changing CI/CD, posting externally, or uploading content.
When encountering an obstacle, investigate the root cause instead of bypassing it with --no-verify, --force, or deletion; investigate unfamiliar files, branches, and configuration befor...

Files:

• scripts/test_gen_spec_source.py
• scripts/gen_spec.py
• manifest.json
• scripts/validate_manifest.py
• scripts/test_validate_manifest.py

**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kt,kts}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add comments unless the rationale is non-obvious, such as a hidden constraint, workaround, subtle invariant, or other necessary explanation; do not narrate behavior or reference the current task in comments.

Files:

• scripts/test_gen_spec_source.py
• scripts/gen_spec.py
• scripts/validate_manifest.py
• scripts/test_validate_manifest.py

scripts/**/*.py

📄 CodeRabbit inference engine (REVIEW.md)

scripts/**/*.py: The CI gates must pass: python3 -m compileall -q scripts and python3 -m unittest discover -s scripts -p "test_*.py" -v.
Packaging and release scripts must preserve draft ownership, refuse to overwrite existing release tags or assets, and fail closed when the complete asset set is unavailable.
Generated specifications must omit authored sha256 hashes; hashes are downloaded and generated by numan-registry during intake.

Files:

• scripts/test_gen_spec_source.py
• scripts/gen_spec.py
• scripts/validate_manifest.py
• scripts/test_validate_manifest.py

scripts/**/*.{py,json}

📄 CodeRabbit inference engine (REVIEW.md)

Packaging and release helpers must have tests covering failure modes, including incomplete asset sets, overwrite attempts, and release transaction errors.

Files:

• scripts/test_gen_spec_source.py
• scripts/gen_spec.py
• scripts/validate_manifest.py
• scripts/test_validate_manifest.py

manifest.json

📄 CodeRabbit inference engine (REVIEW.md)

manifest.json: Each active[] manifest entry must pin a human-facing upstream tag and immutable source_commit; workflows must verify that the tag resolves to the pinned commit.
Changes to the active publish set must match the pull request scope; backlog-only edits must not silently modify active[].

Files:

• manifest.json

scripts/validate_manifest.py

📄 CodeRabbit inference engine (REVIEW.md)

Manifest validation with python3 scripts/validate_manifest.py --verify-upstream must pass.

Files:

• scripts/validate_manifest.py

🪛 Ruff (0.16.1)

scripts/gen_spec.py

[warning] 163-165: Avoid specifying long messages outside the exception class

(TRY003)

scripts/validate_manifest.py

[warning] 78-78: Avoid specifying long messages outside the exception class

(TRY003)

---

[warning] 80-82: Avoid specifying long messages outside the exception class

(TRY003)

---

[warning] 84-87: Avoid specifying long messages outside the exception class

(TRY003)

---

[warning] 89-89: Avoid specifying long messages outside the exception class

(TRY003)

scripts/test_validate_manifest.py

[warning] 105-105: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

---

[warning] 115-115: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

---

[warning] 120-120: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

---

[warning] 134-134: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

🔍 Remote MCP GitHub Copilot

Relevant review context

• The current PR is #82 in tonythethompson/numan-plugins; its checks include successful manifest/packaging validation, while the AI reviewer check is neutral.
• The companion registry PR numan-cli/numan-registry#66 adds the consumer-side source.upstream support. It additionally requires an HTTP(S) URL, rejects source.upstream on non-numan-maintained packages, and rejects an upstream URL identifying the same repository as source.git.
• PR #82’s manifest validation only checks a loose owner/name slug. gen_spec.py also only checks the numan-maintained owner, so malformed or self-referential upstream values are not rejected until the registry intake step.
• The base manifest already contains two entries tagged "fork" but owned by tonythethompson (nu_plugin_qr_maker and nu_plugin_explore); no upstream_repo fields are present on those entries. This is worth reconciling with the PR’s stated “no forks yet” scope and fork-identification policy.
• The related numan-plugins#80 also changes validate_manifest.py and gen_spec.py for commit-snapshot intake, so review should account for possible stacked-PR interactions rather than treating these files as isolated.

🔇 Additional comments (1)

manifest.json (1)

4-6: LGTM!

[tonythethompson]

Addressed in 6b43fcb (pushed): a numan-maintained entry can no longer set upstream_repo equal to its own repo (case-insensitive) — validated in both validate_manifest.py and gen_spec.py's build_spec() for direct callers, with tests covering both.

The two codex comments were already stale against current code: the reverse check (owner == 'numan-maintained' requires upstream_repo) already exists at validate_manifest.py:88-89, and the ADR path referenced in manifest.json's schema_notes (docs/adr/0001-ecosystem-trust-upstream-contribution-fork-stewardship.md in the numan repo) does exist there — verified directly.

Removed the restating docstring flagged by CodeRabbit and replaced it with one stating the actual invariants (fork can't claim original owner's identity, can't omit upstream_repo, can't self-reference) since this repo's docstring-coverage test requires every function to have one.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/gen_spec.py (1)

151-169: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Run the full upstream validator in build_spec.

When upstream_repo is absent or "", if upstream_repo: skips validation. A direct build_spec() call can emit a numan-maintained specification without source.upstream, although manifest validation and registry intake require it. A malformed truthy slug also reaches the output as https://github.com/<value>.

Call validate_upstream_repo(entry) before this branch. Then use if upstream_repo is not None and remove the duplicate ownership and self-reference checks. Add direct build_spec() coverage for absent, blank, and malformed values.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/gen_spec.py` around lines 151 - 169, The build_spec flow must always
validate upstream_repo before branching, including absent, blank, and malformed
values. Call validate_upstream_repo(entry) before the upstream handling, change
the condition to distinguish None from an empty string, and remove the duplicate
owner and self-reference checks while preserving source.upstream generation only
for valid configured values; add direct build_spec coverage for these cases.

Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@scripts/gen_spec.py`:
- Around line 151-169: The build_spec flow must always validate upstream_repo
before branching, including absent, blank, and malformed values. Call
validate_upstream_repo(entry) before the upstream handling, change the condition
to distinguish None from an empty string, and remove the duplicate owner and
self-reference checks while preserving source.upstream generation only for valid
configured values; add direct build_spec coverage for these cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a888067-dc71-48e2-ad01-fdbffd5e9af2

📥 Commits

Reviewing files that changed from the base of the PR and between cde847e and 6b43fcb.

📒 Files selected for processing (4)

• scripts/gen_spec.py
• scripts/test_gen_spec_source.py
• scripts/test_validate_manifest.py
• scripts/validate_manifest.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• tonythethompson/QuickShell (manual)
• tonythethompson/numan (manual) → reviewed against open PR #116 feat/p4-fork-identity-tooling instead of the default branch
• tonythethompson/dependency-chain-substrate (manual)

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{cs,py}

📄 CodeRabbit inference engine (Custom checks)

**/*.{cs,py}: Ensure managed host restarts are safe: modifications involving ManagedVenvHostManager, ContainerizedServiceProbe, ContainerizedInferenceClient, or ContainerizedProviderReadiness must not call StopTrackedHost, RestartAsync, or equivalent host-kill methods unless active request leases are zero. Hosts reporting busy_reason or active_requests > 0 must not be restarted, and readiness probing must not downgrade a known-available host merely because a background refresh is in flight.
Maintain SessionWorkflowStage ordering as Foundation < MediaLoaded < Transcribed < Diarized < Translated < TtsGenerated. Comparisons must use enum members rather than raw integer literals. New or renumbered members require a legacy-compatible JSON converter for old numeric values, and reordered enums require checking every inequality comparison across the solution for preserved semantic meaning.

Files:

• scripts/gen_spec.py
• scripts/test_validate_manifest.py
• scripts/test_gen_spec_source.py
• scripts/validate_manifest.py

**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Use Serena's semantic, symbol-aware tools as the primary tools for code work; use built-in Read, Glob, Grep, and Edit only when Serena cannot perform the task or an explicitly listed exception applies.
Use the appropriate Serena symbol tool for code operations: get_symbols_overview for structure, find_symbol for symbol bodies, find_referencing_symbols for callers, find_declaration or implementations for declarations, replace_symbol_body for edits, insertion tools for nearby additions, replace_content for pattern replacement, and rename, move, safe-delete, or inline tools for refactoring.
Before editing code, obtain a symbol overview of the target file, inspect the specific symbols with bodies included, and edit using Serena's symbol-aware tools.
Understand existing code before changing it and make the smallest change that satisfies the request; avoid unrelated cleanup, premature abstractions, speculative error handling, feature flags, and compatibility shims unless requested.
Prefer editing existing files over creating new ones, and never create Markdown or README files unless explicitly requested.
For exploratory questions, provide a recommendation and the main tradeoff in two or three sentences without implementing changes until the user agrees.
For UI or frontend changes that cannot be tested in a browser, explicitly state that browser testing was not performed rather than claiming success.
Identify and fix security issues when discovered, including injection, XSS, SQL injection, path traversal, and secret leaks.
Pause and obtain confirmation before destructive, hard-to-reverse, externally visible, or third-party-upload actions, including deleting files or branches, dropping tables, killing processes, force-pushing, changing CI/CD, posting externally, or uploading content.
When encountering an obstacle, investigate the root cause instead of bypassing it with --no-verify, --force, or deletion; investigate unfamiliar files, branches, and configuration befor...

Files:

• scripts/gen_spec.py
• scripts/test_validate_manifest.py
• scripts/test_gen_spec_source.py
• scripts/validate_manifest.py

**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kt,kts}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add comments unless the rationale is non-obvious, such as a hidden constraint, workaround, subtle invariant, or other necessary explanation; do not narrate behavior or reference the current task in comments.

Files:

• scripts/gen_spec.py
• scripts/test_validate_manifest.py
• scripts/test_gen_spec_source.py
• scripts/validate_manifest.py

scripts/**/*.py

📄 CodeRabbit inference engine (REVIEW.md)

scripts/**/*.py: The CI gates must pass: python3 -m compileall -q scripts and python3 -m unittest discover -s scripts -p "test_*.py" -v.
Packaging and release scripts must preserve draft ownership, refuse to overwrite existing release tags or assets, and fail closed when the complete asset set is unavailable.
Generated specifications must omit authored sha256 hashes; hashes are downloaded and generated by numan-registry during intake.

Files:

• scripts/gen_spec.py
• scripts/test_validate_manifest.py
• scripts/test_gen_spec_source.py
• scripts/validate_manifest.py

scripts/**/*.{py,json}

📄 CodeRabbit inference engine (REVIEW.md)

Packaging and release helpers must have tests covering failure modes, including incomplete asset sets, overwrite attempts, and release transaction errors.

Files:

• scripts/gen_spec.py
• scripts/test_validate_manifest.py
• scripts/test_gen_spec_source.py
• scripts/validate_manifest.py

scripts/validate_manifest.py

📄 CodeRabbit inference engine (REVIEW.md)

Manifest validation with python3 scripts/validate_manifest.py --verify-upstream must pass.

Files:

• scripts/validate_manifest.py

🪛 Ruff (0.16.1)

scripts/gen_spec.py

[warning] 167-167: Avoid specifying long messages outside the exception class

(TRY003)

scripts/test_validate_manifest.py

[warning] 125-125: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

scripts/test_gen_spec_source.py

[warning] 124-124: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

scripts/validate_manifest.py

[warning] 91-91: Avoid specifying long messages outside the exception class

(TRY003)

🔍 Remote MCP GitHub Copilot

Relevant review context

• PR #82 adds fork metadata only to the plugin build feeder; it does not add an active fork entry. validate_manifest() enforces the new invariants for manifest-driven flows.
• The companion registry PR #66 independently enforces that numan-maintained specs contain a valid HTTP(S) source.upstream, rejects upstream values identifying the same repository as source.git, and rejects source.upstream for non-numan-maintained packages.
• build_spec() only enters fork validation when upstream_repo is truthy. Therefore, a direct build_spec() call with owner: "numan-maintained" and a missing or blank upstream_repo does not fail locally, although manifest validation and registry intake reject it.
• build_spec() also does not validate the owner/name format or URL safety itself; those checks exist only in validate_manifest(). This is weaker than the PR’s stated defense-in-depth behavior for direct callers.
• PR #80 also modifies validate_manifest.py and gen_spec.py, so stacked-PR ordering or merge conflict resolution should be checked before approving.

🔇 Additional comments (4)

scripts/validate_manifest.py (1)

14-14: LGTM!

Also applies to: 72-95, 132-132

scripts/test_validate_manifest.py (1)

103-141: LGTM!

scripts/gen_spec.py (1)

171-181: LGTM!

scripts/test_gen_spec_source.py (1)

67-100: LGTM!

Also applies to: 101-130, 132-159

[Copilot]

Pull request overview

Adds manifest + spec “fork identity” metadata (upstream_repo → emitted as source.upstream) as groundwork for the P4 selective fork policy, along with validation and tests so downstream intake (numan-registry) can reliably attribute forks without actually introducing any forked entries yet.

Changes:

• scripts/validate_manifest.py: adds upstream_repo validation, including “only numan-maintained may set it” and “numan-maintained requires it”.
• scripts/gen_spec.py: when upstream_repo is present, emits source.upstream and appends a fork attribution note to the spec description.
• Updates manifest schema notes and adds unit tests covering the new validation/spec behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
scripts/validate_manifest.py	Adds upstream_repo invariants and integrates them into manifest validation.
scripts/test_validate_manifest.py	Adds unit tests for upstream_repo validation rules and malformed cases.
scripts/gen_spec.py	Emits source.upstream + fork attribution note when generating specs for fork entries.
scripts/test_gen_spec_source.py	Adds tests for fork spec emission and self-referential upstream rejection.
manifest.json	Documents upstream_repo in schema_notes and links to ADR 0001 for policy context.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linear-code]

TS-189