refactor(flows)!: one owner per refusal, read, and write; one wording for both faces

[Vivswan]

Before

$ gsac check --repository o/r --settings-file s.yml        # no token anywhere
error: cannot call the GitHub API: no token was provided. Pass --token, or export GITHUB_TOKEN

The action printed a different line for the same problem (Set the "token" input on the action step (or export GITHUB_TOKEN)), and the command line refused private-report: artifact in a pre-check of its own while the flows re-checked for the uploader.

After

$ gsac check --repository o/r --settings-file s.yml
error: cannot call the GitHub API: no token was provided. Set the "token" input (--token on the command line), or export GITHUB_TOKEN

refusal   private-report: artifact   parseConfig(read, env, { artifactUpload })  -> input-artifact-unsupported
read      repos-dir file             readSettingsFile(path, "central-file")
write     snapshot, merged, init     writeReplacing(path, text): Result<void, string>   (own staging file + rename)
select    "no allowlist"             SectionSelection.ALL everywhere
init      --settings-file            parseSnapshotFileConfig(read, env, "settings-file")
crash     gsac stopped unexpectedly  cliIo(...).io.annotate("error", ...)

What changed

Every refusal, read, and write has one owner, and both faces print one wording per problem.

• Refusals: parseConfig takes RunCapabilities and owns the artifact-channel refusal; the CLI pre-check and the flows' uploader check are gone.
• Prose: the shared renderer is face-neutral, so the CLI's rewording and the describe hook are gone; executeRun returns RunEnd { exitCode, fatal? }.
• Read: the repos-dir file goes through readSettingsFile under a central-file role.
• Write: one writeReplacing for the snapshot, the merged file, and init. It stages under a short name of its own beside the destination (.gsac-<pid>-<hex>.tmp, opened wx), keeps an existing file's mode, and never unlinks a user's file; the dir-form snapshot claims each file once, so two targets folded onto one file by an operator link fail the second.
• Selection and init: SectionSelection replaces every bare ReadonlySet; init reads its destination through the library.
• Crash line: through cliIo, like every other error line.

Proof

bun run check green (3468 unit tests); bun run test:e2e 260/260; build:check zero drift; build:lib + package smoke ok. Accounting against main (git diff origin/main...HEAD --numstat; source = src/ minus *.test.ts, mock.ts, generators.ts, scenarios): source +482 -302 (net +180), tests +546 -185 (net +361), docs +40 -9 (net +31). Source grew because the one writer took over staging ownership, the identity-based layer guard, and the aliased-target claim, replacing three in-place writers and the pre-existing last-writer-wins.

Technical details

