Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/little-fans-burn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
148 changes: 148 additions & 0 deletions .github/scripts/major-release-signoff.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,57 @@ extract_step() {
}

extract_step "Resolve PR context" > "$TMP/context.sh"
extract_step "Restore release tooling from the base branch" > "$TMP/restore-tooling.sh"
extract_step "Compute release preview" > "$TMP/preview.sh"
extract_step "Decide whether a signoff is required" > "$TMP/decision.sh"
extract_step "Regenerate and verify release contents" > "$TMP/verify.sh"

TOOLING_SOURCE="$TMP/tooling-source"
TOOLING_WORK="$TMP/tooling-work"
git init --quiet "$TOOLING_SOURCE"
git -C "$TOOLING_SOURCE" config commit.gpgsign false
git -C "$TOOLING_SOURCE" config user.name test
git -C "$TOOLING_SOURCE" config user.email test@example.com
mkdir -p "$TOOLING_SOURCE/scripts" "$TOOLING_SOURCE/.changeset" \
"$TOOLING_SOURCE/packages/ui" "$TOOLING_SOURCE/examples/demo" "$TOOLING_SOURCE/tools/config"
for file in scripts/preview-release.mjs package.json pnpm-lock.yaml \
pnpm-workspace.yaml .changeset/config.json .npmrc packages/ui/package.json \
examples/demo/package.json tools/config/package.json; do
printf 'main %s\n' "$file" > "$TOOLING_SOURCE/$file"
done
git -C "$TOOLING_SOURCE" add -A
git -C "$TOOLING_SOURCE" commit --quiet -m main
git -C "$TOOLING_SOURCE" branch -M main
git -C "$TOOLING_SOURCE" switch --quiet -c journey
mkdir -p "$TOOLING_SOURCE/packages/pr-only"
printf 'journey-only importer\n' > "$TOOLING_SOURCE/packages/pr-only/package.json"
for file in scripts/preview-release.mjs package.json pnpm-lock.yaml \
pnpm-workspace.yaml .changeset/config.json packages/ui/package.json; do
printf 'journey %s\n' "$file" > "$TOOLING_SOURCE/$file"
done
rm "$TOOLING_SOURCE/.npmrc"
printf 'journey pnpm hook\n' > "$TOOLING_SOURCE/.pnpmfile.cjs"
git -C "$TOOLING_SOURCE" add -A
git -C "$TOOLING_SOURCE" commit --quiet -m journey
TOOLING_HEAD=$(git -C "$TOOLING_SOURCE" rev-parse HEAD)
git -C "$TOOLING_SOURCE" switch --quiet main
git clone --quiet "$TOOLING_SOURCE" "$TOOLING_WORK"
git -C "$TOOLING_WORK" checkout --quiet "$TOOLING_HEAD"

if (cd "$TOOLING_WORK" && bash "$TMP/restore-tooling.sh") >/dev/null 2>&1 &&
[ "$(git -C "$TOOLING_WORK" rev-parse HEAD)" = "$TOOLING_HEAD" ] &&
[ "$(cat "$TOOLING_WORK/package.json")" = 'main package.json' ] &&
[ "$(cat "$TOOLING_WORK/pnpm-lock.yaml")" = 'main pnpm-lock.yaml' ] &&
[ "$(cat "$TOOLING_WORK/packages/ui/package.json")" = 'main packages/ui/package.json' ] &&
[ "$(cat "$TOOLING_WORK/.npmrc")" = 'main .npmrc' ] &&
[ ! -e "$TOOLING_WORK/packages/pr-only/package.json" ] &&
[ ! -e "$TOOLING_WORK/.pnpmfile.cjs" ]; then
pass "restores the complete release dependency graph from trusted main"
else
fail "restores the complete release dependency graph from trusted main" \
"expected main-owned workspace manifests and install inputs with PR-only importers removed"
fi

mkdir "$TMP/bin"
cat > "$TMP/bin/gh" <<'EOF'
#!/usr/bin/env bash
Expand Down Expand Up @@ -290,6 +338,106 @@ else
fail "generated release identity always stays out of the ordinary preview" "preview does not use generated release identity"
fi

