Repository navigation
ci: cut workflow waste and verify locally first #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| -P ubuntu-latest=catthehacker/ubuntu:act-latest | ||
| -P ubuntu-slim=catthehacker/ubuntu:act-latest | ||
| --container-architecture linux/arm64 | ||
| --env npm_config_store_dir=/root/.local/share/pnpm/store | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| name: Setup | ||
| description: Install the project Node version and frozen dependencies with an explicit store cache. | ||
| inputs: | ||
| registry-url: | ||
| description: Registry used when publishing a package. | ||
| required: false | ||
| default: '' | ||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: 22 | ||
| package-manager-cache: false | ||
| registry-url: ${{ inputs.registry-url }} | ||
| - name: Set up pnpm | ||
| uses: pnpm/action-setup@v6 | ||
| - name: Locate dependency cache | ||
| id: store | ||
| shell: bash | ||
| run: echo "path=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" | ||
| - name: Cache dependencies | ||
| uses: actions/cache@v6 | ||
| with: | ||
| path: ${{ steps.store.outputs.path }} | ||
| key: pnpm-store-${{ runner.os }}-${{ runner.arch }}-node22-${{ hashFiles('pnpm-lock.yaml') }} | ||
| restore-keys: pnpm-store-${{ runner.os }}-${{ runner.arch }}-node22- | ||
| - name: Install dependencies | ||
| shell: bash | ||
| run: pnpm install --frozen-lockfile |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| name: Biome | ||
| 'on': | ||
| workflow_call: null | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| biome: | ||
| name: Biome | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup workspace | ||
| uses: ./.github/actions/setup | ||
| - run: pnpm check:ci | ||
| timeout-minutes: 15 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| name: Build | ||
| 'on': | ||
| workflow_call: null | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| build: | ||
| name: Build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup workspace | ||
| uses: ./.github/actions/setup | ||
| - run: pnpm build | ||
| timeout-minutes: 15 |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,29 +1,25 @@ | ||||||||||||||
| name: Publish | ||||||||||||||
|
|
||||||||||||||
| on: | ||||||||||||||
| workflow_dispatch: | ||||||||||||||
| 'on': | ||||||||||||||
| workflow_dispatch: null | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The manual trigger can publish the selected branch using OIDC credentials, bypassing the published-release trigger and publishing code before its release. Assessment: 🟠 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** .github/workflows/publish.yml
**Line:** 3:3
**Comment:**
*Security: The manual trigger can publish the selected branch using OIDC credentials, bypassing the published-release trigger and publishing code before its release.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||||||||||||||
| release: | ||||||||||||||
| types: [published] | ||||||||||||||
|
|
||||||||||||||
| types: | ||||||||||||||
| - published | ||||||||||||||
| permissions: | ||||||||||||||
| contents: read | ||||||||||||||
| id-token: write | ||||||||||||||
|
|
||||||||||||||
| concurrency: | ||||||||||||||
| group: ${{ github.workflow }}-${{ github.ref }} | ||||||||||||||
| cancel-in-progress: true | ||||||||||||||
|
Comment on lines
+10
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,130p' .github/workflows/publish.yml
git diff 9b9f5e30341cbbeb87e4e2830c93c64a5154c768 431f279871a748ee0724fc055b0da92a6be64857 -- .github/workflows/publish.ymlRepository: YosefHayim/agent-session-pack Length of output: 1818 Do not cancel an in-progress publish run. The workflow accepts both Set 🐛 Suggested fix--- "a/.github/workflows/publish.yml"
+++ "b/.github/workflows/publish.yml"
@@ -7,9 +7,9 @@
permissions:
contents: read
id-token: write
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
- cancel-in-progress: true
+ cancel-in-progress: false
jobs:
publish:
name: Publish📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| jobs: | ||||||||||||||
| publish: | ||||||||||||||
| name: Publish | ||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||
| steps: | ||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||
| - uses: pnpm/action-setup@v4 | ||||||||||||||
| - name: Setup workspace | ||||||||||||||
| uses: ./.github/actions/setup | ||||||||||||||
| with: | ||||||||||||||
| version: 10.12.1 | ||||||||||||||
| run_install: false | ||||||||||||||
| - uses: actions/setup-node@v4 | ||||||||||||||
| with: | ||||||||||||||
| node-version: 22 | ||||||||||||||
| registry-url: https://registry.npmjs.org | ||||||||||||||
| cache: pnpm | ||||||||||||||
| - run: pnpm install --frozen-lockfile | ||||||||||||||
| - run: pnpm build | ||||||||||||||
| - run: npm publish --provenance --access public | ||||||||||||||
| timeout-minutes: 15 | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| name: Test | ||
| 'on': | ||
| workflow_call: null | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| test: | ||
| name: Test | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup workspace | ||
| uses: ./.github/actions/setup | ||
| - run: sudo apt-get update && sudo apt-get install -y zstd sqlite3 | ||
| - run: pnpm test | ||
| timeout-minutes: 20 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| name: Typecheck | ||
| 'on': | ||
| workflow_call: null | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| typecheck: | ||
| name: Typecheck | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup workspace | ||
| uses: ./.github/actions/setup | ||
| - run: pnpm typecheck | ||
| timeout-minutes: 15 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,7 +80,7 @@ | |
| "evidence:local": "tsx src/cli/main.ts savings" | ||
| }, | ||
| "engines": { | ||
| "node": ">=20.0.0" | ||
| "node": ">=22" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: This marks Node 20 unsupported, while the Quick start still promises Node 20+; installs that enforce package engines now fail for users following the documented requirement. Assessment: 🟠 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** package.json
**Line:** 83:83
**Comment:**
*Api Mismatch: This marks Node 20 unsupported, while the Quick start still promises Node 20+; installs that enforce package engines now fail for users following the documented requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
| }, | ||
| "devDependencies": { | ||
| "@biomejs/biome": "^2.5.2", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: YosefHayim/agent-session-pack
Length of output: 1846
🏁 Script executed:
Repository: YosefHayim/agent-session-pack
Length of output: 8285
🏁 Script executed:
Repository: YosefHayim/agent-session-pack
Length of output: 6903
Use the host architecture by default.
The root
.actrcforceslinux/arm64for every localactrun. On x86_64 hosts, this can run the Node setup, dependency installation, tests, and build under emulation, which can slow local verification. (docs.docker.com)No inspected repository documentation requires Apple Silicon or an arm64 target. Remove the global override. Developers who need a specific architecture can pass it explicitly to
act.Suggested fix
-P ubuntu-latest=catthehacker/ubuntu:act-latest -P ubuntu-slim=catthehacker/ubuntu:act-latest --- --container-architecture linux/arm64 --env npm_config_store_dir=/root/.local/share/pnpm/store🤖 Prompt for AI Agents