docs: point the README at the code for required variables and collaborators #32

jercik · 2026-10-04T21:33:57Z

jercik commented

2026-10-04 21:33:57 +00:00

The README listed two sets a second time: the forge and runner variables required on every path, and the collaborators src/main.ts hands the orchestrator. Each list now names the code that defines it.

Required variables: forgeCredentials in src/credentials.ts, readWrapperInputs in src/wrapper/event-context.ts and src/main.ts, which read them before the fork gate. The environment table above the paragraph stays.
Collaborators: WrapperDeps in src/contract/types.ts.

This addresses review comment 122213 on #30 and the review's unadjudicated claim 01M44BTESXJKH5PCCHGSW3K4W5 ("README duplicates the required environment-variable set").

Stacked on #30.

🤖 Generated with Claude Code

The README listed two sets a second time: the forge and runner variables required on every path, and the collaborators `src/main.ts` hands the orchestrator. Each list now names the code that defines it. - Required variables: `forgeCredentials` in `src/credentials.ts`, `readWrapperInputs` in `src/wrapper/event-context.ts` and `src/main.ts`, which read them before the fork gate. The environment table above the paragraph stays. - Collaborators: `WrapperDeps` in `src/contract/types.ts`. This addresses [review comment 122213](https://code.j4k.dev/j4k-oss/review-wrapper/pulls/30#issuecomment-122213) on #30 and the review's unadjudicated claim `01M44BTESXJKH5PCCHGSW3K4W5` ("README duplicates the required environment-variable set"). Stacked on #30. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jercik added 1 commit

2026-10-04 21:33:57 +00:00

docs: point the README at the code for required variables and collaborators

commit-msg / commitlint (pull_request) Successful in 23s

Details

Checks / quality-checks (pull_request) Successful in 47s

Details

Review / Review (pull_request_target) Has been cancelled

Details

ab235f39c4

The environment section listed the forge and runner variables a second
time, and the composition section listed the orchestrator's collaborators.
Both lists now name the code that defines them: `forgeCredentials`,
`readWrapperInputs` and `src/main.ts` for the variables, `WrapperDeps` for
the collaborators.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jercik referenced this pull request

2026-10-04 21:34:27 +00:00

docs: point to the outcome set instead of restating it #30

jercik referenced this pull request

2026-10-04 21:34:28 +00:00

docs: point to the outcome set instead of restating it #30

forgejo-actions commented

2026-10-04 21:44:33 +00:00

Review 01M45PCTE20F1BP8DW7MH3EAHG — head bc6feebb4c323e5de12f497db1b545598f82263a

Review — j4k-oss/review-wrapper @ `f5714bee59`

Scope: diff against base tree 211444be4c6d
Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v4",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (5)

medium — The action-input table duplicates the input manifest

claim: 01M45PKHG05A01JTKWN246515H
anchor: README.md (snippet)

## Action inputs

| Input                     | Meaning                                                                                                                                                                                                                   |
| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `expected-service-origin` | Origin `REVIEW_SERVICE_URL` must match exactly, e.g. `https://review.j4k.dev`. Left unset — or set to the empty string an undefined `${{ vars.X }}` expands to — the pin is off and only the `https:` scheme is enforced. |

lens: writing-quality · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M45PRH1M2T7W5PV2GPNJAF2G · valid: The exact-grounded README table lists expected-service-origin and its behavior. The reviewer names action.yml as the runner-read source, reports that its input set contains the same sole member, and accounts for the origin-match and empty-value guidance there. This meets the restated-sets comparison requirement; agreement today leaves a separately maintained copy that can drift. Point to action.yml inputs, retaining distinct security rationale. Earlier matching claims and their rationales supply no concrete refutation; their valid verdicts are not the basis of this judgment.
disposition: none

The README creates a second inventory and description of action inputs that maintainers must update whenever the manifest changes. The copy agrees today, but an input change can leave users following an obsolete contract without any warning.

The source is the inputs mapping in action.yml, which the Actions runner reads. Its members are expected-service-origin; the README table covers expected-service-origin. No member appears in only one list. The table also repeats the manifest description of origin matching and the unset or empty input behavior.

Replace this section’s table with “Action inputs and their descriptions are defined in action.yml.” This follows the writing skill’s One Idea, One Place guidance and the restated-sets rule: it gives callers the authoritative contract while retaining the explanation in the manifest they can open.

I read the complete README, action.yml, and reviewCredentials in src/credentials.ts. The function confirms the documented nonempty origin check and empty-input behavior; this is a static documentation comparison, not a runtime reproduction.

The matching inventory establishes duplication. A separate intended audience unable to open action.yml would refute the source-pointer correction, but the README is distributed beside that manifest and establishes no such restriction.

medium — The recovery table copies the diagnostic formatter’s message inventory

claim: 01M45PM88CFCMMRJB44VRSFMX0
anchor: README.md (snippet)

Before retrying, act on the reported cause:

| Message                                       | Recovery                                                                                                               |
| --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- |
| `provider session limit reached`              | Wait for the session limit to reset or ask the operator to lift it.                                                    |
| `selected model at capacity`                  | Wait for the model to accept new work.                                                                                 |
| `agent execution failed`                      | Ask the operator to inspect the failed agent logs and correct the reported error.                                      |
| `sandbox infrastructure failed`               | Ask the operator to check sandbox provisioning, controller connectivity, and resources.                                |
| `execution failed for an unrecognized reason` | Copy the quoted code from `(code "…")` on that log line and ask the operator to look it up in the review service logs. |

lens: writing-quality · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M45PRH1M2T7W5PV2GPNJAF2G · valid: The exact-grounded table enumerates failure messages and recoveries. The reviewer supplies the four selected codes plus fallback in src/wrapper/remedies.ts, their corresponding display labels in committed dist/index.mjs, and reports that the job diagnostic emits the matching recovery. No case differs today, so this is a medium copied-set defect. The unavailable standalone j4k/review source does not leave an essential comparison missing: the reported shipped bundle provides the labels, and the local map defines the selected remedies. Point to the diagnostic and formatExecutionRecovery, preserving the subsequent retry distinction. Earlier accounts lacking dependency strings do not refute this current comparison.
disposition: none

Maintainers must keep this table synchronized with executable recovery diagnostics even though the job already prints the recovery beside the cause. The copy agrees at this revision, but changing a supported failure code or its remedy can silently leave README readers following an obsolete recovery path.

In src/wrapper/remedies.ts, executionRemedies defines provider-session-limit, provider-capacity, agent-execution-failed, and infrastructure-failure, and formatExecutionRecovery handles the unrecognized-code fallback. The committed dist/index.mjs contains formatExecutionFailure, whose corresponding labels are “provider session limit reached”, “selected model at capacity”, “agent execution failed”, and “sandbox infrastructure failed”, with “execution failed for an unrecognized reason” as fallback. The README covers these same labels and fallback; no member appears in only one inventory. Its recovery instructions also repeat executionRemedies and formatExecutionRecovery.

Replace the anchored passage with “Before retrying, follow the recovery printed beside the cause in the job diagnostic; formatExecutionRecovery in src/wrapper/remedies.ts defines it.” Keep the following paragraph’s distinction between rerunning failed triage and using the printed dispatch command for failed lens slots. This preserves the operational instruction, names its authoritative home, and applies the writing skill’s One Idea, One Place guidance without copying the supported cases.

I read the complete README, src/wrapper/remedies.ts, src/wrapper/orchestrator.ts, and the bundled formatExecutionFailure and formatExecutionRecovery in dist/index.mjs. The formatter imports its message mapping from the owner’s j4k/review repository; that repository itself was unavailable, but its shipped implementation is present in the reviewed bundle. This is a static source comparison, not a runtime reproduction.

The matching formatter cases establish the duplicate inventory. Evidence that these users cannot read either the job diagnostic or the named source would refute the proposed replacement; the README currently directs them to that diagnostic for the dispatch command.

medium — The fork-gate explanation wrongly promises that no credential function runs before it

claim: 01M45PMWXHPY2VRZ1SRGRVV283
anchor: README.md (snippet)

- Forgejo does pass secrets to fork `pull_request_target` runs, so the real boundary is
  code-path ordering: the fork gate is evaluated from the event payload **before** the
  credentials module is ever called. On the fork path the capability token environment
  variable is never read — enforced by a static isolation test and an orchestrator unit
  test.

lens: writing-quality · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M45PRH1M2T7W5PV2GPNJAF2G · valid: The exact-grounded sentence promises that the fork gate precedes any call to the credentials module. The concrete reported trace instead has main.ts call forgeCredentials from that module before readWrapperInputs and runWrapper, while reviewCredentials is deferred through openSession until after the fork branch. The composition claim also grounds the entry point's credential-read role. The static import-isolation check does not prohibit the earlier forgeCredentials call, so it cannot rescue the broad wording. Specify reviewCredentials while preserving the capability-token isolation guarantee. Earlier matching claims contain no concrete refutation; this is a documentation precision defect, not evidence that the capability token leaks.
disposition: none

A security reviewer can infer that fork runs avoid reading the forge task token as well as the service capability token, or treat the entry point’s credential read as a violation of the documented boundary. The passage says the gate runs “before the credentials module is ever called”, although that module supplies forge credentials before the gate on every run. The README’s Environment contract now explicitly describes that ordering, so its security explanation contradicts its setup contract.

src/main.ts calls forgeCredentials() while constructing forge, then awaits readWrapperInputs(forge), then invokes runWrapper. In src/credentials.ts, forgeCredentials reads the forge task token and API base. runWrapper in src/wrapper/orchestrator.ts evaluates deps.inputs.isFork before review calls deps.openSession; only that thunk calls reviewCredentials(). Thus the supported security property concerns the service credential function, not the entire module.

Replace “before the credentials module is ever called” with “before reviewCredentials() is called”. Keep the following sentence about the capability token never being read on the fork path. This preserves the intended boundary while applying the writing skill’s Use Precise Language guidance and preventing the broader, false interpretation.

I traced the complete src/main.ts, src/credentials.ts, readWrapperInputs in src/wrapper/event-context.ts, and runWrapper and review in src/wrapper/orchestrator.ts. I also read src/credentials.test.ts: its isolation checks constrain imports and do not prohibit forgeCredentials from running before the gate. No runtime test was run; the call ordering is explicit in the entry point.

The unconditional forgeCredentials call before runWrapper establishes the wording mismatch. A different deployed entry point could change the operational result, but action.yml selects the bundle generated from this entry point; it would not reconcile the two conflicting README descriptions.

medium — The security model copies counts of credential readers and internal imports

claim: 01M45PQCJ93QGRP4SQPB9CHW1R
anchor: README.md (snippet)

- The capability token is read in exactly one module with zero internal imports
  (`src/credentials.ts`) and flows only into the `Authorization` header of requests to
  `REVIEW_SERVICE_URL`.

lens: writing-quality · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M45PRH1M2T7W5PV2GPNJAF2G · valid: The exact-grounded security sentence copies counts of token-reading modules and internal imports. The reviewer identifies src/credentials.ts as the sole application reader and reports its node:process import and empty internal-import set, supplying both source sets and matching counts. These numbers describe inventory sizes, not a capacity argument, and naming the file does not exempt adjacent copied counts under the pointer exception. The proposed reference to reviewCredentials retains the token-flow constraint and points to the implementation where the reviewer says the isolation requirement remains. Remove the counts and use that pointer. With no present mismatch reported, medium is appropriate.
disposition: none

This sentence makes the README another place maintainers must update when the credential module’s structure changes. The counts agree with the application source today, but the inventory can become stale independently of the module’s actual imports and token acquisition.

The copy says “exactly one module with zero internal imports”. In the unbundled application source, the module reading the capability-token environment variable is src/credentials.ts, so the source’s reader set is {src/credentials.ts}, matching the count of one. That module imports node:process and has no repository-internal imports; its internal-import set is empty, matching the count of zero. Neither count currently disagrees with the source.

Replace the anchored sentence with “The capability token acquired by reviewCredentials() in src/credentials.ts flows only into the Authorization header of requests to REVIEW_SERVICE_URL.” The token-flow constraint stays explicit. The module’s isolation requirement remains beside its implementation in src/credentials.ts, which this replacement names; the README no longer copies its dependency inventory. This applies the restated-sets rule and the writing skill’s preference for authoritative lookups over copied facts.

I read src/credentials.ts, src/main.ts, src/review/session.ts, and src/credentials.test.ts, and searched application TypeScript with rg for REVIEW_CAPABILITY_TOKEN and import statements, excluding test files. reviewCredentials is the environment read and main passes its result to createReviewSession; the isolation test checks built-in-only imports. This is a static trace; I did not run the tests.

The matching counts establish the duplicate inventories. A requirement for this README audience to consume dependency counts without opening the named module would refute the pointer-based correction; no such audience restriction is stated.

low — Edited composition paragraph retains internal wiring prose in the README

claim: 01M45PMCC4EPZNY064T5AD50YF
anchor: README.md (snippet)

`src/main.ts` is the only place the modules meet. It reads the forge credentials and
`GITHUB_REPOSITORY`, builds the forge client, derives the run's inputs from the event
payload, and hands the orchestrator the collaborators that `WrapperDeps` in
`src/contract/types.ts` names. The orchestrator's exit code becomes the process exit
code, so the job's conclusion follows the outcome's exit code.

lens: project-docs · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M45PRH1M2T7W5PV2GPNJAF2G · valid: The exact-grounded paragraph narrates private startup wiring rather than defining a term or explaining a decision. The reviewer explicitly reports that the diff edits this paragraph and supplies the corresponding main.ts call sequence and WrapperDeps contract. Project-docs Division of labor and Consolidating stray documentation require deleting implementation narration from a touched stray passage; the README onboarding exception does not cover this private composition account. Keep any useful observable job-conclusion explanation. No contrary repository convention is evidenced. This is a low wrong-home defect.
disposition: none

Maintainers must keep a second prose account of internal startup wiring synchronized with the executable entry point. Replacing the collaborator list with a type reference still leaves the edited paragraph describing which module reads credentials, constructs the client, derives inputs, calls the orchestrator, and assigns the exit code.

The diff edits the Composition paragraph anchored here, including its description of handing collaborators to the orchestrator. In src/main.ts the same sequence is directly expressed by forgeCredentials(), createForgeClient(), readWrapperInputs(forge), and process.exitCode = await runWrapper({. WrapperDeps in src/contract/types.ts defines the dependency contract. This is an account of how the implementation works, rather than a term definition or a decision rationale.

Apply project-docs' Division of labor and Consolidating stray documentation rules: delete the internal wiring narration from this touched passage and let src/main.ts and src/contract/types.ts remain its source of truth. Any concise statement explaining the action's observable job conclusion can remain as human onboarding; no migration to a glossary or ADR is needed for the implementation account.

I read the complete README.md, the served diff, AGENTS.md and CLAUDE.md for documentation conventions, and the relevant entry point, credentials, input reader, dependency interface, and runWrapper implementation. The README exception permits human installation and use documentation, but this paragraph describes private module composition; it is neither a procedure, a generated reference, nor a recorded trade-off. I found no repository convention assigning internal implementation prose to the README. This is a static documentation-placement finding, not a runtime failure or a claim that the described wiring is inaccurate.

An explicit repository convention retaining this kind of internal composition reference, or evidence that this passage is required for installing or using the action, would refute the placement finding. The inspected records provide neither.

Other claims

grounding-pending (0)
ungrounded (0)
rejected (0)
duplicate-of (0)
unadjudicated (0)

Coverage

Coverage pass: 01M45PCTFWRATQ0C95YXRWAM8Z
Accounting: complete
Slot health: healthy

lens	part	arm	unit status	runs	loss
general-bug	whole	default	no-claims	1	no
writing-quality	whole	default	claims-emitted	1	no
test-trimming	whole	default	no-claims	1	no
restated-sets	whole	default	no-claims	1	no
project-docs	whole	default	claims-emitted	1	no

**Review** `01M45PCTE20F1BP8DW7MH3EAHG` — head `bc6feebb4c323e5de12f497db1b545598f82263a` # Review — j4k-oss/review-wrapper @ f5714bee5980 Scope: diff against base tree `211444be4c6d` Status: dispatched — coverage complete (5/5 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (5) ### medium — The action-input table duplicates the input manifest - claim: `01M45PKHG05A01JTKWN246515H` - anchor: `README.md` (snippet) ``` ## Action inputs | Input | Meaning | | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `expected-service-origin` | Origin `REVIEW_SERVICE_URL` must match exactly, e.g. `https://review.j4k.dev`. Left unset — or set to the empty string an undefined `${{ vars.X }}` expands to — the pin is off and only the `https:` scheme is enforced. | ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M45PRH1M2T7W5PV2GPNJAF2G` · valid: The exact-grounded README table lists expected-service-origin and its behavior. The reviewer names action.yml as the runner-read source, reports that its input set contains the same sole member, and accounts for the origin-match and empty-value guidance there. This meets the restated-sets comparison requirement; agreement today leaves a separately maintained copy that can drift. Point to action.yml inputs, retaining distinct security rationale. Earlier matching claims and their rationales supply no concrete refutation; their valid verdicts are not the basis of this judgment. - disposition: none > The README creates a second inventory and description of action inputs that maintainers must update whenever the manifest changes. The copy agrees today, but an input change can leave users following an obsolete contract without any warning. > > The source is the inputs mapping in action.yml, which the Actions runner reads. Its members are expected-service-origin; the README table covers expected-service-origin. No member appears in only one list. The table also repeats the manifest description of origin matching and the unset or empty input behavior. > > Replace this section’s table with “Action inputs and their descriptions are defined in action.yml.” This follows the writing skill’s One Idea, One Place guidance and the restated-sets rule: it gives callers the authoritative contract while retaining the explanation in the manifest they can open. > > I read the complete README, action.yml, and reviewCredentials in src/credentials.ts. The function confirms the documented nonempty origin check and empty-input behavior; this is a static documentation comparison, not a runtime reproduction. > > The matching inventory establishes duplication. A separate intended audience unable to open action.yml would refute the source-pointer correction, but the README is distributed beside that manifest and establishes no such restriction. ### medium — The recovery table copies the diagnostic formatter’s message inventory - claim: `01M45PM88CFCMMRJB44VRSFMX0` - anchor: `README.md` (snippet) ``` Before retrying, act on the reported cause: | Message | Recovery | | --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | | `provider session limit reached` | Wait for the session limit to reset or ask the operator to lift it. | | `selected model at capacity` | Wait for the model to accept new work. | | `agent execution failed` | Ask the operator to inspect the failed agent logs and correct the reported error. | | `sandbox infrastructure failed` | Ask the operator to check sandbox provisioning, controller connectivity, and resources. | | `execution failed for an unrecognized reason` | Copy the quoted code from `(code "…")` on that log line and ask the operator to look it up in the review service logs. | ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M45PRH1M2T7W5PV2GPNJAF2G` · valid: The exact-grounded table enumerates failure messages and recoveries. The reviewer supplies the four selected codes plus fallback in src/wrapper/remedies.ts, their corresponding display labels in committed dist/index.mjs, and reports that the job diagnostic emits the matching recovery. No case differs today, so this is a medium copied-set defect. The unavailable standalone j4k/review source does not leave an essential comparison missing: the reported shipped bundle provides the labels, and the local map defines the selected remedies. Point to the diagnostic and formatExecutionRecovery, preserving the subsequent retry distinction. Earlier accounts lacking dependency strings do not refute this current comparison. - disposition: none > Maintainers must keep this table synchronized with executable recovery diagnostics even though the job already prints the recovery beside the cause. The copy agrees at this revision, but changing a supported failure code or its remedy can silently leave README readers following an obsolete recovery path. > > In src/wrapper/remedies.ts, executionRemedies defines provider-session-limit, provider-capacity, agent-execution-failed, and infrastructure-failure, and formatExecutionRecovery handles the unrecognized-code fallback. The committed dist/index.mjs contains formatExecutionFailure, whose corresponding labels are “provider session limit reached”, “selected model at capacity”, “agent execution failed”, and “sandbox infrastructure failed”, with “execution failed for an unrecognized reason” as fallback. The README covers these same labels and fallback; no member appears in only one inventory. Its recovery instructions also repeat executionRemedies and formatExecutionRecovery. > > Replace the anchored passage with “Before retrying, follow the recovery printed beside the cause in the job diagnostic; formatExecutionRecovery in src/wrapper/remedies.ts defines it.” Keep the following paragraph’s distinction between rerunning failed triage and using the printed dispatch command for failed lens slots. This preserves the operational instruction, names its authoritative home, and applies the writing skill’s One Idea, One Place guidance without copying the supported cases. > > I read the complete README, src/wrapper/remedies.ts, src/wrapper/orchestrator.ts, and the bundled formatExecutionFailure and formatExecutionRecovery in dist/index.mjs. The formatter imports its message mapping from the owner’s j4k/review repository; that repository itself was unavailable, but its shipped implementation is present in the reviewed bundle. This is a static source comparison, not a runtime reproduction. > > The matching formatter cases establish the duplicate inventory. Evidence that these users cannot read either the job diagnostic or the named source would refute the proposed replacement; the README currently directs them to that diagnostic for the dispatch command. ### medium — The fork-gate explanation wrongly promises that no credential function runs before it - claim: `01M45PMWXHPY2VRZ1SRGRVV283` - anchor: `README.md` (snippet) ``` - Forgejo does pass secrets to fork `pull_request_target` runs, so the real boundary is code-path ordering: the fork gate is evaluated from the event payload **before** the credentials module is ever called. On the fork path the capability token environment variable is never read — enforced by a static isolation test and an orchestrator unit test. ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M45PRH1M2T7W5PV2GPNJAF2G` · valid: The exact-grounded sentence promises that the fork gate precedes any call to the credentials module. The concrete reported trace instead has main.ts call forgeCredentials from that module before readWrapperInputs and runWrapper, while reviewCredentials is deferred through openSession until after the fork branch. The composition claim also grounds the entry point's credential-read role. The static import-isolation check does not prohibit the earlier forgeCredentials call, so it cannot rescue the broad wording. Specify reviewCredentials while preserving the capability-token isolation guarantee. Earlier matching claims contain no concrete refutation; this is a documentation precision defect, not evidence that the capability token leaks. - disposition: none > A security reviewer can infer that fork runs avoid reading the forge task token as well as the service capability token, or treat the entry point’s credential read as a violation of the documented boundary. The passage says the gate runs “before the credentials module is ever called”, although that module supplies forge credentials before the gate on every run. The README’s Environment contract now explicitly describes that ordering, so its security explanation contradicts its setup contract. > > src/main.ts calls forgeCredentials() while constructing forge, then awaits readWrapperInputs(forge), then invokes runWrapper. In src/credentials.ts, forgeCredentials reads the forge task token and API base. runWrapper in src/wrapper/orchestrator.ts evaluates deps.inputs.isFork before review calls deps.openSession; only that thunk calls reviewCredentials(). Thus the supported security property concerns the service credential function, not the entire module. > > Replace “before the credentials module is ever called” with “before reviewCredentials() is called”. Keep the following sentence about the capability token never being read on the fork path. This preserves the intended boundary while applying the writing skill’s Use Precise Language guidance and preventing the broader, false interpretation. > > I traced the complete src/main.ts, src/credentials.ts, readWrapperInputs in src/wrapper/event-context.ts, and runWrapper and review in src/wrapper/orchestrator.ts. I also read src/credentials.test.ts: its isolation checks constrain imports and do not prohibit forgeCredentials from running before the gate. No runtime test was run; the call ordering is explicit in the entry point. > > The unconditional forgeCredentials call before runWrapper establishes the wording mismatch. A different deployed entry point could change the operational result, but action.yml selects the bundle generated from this entry point; it would not reconcile the two conflicting README descriptions. ### medium — The security model copies counts of credential readers and internal imports - claim: `01M45PQCJ93QGRP4SQPB9CHW1R` - anchor: `README.md` (snippet) ``` - The capability token is read in exactly one module with zero internal imports (`src/credentials.ts`) and flows only into the `Authorization` header of requests to `REVIEW_SERVICE_URL`. ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M45PRH1M2T7W5PV2GPNJAF2G` · valid: The exact-grounded security sentence copies counts of token-reading modules and internal imports. The reviewer identifies src/credentials.ts as the sole application reader and reports its node:process import and empty internal-import set, supplying both source sets and matching counts. These numbers describe inventory sizes, not a capacity argument, and naming the file does not exempt adjacent copied counts under the pointer exception. The proposed reference to reviewCredentials retains the token-flow constraint and points to the implementation where the reviewer says the isolation requirement remains. Remove the counts and use that pointer. With no present mismatch reported, medium is appropriate. - disposition: none > This sentence makes the README another place maintainers must update when the credential module’s structure changes. The counts agree with the application source today, but the inventory can become stale independently of the module’s actual imports and token acquisition. > > The copy says “exactly one module with zero internal imports”. In the unbundled application source, the module reading the capability-token environment variable is src/credentials.ts, so the source’s reader set is {src/credentials.ts}, matching the count of one. That module imports node:process and has no repository-internal imports; its internal-import set is empty, matching the count of zero. Neither count currently disagrees with the source. > > Replace the anchored sentence with “The capability token acquired by reviewCredentials() in src/credentials.ts flows only into the Authorization header of requests to REVIEW_SERVICE_URL.” The token-flow constraint stays explicit. The module’s isolation requirement remains beside its implementation in src/credentials.ts, which this replacement names; the README no longer copies its dependency inventory. This applies the restated-sets rule and the writing skill’s preference for authoritative lookups over copied facts. > > I read src/credentials.ts, src/main.ts, src/review/session.ts, and src/credentials.test.ts, and searched application TypeScript with rg for REVIEW_CAPABILITY_TOKEN and import statements, excluding test files. reviewCredentials is the environment read and main passes its result to createReviewSession; the isolation test checks built-in-only imports. This is a static trace; I did not run the tests. > > The matching counts establish the duplicate inventories. A requirement for this README audience to consume dependency counts without opening the named module would refute the pointer-based correction; no such audience restriction is stated. ### low — Edited composition paragraph retains internal wiring prose in the README - claim: `01M45PMCC4EPZNY064T5AD50YF` - anchor: `README.md` (snippet) ``` `src/main.ts` is the only place the modules meet. It reads the forge credentials and `GITHUB_REPOSITORY`, builds the forge client, derives the run's inputs from the event payload, and hands the orchestrator the collaborators that `WrapperDeps` in `src/contract/types.ts` names. The orchestrator's exit code becomes the process exit code, so the job's conclusion follows the outcome's exit code. ``` - lens: project-docs · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M45PRH1M2T7W5PV2GPNJAF2G` · valid: The exact-grounded paragraph narrates private startup wiring rather than defining a term or explaining a decision. The reviewer explicitly reports that the diff edits this paragraph and supplies the corresponding main.ts call sequence and WrapperDeps contract. Project-docs Division of labor and Consolidating stray documentation require deleting implementation narration from a touched stray passage; the README onboarding exception does not cover this private composition account. Keep any useful observable job-conclusion explanation. No contrary repository convention is evidenced. This is a low wrong-home defect. - disposition: none > Maintainers must keep a second prose account of internal startup wiring synchronized with the executable entry point. Replacing the collaborator list with a type reference still leaves the edited paragraph describing which module reads credentials, constructs the client, derives inputs, calls the orchestrator, and assigns the exit code. > > The diff edits the Composition paragraph anchored here, including its description of handing collaborators to the orchestrator. In src/main.ts the same sequence is directly expressed by forgeCredentials(), createForgeClient(), readWrapperInputs(forge), and `process.exitCode = await runWrapper({`. WrapperDeps in src/contract/types.ts defines the dependency contract. This is an account of how the implementation works, rather than a term definition or a decision rationale. > > Apply project-docs' Division of labor and Consolidating stray documentation rules: delete the internal wiring narration from this touched passage and let src/main.ts and src/contract/types.ts remain its source of truth. Any concise statement explaining the action's observable job conclusion can remain as human onboarding; no migration to a glossary or ADR is needed for the implementation account. > > I read the complete README.md, the served diff, AGENTS.md and CLAUDE.md for documentation conventions, and the relevant entry point, credentials, input reader, dependency interface, and runWrapper implementation. The README exception permits human installation and use documentation, but this paragraph describes private module composition; it is neither a procedure, a generated reference, nor a recorded trade-off. I found no repository convention assigning internal implementation prose to the README. This is a static documentation-placement finding, not a runtime failure or a claim that the described wiring is inaccurate. > > An explicit repository convention retaining this kind of internal composition reference, or evidence that this passage is required for installing or using the action, would refute the placement finding. The inspected records provide neither. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M45PCTFWRATQ0C95YXRWAM8Z Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | project-docs | whole | default | claims-emitted | 1 | no |

🚀 1

jercik referenced this pull request

2026-10-04 21:53:05 +00:00

docs: say the fork gate guards only the review credentials #33

jercik referenced this pull request

2026-10-04 21:53:13 +00:00

docs: point the README at action.yml and the logged remedy #34

jercik referenced this pull request

2026-10-04 21:53:14 +00:00

docs: point the README intro at the service's HTTP contract #35

jercik commented

2026-10-04 21:57:02 +00:00

Replying to comment #122860

All four findings are summary-only, so I'm answering them here. Each one is about README text this PR doesn't change, so each fix targets main on its own.

01M44DH0VFHYT6MWYWH96G8Z8Y ("Security model says the fork gate precedes all credential reads") is valid. src/main.ts calls forgeCredentials(), which reads FORGEJO_TOKEN and GITHUB_API_URL, before the fork gate. Only reviewCredentials() waits behind it. Tracked in #33, which says the gate guards only the review credentials and describes what the two tests actually check. It also covers the duplicate 01M44DPZPHFRSW7SEY35PBGPJ6.
01M44DMTYMV2XX9RFN5A2MCZCP (action input table) and 01M44DNK5MWG0DWZ49QPQ50Q54 (recovery table) are valid. Tracked in #34, which points at inputs in action.yml and at the remedy formatExecutionRecovery prints on the failure log line.
01M44DRX5S6FCDZFDHJHVECRGJ ("The introduction narrates where the HTTP contract was decided") is valid. Tracked in #35, which points the introduction at section 8 of the service's docs/SPEC.md. It also covers the duplicate 01M44DTFZKA23QS4RKFKFE5E98.

> Replying to comment #122860 All four findings are summary-only, so I'm answering them here. Each one is about README text this PR doesn't change, so each fix targets `main` on its own. - `01M44DH0VFHYT6MWYWH96G8Z8Y` ("Security model says the fork gate precedes all credential reads") is valid. `src/main.ts` calls `forgeCredentials()`, which reads `FORGEJO_TOKEN` and `GITHUB_API_URL`, before the fork gate. Only `reviewCredentials()` waits behind it. Tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/33, which says the gate guards only the review credentials and describes what the two tests actually check. It also covers the duplicate `01M44DPZPHFRSW7SEY35PBGPJ6`. - `01M44DMTYMV2XX9RFN5A2MCZCP` (action input table) and `01M44DNK5MWG0DWZ49QPQ50Q54` (recovery table) are valid. Tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/34, which points at `inputs` in `action.yml` and at the remedy `formatExecutionRecovery` prints on the failure log line. - `01M44DRX5S6FCDZFDHJHVECRGJ` ("The introduction narrates where the HTTP contract was decided") is valid. Tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/35, which points the introduction at section 8 of the service's `docs/SPEC.md`. It also covers the duplicate `01M44DTFZKA23QS4RKFKFE5E98`.

jercik referenced this pull request

2026-10-04 21:57:08 +00:00

docs: point to the outcome set instead of restating it #30

jercik referenced this pull request

2026-10-04 21:59:59 +00:00

docs: point to the outcome set instead of restating it #30

jercik referenced this pull request

2026-10-04 22:11:10 +00:00

docs: point the README at action.yml and the logged remedy #34

jercik referenced this pull request

2026-10-04 22:13:44 +00:00

docs: say the fork gate guards only the review credentials #33

jercik referenced this pull request

2026-10-04 22:31:08 +00:00

docs: drop the pass-bound operation list from the README #36

jercik referenced this pull request

2026-10-04 22:49:16 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik referenced this pull request

2026-10-04 22:50:31 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik referenced this pull request

2026-10-04 23:04:10 +00:00

docs: say the fork gate guards only the review credentials #33

jercik referenced this pull request

2026-10-04 23:16:43 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik referenced this pull request

2026-10-04 23:16:48 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik referenced this pull request

2026-10-04 23:28:06 +00:00

docs: reduce the fork-gate bullet to its guarantee #39

jercik referenced this pull request

2026-10-04 23:40:22 +00:00

docs: say which reads a superseded run keeps on its pass #38

jercik referenced this pull request

2026-10-05 00:13:47 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik referenced this pull request

2026-10-05 00:18:17 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik referenced this pull request

2026-10-05 01:07:10 +00:00

docs: say the fork gate guards only the review credentials #33

jercik referenced this pull request

2026-10-05 01:07:11 +00:00

docs: correct the service URL and PR-input rows of the environment table #37

jercik changed target branch from docs/outcome-set-pointers to main

2026-10-05 09:30:16 +00:00

jercik force-pushed docs/readme-set-pointers from ab235f39c4

commit-msg / commitlint (pull_request) Successful in 23s

Details

Checks / quality-checks (pull_request) Successful in 47s

Details

Review / Review (pull_request_target) Has been cancelled

Details

to bc6feebb4c

commit-msg / commitlint (pull_request) Successful in 22s

Details

Checks / quality-checks (pull_request) Successful in 46s

Details

Review / Review (pull_request_target) Successful in 10m27s

Details

2026-10-05 09:30:20 +00:00

Compare

jercik merged commit 94aa446758 into main

2026-10-05 09:31:21 +00:00

jercik referenced this pull request from a commit

2026-10-05 09:31:21 +00:00

docs: point the README at the code for required variables and collaborators (#32)

forgejo-actions reviewed

2026-10-05 09:40:47 +00:00

README.md

Lines 78-82

					
				@ -150,5 +150,5 @@

				`src/main.ts` is the only place the modules meet. It reads the forge credentials and

				`GITHUB_REPOSITORY`, builds the forge client, derives the run's inputs from the event

				payload, and hands the orchestrator its collaborators: the wait protocol, the clock, and

				the three reconcilers (summary, inline, dispositions). The orchestrator's exit code

				becomes the process exit code, so the job's conclusion follows the outcome's exit code.

				payload, and hands the orchestrator the collaborators that `WrapperDeps` in

				`src/contract/types.ts` names. The orchestrator's exit code becomes the process exit

				code, so the job's conclusion follows the outcome's exit code.

low — Edited composition paragraph retains internal wiring prose in the README

Maintainers must keep a second prose account of internal startup wiring synchronized with the executable entry point. Replacing the collaborator list with a type reference still leaves the edited paragraph describing which module reads credentials, constructs the client, derives inputs, calls the orchestrator, and assigns the exit code.

The diff edits the Composition paragraph anchored here, including its description of handing collaborators to the orchestrator. In src/main.ts the same sequence is directly expressed by forgeCredentials(), createForgeClient(), readWrapperInputs(forge), and process.exitCode = await runWrapper({. WrapperDeps in src/contract/types.ts defines the dependency contract. This is an account of how the implementation works, rather than a term definition or a decision rationale.

Apply project-docs' Division of labor and Consolidating stray documentation rules: delete the internal wiring narration from this touched passage and let src/main.ts and src/contract/types.ts remain its source of truth. Any concise statement explaining the action's observable job conclusion can remain as human onboarding; no migration to a glossary or ADR is needed for the implementation account.

I read the complete README.md, the served diff, AGENTS.md and CLAUDE.md for documentation conventions, and the relevant entry point, credentials, input reader, dependency interface, and runWrapper implementation. The README exception permits human installation and use documentation, but this paragraph describes private module composition; it is neither a procedure, a generated reference, nor a recorded trade-off. I found no repository convention assigning internal implementation prose to the README. This is a static documentation-placement finding, not a runtime failure or a claim that the described wiring is inaccurate.

An explicit repository convention retaining this kind of internal composition reference, or evidence that this passage is required for installing or using the action, would refute the placement finding. The inspected records provide neither.

lens project-docs · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45PMCC4EPZNY064T5AD50YF of review 01M45PCTE20F1BP8DW7MH3EAHG

**low** — Edited composition paragraph retains internal wiring prose in the README > Maintainers must keep a second prose account of internal startup wiring synchronized with the executable entry point. Replacing the collaborator list with a type reference still leaves the edited paragraph describing which module reads credentials, constructs the client, derives inputs, calls the orchestrator, and assigns the exit code. > > The diff edits the Composition paragraph anchored here, including its description of handing collaborators to the orchestrator. In src/main.ts the same sequence is directly expressed by forgeCredentials(), createForgeClient(), readWrapperInputs(forge), and `process.exitCode = await runWrapper({`. WrapperDeps in src/contract/types.ts defines the dependency contract. This is an account of how the implementation works, rather than a term definition or a decision rationale. > > Apply project-docs' Division of labor and Consolidating stray documentation rules: delete the internal wiring narration from this touched passage and let src/main.ts and src/contract/types.ts remain its source of truth. Any concise statement explaining the action's observable job conclusion can remain as human onboarding; no migration to a glossary or ADR is needed for the implementation account. > > I read the complete README.md, the served diff, AGENTS.md and CLAUDE.md for documentation conventions, and the relevant entry point, credentials, input reader, dependency interface, and runWrapper implementation. The README exception permits human installation and use documentation, but this paragraph describes private module composition; it is neither a procedure, a generated reference, nor a recorded trade-off. I found no repository convention assigning internal implementation prose to the README. This is a static documentation-placement finding, not a runtime failure or a claim that the described wiring is inaccurate. > > An explicit repository convention retaining this kind of internal composition reference, or evidence that this passage is required for installing or using the action, would refute the placement finding. The inspected records provide neither. lens `project-docs` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45PMCC4EPZNY064T5AD50YF` of review `01M45PCTE20F1BP8DW7MH3EAHG`

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

j4k-oss/review-wrapper!32

Rows
Columns