PREVIEW_REMOTE="$TMP/preview-remote.git"
PREVIEW_REPO="$TMP/preview-repo"
git init --quiet --bare "$PREVIEW_REMOTE"
git init --quiet "$PREVIEW_REPO"
git -C "$PREVIEW_REPO" config commit.gpgsign false
git -C "$PREVIEW_REPO" config user.name test
git -C "$PREVIEW_REPO" config user.email test@example.com
git -C "$PREVIEW_REPO" remote add origin "$PREVIEW_REMOTE"
mkdir -p "$PREVIEW_REPO/scripts" "$PREVIEW_REPO/.changeset" \
"$PREVIEW_REPO/packages/core" "$PREVIEW_REPO/packages/hooks" "$PREVIEW_REPO/packages/ui"
cp "$ROOT/scripts/preview-release.mjs" "$PREVIEW_REPO/scripts/preview-release.mjs"
cp "$ROOT/.changeset/config.json" "$PREVIEW_REPO/.changeset/config.json"
cat > "$PREVIEW_REPO/package.json" <<'EOF'
{"name":"preview-fixture","private":true,"packageManager":"pnpm@11.10.0"}
EOF
cat > "$PREVIEW_REPO/pnpm-workspace.yaml" <<'EOF'
packages:
- "packages/*"
EOF
for package in core hooks ui; do
case "$package" in
core) name='@youversion/platform-core' ;;
hooks) name='@youversion/platform-react-hooks' ;;
ui) name='@youversion/platform-react-ui' ;;
esac
printf '{"name":"%s","version":"1.0.0"}\n' "$name" > "$PREVIEW_REPO/packages/$package/package.json"
done
git -C "$PREVIEW_REPO" add -A
git -C "$PREVIEW_REPO" commit --quiet -m main
git -C "$PREVIEW_REPO" branch -M main
git -C "$PREVIEW_REPO" push --quiet origin main

git -C "$PREVIEW_REPO" switch --quiet -c target
cat > "$PREVIEW_REPO/.changeset/target-major.md" <<'EOF'
---
"@youversion/platform-core": major
---

Major change inherited from the target branch.
EOF
git -C "$PREVIEW_REPO" add -A
git -C "$PREVIEW_REPO" commit --quiet -m 'target major'
git -C "$PREVIEW_REPO" push --quiet origin target
PREVIEW_BASE_SHA=$(git -C "$PREVIEW_REPO" rev-parse HEAD)

