-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Add WebSocket hub lifecycle tests #22
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
61f27f9
5ce8ce7
2b54872
643a701
265968a
77e9880
c4f8277
e47e323
25c01bc
9ee8a7d
0392239
1713c46
ccaf7b7
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,78 @@ | ||
| name: Auto Diagnostic Bundle | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - 'feat/**' | ||
| - 'fix/**' | ||
| - 'chore/**' | ||
|
|
||
| # Skip bot commits to avoid infinite loop | ||
| concurrency: | ||
| group: diagnostic-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
||
| jobs: | ||
| build-diagnostic: | ||
| name: Run build.py and commit diagnostic bundle | ||
| runs-on: ubuntu-latest | ||
| if: "!contains(github.event.head_commit.author.name, 'github-actions')" | ||
|
|
||
| steps: | ||
| - name: Checkout branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
|
Comment on lines
+25
to
+30
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. 🧩 Analysis chain🏁 Script executed: cd .github/workflows && cat -n auto-diagnostic.yml | head -50Repository: jackjin1997/TentOfTrials Length of output: 1495 🏁 Script executed: rg "build\.py" --type yml -B5 -A5Repository: jackjin1997/TentOfTrials Length of output: 96 🏁 Script executed: # Let's also check what the persist-credentials parameter does
web_search
GitHub Actions actions/checkout persist-credentials security token git configRepository: jackjin1997/TentOfTrials Length of output: 164 🏁 Script executed: cat .github/workflows/auto-diagnostic.yml | tail -50Repository: jackjin1997/TentOfTrials Length of output: 1452 🏁 Script executed: rg "build\.py" -t yaml -B2 -A2Repository: jackjin1997/TentOfTrials Length of output: 50 🌐 Web query:
💡 Result: In GitHub Actions, actions/checkout can persist the auth token/credentials into Git’s configuration so later steps can run authenticated git commands. 1) What persist-credentials does - The actions/checkout input persist-credentials “configur[es] the token or SSH key with the local git config” [1]. - When persist-credentials is enabled, “The auth token is persisted in the local git config. This enables your scripts to run authenticated git commands. The token is removed during post-job cleanup.” [2] - To opt out, set persist-credentials: false [2]. 2) Is the token stored in.git/config or elsewhere? - The checkout README notes that the token is persisted in the “local git config” [2]. - The project’s own documentation also states an improved security behavior: persist-credentials now stores credentials in a separate file under $RUNNER_TEMP “instead of directly in.git/config” [3]. 3) How git config is involved (what you should expect) - With persist-credentials enabled, checkout configures git so authenticated operations (e.g., git fetch/push or other git commands) can work without re-providing credentials each time; this is accomplished by writing the token/credential configuration into the runner’s local git configuration area [2]. - With persist-credentials: false, checkout does not configure those credentials in git, so subsequent git commands in the workflow will not automatically authenticate using the checkout token [2]. Practical takeaway (security) - If you don’t need later steps to run authenticated git commands using the checkout token, use persist-credentials: false to avoid having checkout persist credentials into git configuration [2]. - If you do need it, treat the job as having access to the persisted auth context for any steps that can read runner filesystem/git config, since checkout’s purpose is to enable authenticated git commands after checkout [2]. Example - uses: actions/checkout@ with: persist-credentials: false Citations:
Disable checkout credential persistence before running untrusted code. This job persists a write-capable token in git config while executing Suggested hardening patch - name: Checkout branch
uses: actions/checkout@v4
with:
fetch-depth: 0
- token: ${{ secrets.GITHUB_TOKEN }}
+ persist-credentials: false
+ # ...
- name: Push diagnostic bundle
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
- git push origin HEAD
+ git push "https://x-access-token:${GITHUB_TOKEN}`@github.com/`${{ github.repository }}.git" "HEAD:${{ github.ref_name }}"🧰 Tools🪛 zizmor (1.25.2)[warning] 19-23: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false (artipacked) [error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.22' | ||
|
|
||
| - name: Set up Rust | ||
| uses: dtolnay/rust-toolchain@stable | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
|
|
||
| - name: Set up Java | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: 'temurin' | ||
| java-version: '21' | ||
|
|
||
| - name: Install system dependencies | ||
| run: | | ||
| sudo apt-get update -qq | ||
| sudo apt-get install -y --no-install-recommends \ | ||
| gcc g++ cmake make lua5.4 luajit ruby ghc | ||
|
|
||
| - name: Make encryptly executable | ||
| run: | | ||
| chmod +x tools/encryptly/linux-x64/encryptly | ||
| chmod +x tools/encryptly/linux-arm64/encryptly || true | ||
|
|
||
| - name: Configure git identity | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
|
||
| - name: Run build.py | ||
| run: python3 build.py | ||
| continue-on-error: true | ||
|
|
||
| - name: Commit and push diagnostic bundle | ||
| run: | | ||
| git add diagnostic/ || true | ||
| if git diff --cached --quiet; then | ||
| echo "No diagnostic files to commit" | ||
| exit 0 | ||
| fi | ||
| git commit -m "ci: add diagnostic bundle [skip ci]" | ||
| git push origin HEAD | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package ws | ||
|
|
||
| import ( | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| func TestHubClientRegistration(t *testing.T) { | ||
| hub := NewHub() | ||
| go hub.Run() | ||
| defer hub.Stop() | ||
|
|
||
| client := NewMockClient("test-1") | ||
| hub.Register(client) | ||
| time.Sleep(10 * time.Millisecond) | ||
|
|
||
| if !hub.HasClient(client) { | ||
| t.Error("client should be registered") | ||
| } | ||
| } | ||
|
|
||
| func TestHubClientUnregistration(t *testing.T) { | ||
| hub := NewHub() | ||
| go hub.Run() | ||
| defer hub.Stop() | ||
|
|
||
| client := NewMockClient("test-2") | ||
| hub.Register(client) | ||
| time.Sleep(10 * time.Millisecond) | ||
| hub.Unregister(client) | ||
| time.Sleep(10 * time.Millisecond) | ||
|
|
||
| if hub.HasClient(client) { | ||
| t.Error("client should be unregistered") | ||
| } | ||
| } | ||
|
|
||
| func TestHubBroadcast(t *testing.T) { | ||
| hub := NewHub() | ||
| go hub.Run() | ||
| defer hub.Stop() | ||
|
|
||
| c1 := NewMockClient("c1") | ||
| c2 := NewMockClient("c2") | ||
| hub.Register(c1) | ||
| hub.Register(c2) | ||
| time.Sleep(10 * time.Millisecond) | ||
|
|
||
| hub.Broadcast([]byte("hello")) | ||
| time.Sleep(10 * time.Millisecond) | ||
|
|
||
| if len(c1.received) != 1 || len(c2.received) != 1 { | ||
| t.Error("both clients should receive broadcast") | ||
| } | ||
|
Comment on lines
+53
to
+55
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. Broadcast assertion is checking the wrong observable.
Proposed fix- if len(c1.received) != 1 || len(c2.received) != 1 {
- t.Error("both clients should receive broadcast")
- }
+ select {
+ case msg := <-c1.send:
+ if string(msg) != "hello" {
+ t.Fatalf("unexpected message for c1: %q", string(msg))
+ }
+ case <-time.After(200 * time.Millisecond):
+ t.Fatal("c1 did not receive broadcast")
+ }
+ select {
+ case msg := <-c2.send:
+ if string(msg) != "hello" {
+ t.Fatalf("unexpected message for c2: %q", string(msg))
+ }
+ case <-time.After(200 * time.Millisecond):
+ t.Fatal("c2 did not receive broadcast")
+ }🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package ws | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| // SlowClient simulates a client that blocks on send | ||
| type SlowClient struct { | ||
| id string | ||
| send chan []byte | ||
| } | ||
|
|
||
| func NewSlowClient(id string) *SlowClient { | ||
| return &SlowClient{ | ||
| id: id, | ||
| send: make(chan []byte), // unbuffered - will block | ||
| } | ||
| } | ||
|
Comment on lines
+9
to
+19
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.
The hub loop in Also applies to: 35-39 🤖 Prompt for AI Agents |
||
|
|
||
| func (c *SlowClient) Send(data []byte) bool { | ||
| select { | ||
| case c.send <- data: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| func TestHubSlowClientCleanup(t *testing.T) { | ||
| hub := NewHub() | ||
| go hub.Run() | ||
| defer hub.Stop() | ||
|
|
||
| slow := NewSlowClient("slow") | ||
| fast := NewMockClient("fast") | ||
|
|
||
| hub.Register(slow) | ||
| hub.Register(fast) | ||
| time.Sleep(10 * time.Millisecond) | ||
|
|
||
| // Broadcast should not deadlock due to slow client | ||
| hub.Broadcast([]byte("test")) | ||
| time.Sleep(50 * time.Millisecond) | ||
|
|
||
| // Slow client should be removed after failed send | ||
| if hub.HasClient(slow) { | ||
| t.Error("slow client should be cleaned up") | ||
| } | ||
| if !hub.HasClient(fast) { | ||
| t.Error("fast client should still be registered") | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,45 @@ | ||||||||||||||||||||||||||||||||
| package ws | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||
| "sync" | ||||||||||||||||||||||||||||||||
| "testing" | ||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // MockClient implements a test client for hub tests | ||||||||||||||||||||||||||||||||
| type MockClient struct { | ||||||||||||||||||||||||||||||||
| id string | ||||||||||||||||||||||||||||||||
| send chan []byte | ||||||||||||||||||||||||||||||||
| closed bool | ||||||||||||||||||||||||||||||||
| closeMu sync.Mutex | ||||||||||||||||||||||||||||||||
| received [][]byte | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| func NewMockClient(id string) *MockClient { | ||||||||||||||||||||||||||||||||
| return &MockClient{ | ||||||||||||||||||||||||||||||||
| id: id, | ||||||||||||||||||||||||||||||||
| send: make(chan []byte, 10), | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+23
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.
Suggested direction- type MockClient struct { ... }
- func NewMockClient(id string) *MockClient { ... }
+ // Build tests around real *Client instances, and add helper functions
+ // that configure send-buffer behavior and message observation.
+ // e.g. newTestClient(remote string, sendBuf int) *Client🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| func (c *MockClient) Send(data []byte) bool { | ||||||||||||||||||||||||||||||||
| c.closeMu.Lock() | ||||||||||||||||||||||||||||||||
| defer c.closeMu.Unlock() | ||||||||||||||||||||||||||||||||
| if c.closed { | ||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| select { | ||||||||||||||||||||||||||||||||
| case c.send <- data: | ||||||||||||||||||||||||||||||||
| c.received = append(c.received, data) | ||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| func (c *MockClient) Close() { | ||||||||||||||||||||||||||||||||
| c.closeMu.Lock() | ||||||||||||||||||||||||||||||||
| defer c.closeMu.Unlock() | ||||||||||||||||||||||||||||||||
| c.closed = true | ||||||||||||||||||||||||||||||||
| close(c.send) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+45
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.
Line 44 closes Proposed fix func (c *MockClient) Close() {
c.closeMu.Lock()
defer c.closeMu.Unlock()
+ if c.closed {
+ return
+ }
c.closed = true
close(c.send)
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: jackjin1997/TentOfTrials
Length of output: 2253
🌐 Web query:
latest commit SHA actions/checkout v4 GitHub Actions💡 Result:
The tag v4 for the GitHub Action actions/checkout is a moving tag [1][2]. Because it is a branch-based tag that receives updates, it does not point to a single static commit SHA [1][3]. As of June 2026, it is maintained as a branch that tracks the latest releases within the v4 major version series [1][4]. If you require a specific, immutable version for security or reproducibility, you should use the full commit SHA of a specific release (e.g., v4.1.0 or v4.2.0) instead of the v4 tag. You can find the commit SHAs for all releases on the official GitHub actions/checkout releases page: https://github.com/actions/checkout/releases [3].
Citations:
🌐 Web query:
latest commit SHA actions/setup-go v5 GitHub Actions💡 Result:
Latest commit SHA for actions/setup-go major version v5: def8c394e3ad351a79bc93815e4a585520fe993b [1]. This SHA corresponds to commit “Rearrange README.md, add advanced-usage.md (
#724)” in the actions/setup-go repository, which is a v5-series change merged on 2026-02-26 [1]. (For reference, the v5 branch/tag family is still distinct from the repository’s newer v6 releases.) [2][3]Citations:
🌐 Web query:
latest commit SHA actions/setup-node v4 GitHub Actions💡 Result:
Latest commit SHA for actions/setup-node@v4 is 72476173717e9fa92b08abe87e9c9dc3a42dc0c8.[1] This is the commit that the v4 tag currently points to on GitHub (viewable via the tag’s tree URL). [1] Note: v4 is a major-version tag; the action’s underlying repository continues to receive commits even after the tag is set.
Citations:
🌐 Web query:
latest commit SHA actions/setup-java v4 GitHub Actions💡 Result:
As of June 21, 2026, the
actions/setup-javaGitHub Action has progressed well beyond version 4. The latest stable release is v5.3.0, published on June 16, 2026 [1]. Because thev4tag is a mutable Git reference, it points to the most recent commit tagged or associated with that version series. However, using specific semantic versioning tags or commit SHAs is the recommended practice for security and stability. Recent commits to theactions/setup-javarepository include: - Commit c76542e (April 15, 2026): Upgrade dependencies (@actions/core, cache, glob, etc.) [2]. - Commit 0756542 (April 13, 2026): Refactor error handling and improve test logging [3]. Since you are looking foractions/setup-java v4, please note that the repository has moved tov5.xreleases [4][1]. If you must use a version from thev4series, you should refer to the specific tagged release (e.g.,v4.7.1) or the commit SHA corresponding to the desired state of that version in the repository's history, rather than relying on the mutablev4tag. To find the exact current SHA of thev4tag, you can view thev4tag reference directly in the GitHub repository interface [4].Citations:
🌐 Web query:
latest commit SHA dtolnay/rust-toolchain stable GitHub Actions💡 Result:
The dtolnay/rust-toolchain GitHub Action does not use a fixed commit SHA for the "stable" version [1][2]. Instead, it uses a dynamic tagging scheme where "stable" is a branch or tag that points to the most recent version of the action [1][3]. If you wish to pin the action to a specific, immutable version for security or reproducibility, the project documentation requires you to select a full-length commit SHA directly from the history of the master branch [1][2]. To find the latest commit SHA, you should check the master branch history of the dtolnay/rust-toolchain repository [1]. Note that the author explicitly does not distinguish between different commits on master based on stability [3]. If you decide to pin to a specific SHA, it is your responsibility to review the commits and choose one you consider stable [3]. Additionally, because the action behaves dynamically based on the tag or ref provided (e.g.,
@stablevs@1.94.1) [4][5], pinning the action using a commit SHA will lock you to the behavior of the action at that specific point in time [1][2].Citations:
Pin all GitHub Actions to immutable commit SHAs.
Floating refs (
@v*/@stable) are mutable tags that allow upstream tag retargeting, which can introduce unexpected behavior changes and violates security hardening policies.Suggested hardening patch
Replace
<commit_sha>with the specific commit SHA from each action's repository.Applies to: lines 20, 26, 31, 34, 39
🧰 Tools
🪛 zizmor (1.25.2)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Source: Linters/SAST tools