Based on main eac7de6 (#206, #207, #203 merged). Upgrade entries: docs/upgrading/v2-to-v3.md sections 22 (wording) and 23 (library). Ten commits: the PR commit, the docs commit carried over from #206's branch (the teams playbook and upgrade-page wording), the writer's unique staging name, the lead's gate folds, the redacted-partial line, two codex fold rounds on the writer and the claim, the fold of the second Copilot round (four threads) with its own codex rounds, and the identity-based merge guard (the third Copilot round's one thread).

Decisions

Item	Decision
Input refusals	parseConfig(read, env, capabilities) owns the artifact refusal (input-artifact-unsupported); the CLI pre-check, requireUploader, and artifact-uploader-missing are deleted. openReportChannel keeps a BUG throw for a face that declares the capability but hands no uploader
Remedy prose and describe	The shared prose is face-neutral, so describeCliProblem is deleted; with no face wording left, the describe hook goes too and executeRun returns RunEnd (the CLI's --json envelope reads the fatal problem from it). Threading describe through multi's per-target path therefore has nothing to thread; describeProblem stays
Central read	SettingsFileRole gains central-file; a YAML syntax error in a repos-dir file now reads through the same role instead of a separate cannot parse line
File writes	writeReplacing in src/flows/settings-write.ts opens .gsac-<pid>-<8 hex>.tmp in the destination's directory (spelled as the caller spelled it, so a link/.. segment resolves the same way for both names and the rename never crosses a filesystem; a fixed short name, so a destination leaf at NAME_MAX still writes) with wx, owns it from that open (only a file it created is removed on failure), copies an existing regular destination's permission bits, writes, closes, renames. Ownership is not carried (needs privileges). A destination that is a symlink is replaced by the rename (the link, never its referent)
Merge guard	Entries compared by identity (dev:ino), never by name: renameEntry(dest) is the entry the rename replaces (the leaf under its OS-resolved parent, a link at the leaf unfollowed; null for a new entry), readEntries(layer) is every entry the read follows (each component, every link hop of a chain, the final file; a 64-hop budget bounds cycles). Refuse when a layer's set holds the replaced entry. out.yml -> layer.yml as the destination is admitted (the link is replaced, the layer stands); alias.yml -> repo.yml as a layer with repo.yml as the destination, a chain, a directory link the layer is read through, a case alias of the layer or of its link, and a layer that IS the destination link are refused. Scope, in the writer's header: an input layer is never the destination, under any name the read follows or the rename reaches; deliberate evasion (hardlinks, mounts, races) is out of scope
Landing names	landingNames(path) in settings-write.ts: the rename target (the directory as the filesystem names it plus the leaf as spelled) and, unless the leaf is a link, the referent as the filesystem names it now (on a case-insensitive filesystem, an existing file's own spelling). The dir-form snapshot claims each rename target and refuses a later target whose landing names hit a claim, in either target order; the refusal names the earlier target through the redaction plan's display, never the landing path
Redacted partial	emitRedactedResult switches exhaustively over RunOutcome (a never default); partial prints <display>: partial - <skipped> (<code>). <details hidden> where it printed nothing
Selection	SectionSelection replaces every bare ReadonlySet<SectionKey>
init's read	parseSnapshotFileConfig(read, env, "settings-file") refuses a list separator with input-settings-file-is-list (mode: "init", its own wording); init's LIST_SEPARATOR and init-settings-file-is-list are deleted
Public entry	RunCapabilities and RunEnd are public (in parseConfig's and executeRun's signatures, with library-page rows); writeReplacing and snapshotFileDestination are the CLI's and sit in src/internal.ts

Proof detail

• Owner pins: parseConfig refuses the artifact channel for { artifactUpload: false } and admits it for true; the CLI/action equivalence table drives both through it; executeRun's fatal problem comes back beside the exit code.
• Writer: the merge writes with a merged.yml.tmp layer present and leaves it intact; the snapshot leaves a user's snapshot.yml.tmp directory intact and no staging file behind; a rename failure removes the run's own staging file; a 0600 merged file stays 0600 under umask 022; a 255-character leaf writes; link/../out.yml stages beside the resolved destination (the test makes the lexical directory read-only, so a staging file placed there fails the run; the author ran the negative control with the join() spelling and the test failed).
• Guards: merge admits out.yml -> repo.yml, refuses a layer alias.yml -> repo.yml with repo.yml as the destination, the chain alias -> out -> repo with out.yml as the destination, a layer folder/repo.yml through folder -> . with folder as the destination, REPO.YML over repo.yml and ALIAS.YML over the layer link alias.yml on a case-insensitive filesystem (probe-gated), and a layer that is the destination link; snapshot refuses the second of two targets folded by an owner link (a leaf that was a link, both orders of a case alias), admits old/r after alice/r whose leaf linked to old/r.yml.
• Redacted partial: through runSingle with a private repository, labels denied 403 under warn: result partial, one line private repository #1: partial - labels (403). <note>, plus the unit case in test/flows/redact.test.ts.

Line accounting

Kind	+	-	Net	Why
Source	482	302	+180	Deletions: the CLI override and pre-check (commands.ts, init.ts), requireUploader (deliver.ts), the inline central read (multi.ts), three in-place writers (snapshot.ts, merge.ts). Growth: the shared writer with canonicalPath, renameTarget, landingNames, and the identity walk readEntries/renameEntry (settings-write.ts), the per-run claim (snapshot.ts), RunCapabilities and the destination reader (inputs.ts), RunEnd (execute.ts), the exhaustive result switch (redact.ts), the artifact prose (problem.ts)
Tests	546	185	+361	The writer's sibling, mode, name-length, link/.., and rename tests, the guard and claim tests (nine link and case shapes), the redacted-partial line, the artifact refusal, RunEnd, the central-file and init wordings; the rest are SectionSelection.ALL and wording updates on main's test fakes
Docs	40	9	+31	Upgrade sections 22 and 23, the library page's RunEnd/RunCapabilities/central-file rows, and the teams playbook and upgrade-page wording

• Small fixes on the way: emitRedactedResult's missing partial arm (a redacted private repository under warn with one denied section printed nothing); the writer's <file>.tmp unlink and umask mode drop, pre-existing in the snapshot writer, fixed for every caller at once.
• Recorded, not built: artifactUpload: true with no uploader reaches the BUG throw for a library caller (the capability should carry the uploader; a later PR); an init-specific settings.yml.tmp regression (all three callers share one writer); clearing fd before closeSync; on Windows only, a link\.. destination spelling (Windows collapses it lexically before following the link, the walk follows the link first) and a 64-bit NTFS file id folded to a 53-bit Number (two distinct files could read as one and refuse a safe destination), both under the header's out-of-scope clause, neither with a concrete input a CLI user hits today; a case alias where the first snapshot destination already exists in another on-disk case; the sections' own re-run the workflow strings (src/sections/**, src/main.ts) are another builder's.
• Codex rubber duck: 11 rounds. PR commit: round 1 one blocking (the shared writer unlinked a layer at <merged-file>.tmp), round 2 one blocking (the guard compared lexically) plus one literal, round 3 clean; one clean round on the replay onto refactor(library)!: a documented public entry and an internal one; one naming family per layer #205 and chore(test): one generator registry behind build:check, one Io capture fake, one ROOT, conventions stated #200; one clean round on the docs commit and the replay onto main 05e6d41. Writer and claim: round 1 three blocking (wx EEXIST removed another writer's file; a leaf link changed the claim key; the refusal named a possibly private earlier target through a public channel), round 2 two blocking (a case-aliased leaf; the ownership flag set after the write) plus one comment, round 3 clean. Second Copilot fold: round 4 two blocking (a case-aliased merge destination; staging under cwd for a link/.. destination), round 5 one blocking (a layer that is the destination link) plus a test that could not fail, round 6 one comment, then clean. Identity guard: one round, 2 Windows-only findings recorded (above), 0 POSIX findings; the reviewer traced the walk against OS resolution on macOS and confirmed the relative-cwd and link-target handling. The lead's gate review: 0 blocking, 4 non-blocking, all folded.
• Copilot: 12 threads; 11 fixed (the .tmp sibling unlink, the mode drop, NAME_MAX, the two false refusals on link destinations, the layer read through a link to the destination, the teams docs and upgrade-page wording), 1 rejected as recorded (the capability carrying the uploader); all resolved.
• Reviewer note (Copilot): executeRun's return type, parseConfig's third argument, and validateSettings's option type are the v3 library breaks listed in upgrade section 23; the sections' remedy strings are out of scope here.

BEGIN_COMMIT_OVERRIDE
refactor(flows)!: one owner per refusal, read, and write; one wording for both faces

BREAKING CHANGE: parseConfig(read, env, capabilities) owns the private-report: artifact refusal (input-artifact-unsupported); artifact-uploader-missing and the flows' uploader check are gone.
executeRun returns RunEnd { exitCode, fatal? } and RunDeps has no describe; failRun(io, problem) takes no wording hook.
validateSettingsDoc and validateSettings take a SectionSelection where they took a ReadonlySet<SectionKey>.
parseSnapshotFileConfig(read, env, "settings-file") reads the destination itself; SettingsFileRole gains central-file.
The action and the command line print one wording per problem; remedies name the input and its flag and say "re-run" without "the workflow".
END_COMMIT_OVERRIDE

[github-actions]

File size check

0 over a hard cap (fails), 13 warning(s).

File	Size	Tier	Cap
.github/scripts/release-pipeline.ts:6	156 chars	warn	150
.github/scripts/release-pipeline.ts:449	151 chars	warn	150
.github/scripts/release-pipeline.ts:1	30 comment lines (header)	warn	25
.github/scripts/release-pipeline.ts:445	14 comment lines	warn	10
.github/workflows/post-green.yml:27	153 chars	warn	150
.github/workflows/post-green.yml:140	11 comment lines	warn	10
.github/workflows/update-release.yml:67	164 chars	warn	150
src/flows/settings-write.ts:130	159 chars	warn	150
src/flows/settings-write.ts:29	11 comment lines	warn	10
src/flows/snapshot.ts:192	13 comment lines	warn	10
test/action/run.test.ts:759	151 chars	warn	150
test/action/run.test.ts:780	155 chars	warn	150
test/docs/guides.test.ts:469	155 chars	warn	150

Split the file, wrap the line, shorten or exempt the comment, or list the path in .file-size-allow.local with a # reason.

4 managed file(s) skipped; repo-platform owns them.

[Copilot]

🟡 Changes recommended

The shared writer can silently delete an existing user-owned .tmp sibling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Centralizes configuration refusal, file I/O, section selection, and error rendering across the action, CLI, and library.

Changes:

• Adds capability-aware parsing, RunEnd, and shared problem wording.
• Consolidates settings reads and atomic replacement writes.
• Standardizes SectionSelection usage and updates tests/docs.

File summaries

File	Description
test/sections/contract.test.ts	Updates retry wording.
test/report/issue-report.test.ts	Updates warnings and selection API.
test/report/delivery.test.ts	Updates artifact warning and selection API.
test/report/artifact-report.test.ts	Updates artifact warning expectations.
test/problem.test.ts	Covers unified problem messages.
test/github/graphql.test.ts	Updates transport retry wording.
test/github/api.test.ts	Updates API failure wording.
test/flows/single.test.ts	Removes flow-level uploader checks.
test/flows/merge.test.ts	Tests staging-path collision protection.
test/flows/library.test.ts	Updates library failure wording.
test/flows/inputs.test.ts	Supplies action capabilities.
test/flows/execute.test.ts	Tests RunEnd behavior.
test/flows/deliver.test.ts	Updates shared failure wording.
test/engine/secrets.test.ts	Uses SectionSelection.
test/engine/orchestrate.test.ts	Migrates validation selections.
test/engine/layers.test.ts	Uses all-section selection.
test/engine/check-purity.test.ts	Migrates fixture validation.
test/e2e/oracle.ts	Migrates oracle validation.
test/e2e/generators.ts	Migrates generator validation.
test/e2e/generators.test.ts	Migrates generator tests.
test/docs/settings-examples.ts	Migrates example validation.
test/docs/guides.test.ts	Migrates guide validation.
test/cli/program.test.ts	Updates unified CLI messages.
test/cli/inputs.test.ts	Tests capability-aware parsing.
test/cli/init.test.ts	Updates init refusal wording.
test/cli/execution.test.ts	Supplies action capabilities.
test/action/artifact.test.ts	Updates artifact retry wording.
src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts	Migrates section validation.
src/sections/actions_secrets/actions_secrets.test.ts	Migrates section validation.
src/report/issue-report.ts	Makes retry advice face-neutral.
src/report/delivery.ts	Replaces uploader refusal with invariant.
src/report/artifact-report.ts	Makes retry advice face-neutral.
src/problem.ts	Adds problems, roles, and unified wording.
src/internal.ts	Exports internal I/O helpers.
src/index.ts	Exports new public run types.
src/github/api.ts	Unifies API retry wording.
src/flows/snapshot.ts	Uses the shared replacement writer.
src/flows/single.ts	Removes duplicate uploader validation.
src/flows/settings-write.ts	Adds shared staged file replacement.
src/flows/multi.ts	Centralizes file reading and validation.
src/flows/merge.ts	Adds staged writes and collision checks.
src/flows/library.ts	Passes SectionSelection directly.
src/flows/layers.ts	Standardizes all-section validation.
src/flows/inputs.ts	Adds capabilities and init destination parsing.
src/flows/execute.ts	Returns structured run results.
src/flows/deliver.ts	Removes duplicate uploader checks and renderer hook.
src/engine/snapshot.ts	Uses all-section selection.
src/engine/orchestrate.ts	Accepts SectionSelection.
src/cli/program.ts	Uses shared parsing, rendering, and crash I/O.
src/cli/init.ts	Uses shared parsing and replacement writes.
src/cli/commands.ts	Removes CLI-specific problem rendering.
src/action/run.ts	Adapts to RunEnd.
src/action/inputs.ts	Declares artifact capability.
docs/upgrading/v2-to-v3.md	Documents new breaking changes.
docs/reference/library.md	Documents revised public APIs.
docs/playbooks/teams-not-collaborators.md	Clarifies undeclared team behavior.

Review details

• Files reviewed: 56/56 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Path alias handling and the shared writer introduce reproducible false refusals, filename-limit failures, and metadata regressions.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 59/59 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The public artifact path can still crash, replacement writes can widen permissions, and merge misclassifies a leaf symlink as an input collision.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/flows/settings-write.ts:65

• Replacing an existing file resets its permission bits to the process default. For example, with umask 022, gsac init --force on a mode-0600 settings file creates this staging file as 0644 and renames it over the protected file; repeated merge writes have the same regression, and settings can contain sensitive values. Create the staging file with the existing regular destination's mode (while retaining the documented symlink behavior), rather than always using openSync's default mode.

    fd = openSync(staging, "wx");

src/flows/merge.ts:30

• Following the destination's leaf symlink makes a safe merge fail as if it would overwrite a layer. If merged.yml is a symlink to the repo.yml layer, both calls return repo.yml and this emits merged-file-is-layer, although writeReplacing renames over the merged.yml link and leaves repo.yml untouched. Compute the destination identity using the canonical parent plus its leaf for symlinks, while retaining real-path comparison for regular files and case aliases, so this guard matches the writer's landing semantics.

  const landing = canonicalPath(cfg.mergedFile);
  const index = cfg.settingsFiles.findIndex((layer) => canonicalPath(layer) === landing);

• Files reviewed: 59/59 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The merge collision guard can overwrite a layer read through a final symlink.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 59/59 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The v2-to-v3 guide incorrectly states that snapshot writes were previously performed in place.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 59/59 changed files
• Comments generated: 1
• Review effort level: Balanced

[Vivswan]

Agreed: the old snapshot writer already staged through <path>.tmp and renamed; only merge and init wrote in place. The upgrade-table row is reworded in #211, which edits that table next (renumbering after this landing).