git -C "$PREVIEW_REPO" switch --quiet -c pr
printf 'non-major PR change\n' > "$PREVIEW_REPO/README.md"
git -C "$PREVIEW_REPO" add README.md
git -C "$PREVIEW_REPO" commit --quiet -m 'non-major PR change'
git -C "$PREVIEW_REPO" push --quiet origin pr
ln -s "$ROOT/node_modules" "$PREVIEW_REPO/node_modules"
PREVIEW_BIN="$TMP/preview-bin"
mkdir "$PREVIEW_BIN"
cat > "$PREVIEW_BIN/pnpm" <<EOF
#!/usr/bin/env bash
set -euo pipefail
[ "\$1" = exec ] && [ "\$2" = changeset ] && [ "\$3" = status ]
output=\${4#--output=}
printf '%s\n' \
'{"releases":[{"name":"@youversion/platform-core","type":"major","oldVersion":"1.0.0","newVersion":"2.0.0"}]}' \
> "\$output"
EOF
chmod +x "$PREVIEW_BIN/pnpm"

run_preview_case() {
local name="$1" expected_introduced="$2" expected_output="$3"
local output="$TMP/preview-output"
local error="$TMP/preview-error"
local preview="$PREVIEW_REPO/preview.json"
local head_sha
head_sha=$(git -C "$PREVIEW_REPO" rev-parse HEAD)
: > "$output"
rm -f "$preview"
if (cd "$PREVIEW_REPO" && \
PATH="$PREVIEW_BIN:$PATH" BASE_SHA="$PREVIEW_BASE_SHA" HEAD_SHA="$head_sha" \
GITHUB_OUTPUT="$output" \
bash "$TMP/preview.sh") >/dev/null 2> "$error" &&
jq -e ".introduced_major == $expected_introduced" "$preview" >/dev/null &&
grep -Fxq "is_major=$expected_output" "$output"; then
pass "$name"
else
fail "$name" \
"expected introduced_major=$expected_introduced and is_major=$expected_output; preview: $(cat "$preview" 2>/dev/null || true); output: $(tr '\n' ' ' < "$output"); error: $(tr '\n' ' ' < "$error")"
fi
}

run_preview_case "stacked PRs ignore a major inherited from their target branch" false 0

cat > "$PREVIEW_REPO/.changeset/pr-major.md" <<'EOF'
---
"@youversion/platform-react-ui": major
---

Major change introduced by the pull request.
EOF
git -C "$PREVIEW_REPO" add .changeset/pr-major.md
git -C "$PREVIEW_REPO" commit --quiet -m 'PR major'
git -C "$PREVIEW_REPO" push --quiet origin pr
run_preview_case "stacked PRs require signoff for a newly introduced major" true 1

if grep -Fq 'Generated release PR; major signoff is enforced on source PRs.' "$WORKFLOW"; then
pass "generated releases publish an explicit lifecycle-aware success"
else
Expand Down
29 changes: 22 additions & 7 deletions .github/workflows/major-release-signoff.yml
Original file line number Diff line number Diff line change
Expand Up @@ -203,20 +203,33 @@ jobs:

- name: Restore release tooling from the base branch
# The PR supplies changeset *data* only. The detector that reads it, the dependency
# set it runs under, and the changesets config all come from `main`, so a branch
# cannot rewrite the check that gates it. This runs before pnpm is set up, since
# `pnpm/action-setup` reads `packageManager` out of package.json.
# graph it runs under, and the changesets config all come from trusted `main`, so a
# branch cannot rewrite the check that gates it. This runs before pnpm is set up,
# since `pnpm/action-setup` reads `packageManager` out of package.json.
run: |
set -euo pipefail
git fetch --no-tags --force origin main:refs/heads/main
# All of main's tooling, or none of it. The detector, the dependency set it runs
# under, and the changesets config have to agree, so restoring some from main and
# taking the rest from the PR gives a detector without its own dependencies.
if git cat-file -e main:scripts/preview-release.mjs 2>/dev/null; then
# Frozen workspace installs validate every importer. Remove PR-only importers,
# then restore every manifest selected by main's workspace configuration so its
# lockfile and dependency graph remain internally consistent.
git ls-files |
grep -E '^(packages|examples|tools)/[^/]+/package[.]json$' |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Workspace pattern duplicates configuration

The cleanup and restoration passes hard-code the current one-level workspace layout instead of deriving it from the restored pnpm-workspace.yaml. When main adds a nested importer, another workspace root, or a broader glob, its manifest will not be restored consistently, increasing maintenance cost and allowing the frozen install to reject the main lockfile.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/major-release-signoff.yml
Line: 220

Comment:
**Workspace pattern duplicates configuration**

The cleanup and restoration passes hard-code the current one-level workspace layout instead of deriving it from the restored `pnpm-workspace.yaml`. When main adds a nested importer, another workspace root, or a broader glob, its manifest will not be restored consistently, increasing maintenance cost and allowing the frozen install to reject the main lockfile.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Cursor Fix in Codex

while IFS= read -r f; do
git cat-file -e "main:$f" 2>/dev/null || rm -f "$f"
done
for f in scripts/preview-release.mjs package.json pnpm-lock.yaml \
pnpm-workspace.yaml .changeset/config.json; do
git cat-file -e "main:$f" 2>/dev/null && git checkout main -- "$f"
done
git ls-tree -r --name-only main |
grep -E '^(packages|examples|tools)/[^/]+/package[.]json$' |
while IFS= read -r f; do
git checkout main -- "$f"
done
# Install hooks a PR could add to rewrite what we just restored. Take main's, or
# remove it outright: leaving a PR-authored one hands back the control.
for f in .npmrc .pnpmfile.cjs; do
Expand Down Expand Up @@ -248,17 +261,19 @@ jobs:
- name: Compute release preview
id: preview
env:
BASE_SHA: ${{ needs.context.outputs.base_sha }}
HEAD_SHA: ${{ needs.context.outputs.head_sha }}
run: |
set -euo pipefail
# `changeset status` resolves `baseBranch` (main) as a LOCAL ref and fails with
# "Failed to find where HEAD diverged from main" if it is missing. A PR checkout
# has only origin/main, so create the local branch too.
git fetch --no-tags --force origin main:refs/heads/main
# Merge-base, not the PR's base sha: on a synchronize event the base
# branch may have moved, and diffing against its tip would attribute
# someone else's changesets to this PR.
BASE=$(git merge-base main "$HEAD_SHA")
git fetch --no-tags --force origin "$BASE_SHA"
# Compare from the common ancestor of the immutable PR endpoints. This excludes
# major changesets inherited from a stacked PR's base without attributing later
# movement on that base branch to the PR being evaluated.
BASE=$(git merge-base "$BASE_SHA" "$HEAD_SHA")
node scripts/preview-release.mjs --base "$BASE" --head "$HEAD_SHA" > preview.json
cat preview.json
# `introduced_major`, not `is_major`: a major already pending on main
Expand Down
6 changes: 4 additions & 2 deletions packages/ui/src/components/bible-reader-controlled.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ describe('BibleReader controlled mode - pure projection', () => {
});

describe('BibleReader controlled mode - provable inertness', () => {
// The two rendered selection workflows can exceed Vitest's default 5s timeout
// when the unit and Storybook projects contend on a loaded CI runner.
it('never touches the network or localStorage for highlights, even across select/apply/clear', async () => {
const fetchSpy = vi.fn();
vi.stubGlobal('fetch', fetchSpy);
Expand Down Expand Up @@ -371,7 +373,7 @@ describe('BibleReader controlled mode - provable inertness', () => {
getItemSpy.mockRestore();
setItemSpy.mockRestore();
}
});
}, 20_000);

it('color taps paint nothing (no optimistic echo)', async () => {
const { container } = renderReader({ highlights: [], onHighlightApply: vi.fn() });
Expand Down Expand Up @@ -506,7 +508,7 @@ describe('BibleReader controlled mode - events', () => {
await waitFor(() => {
expect(screen.queryByRole('dialog')).toBeNull();
});
});
}, 20_000);

it('ignores onHighlightApply / onHighlightRemove in self-contained mode', async () => {
const onHighlightApply = vi.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,12 @@ export const StylingFocusDismissalAndRapidReopen: Story = {
},
};

export const DirectionOnlyInheritanceRejectsHostVisualValues: Story = {
export const ProviderDirectionRejectsHostVisualValues: Story = {
globals: { interfaceDirection: 'rtl' },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Setting the provider to RTL inside an LTR host makes this test prove which direction the SDK follows.

For Agents: independent direction inputs

The explicit interfaceDirection: 'rtl' and host dir="ltr" distinguish provider-owned direction from accidental host inheritance. The story checks an RTL trigger and heading while the wrapper stays LTR, including after hostile styles are applied. All eight stories in this file passed in the local Chromium run.

Written by Code Reviewer bot on behalf of Cam.

render: () => (
<div
data-testid="hostile-inheritance-container"
dir="rtl"
dir="ltr"
style={{ display: 'flex', alignItems: 'center', gap: 24 }}
>
<span data-testid="hostile-inheritance-control">Host control</span>
Expand Down Expand Up @@ -420,9 +421,9 @@ export const DirectionOnlyInheritanceRejectsHostVisualValues: Story = {
const headingTypography = typographySnapshot(heading, ownerWindow);
void expect(headingTypography.fontFamily).toContain('Inter');
void expect(ownerWindow.getComputedStyle(trigger).direction).toBe('rtl');
void expect(ownerWindow.getComputedStyle(wrapper).direction).toBe('rtl');
void expect(ownerWindow.getComputedStyle(wrapper).direction).toBe('ltr');

await userEvent.click(within(panel).getByRole('button', { name: /select language/i }));
await userEvent.click(within(panel).getByRole('button', { name: /select a language/i }));
const languageTabs = await waitForElement<HTMLElement>(
panel,
'[data-slot="tabs-list"]',
Expand Down Expand Up @@ -481,7 +482,7 @@ export const DirectionOnlyInheritanceRejectsHostVisualValues: Story = {
const wrapperStyle = ownerWindow.getComputedStyle(wrapper);
const headingStyle = ownerWindow.getComputedStyle(heading);
const panelStyle = ownerWindow.getComputedStyle(panel);
void expect(wrapperStyle.direction).toBe('rtl');
void expect(wrapperStyle.direction).toBe('ltr');
void expect(wrapperStyle.writingMode).toBe('horizontal-tb');
void expect(wrapperStyle.textOrientation).toBe('mixed');
void expect(headingStyle.direction).toBe('rtl');
Expand Down
Loading