Repository navigation
Watch for connector requests with a one-shot background watcher, not Monitor #902
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
Open
robzolkos
wants to merge
8
commits into
main
Choose a base branch
from
connect-one-shot-watcher
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
40bcbca
Watch for connector requests with a one-shot background watcher, not …
robzolkos 2ace6c1
Drain a stopped connector's file, read lines from the watcher's outpu…
robzolkos e70f31b
Require a writable handled file, and have a compacted session check t…
robzolkos e0dc39d
Stop the watcher when the connector output it watches can't be read
robzolkos ff0e32d
Keep request-watcher regressions in the existing BATS suite
robzolkos 2830aac
Keep watcher instructions inside the existing per-request skill budget
robzolkos d5b976f
Remove repeated skill introduction without dropping request instructions
robzolkos 20449f0
Fail closed if a watcher scan loses its handled file
robzolkos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| #!/usr/bin/env bats | ||
|
|
||
| # Focused regressions for the shipped request-watching script, using the | ||
| # existing BATS suite. No Basecamp server or credentials are used. | ||
| setup() { | ||
| WATCHER="${WATCHER_UNDER_TEST:-$BATS_TEST_DIRNAME/../skills/basecamp-connect/watch-requests.sh}" | ||
| WORK="$(mktemp -d)" | ||
| OUT="$WORK/connector.log" | ||
| HANDLED="$WORK/handled" | ||
| : > "$OUT" | ||
| } | ||
|
|
||
| teardown() { | ||
| if [ -n "${WATCH_PID:-}" ]; then | ||
| kill "$WATCH_PID" 2>/dev/null || true | ||
| wait "$WATCH_PID" 2>/dev/null || true | ||
| fi | ||
| chmod -R u+rwX "$WORK" | ||
| rm -rf "$WORK" | ||
| } | ||
|
|
||
| @test "watcher reports usage without its required paths" { | ||
| run bash "$WATCHER" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *usage:* ]] | ||
| } | ||
|
|
||
| @test "watcher refuses a missing connector output" { | ||
| run bash "$WATCHER" --once "$WORK/missing" "$HANDLED" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *"no connector output"* ]] | ||
| } | ||
|
|
||
| @test "watcher can drain an empty output" { | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ -z "$output" ] | ||
| } | ||
|
|
||
| @test "watcher creates the handled directory and returns a fresh request" { | ||
| printf '%s\n' '{"type":"request","event_id":101}' > "$OUT" | ||
| run bash "$WATCHER" --once "$OUT" "$WORK/nested/handled" | ||
| [ "$status" -eq 0 ] | ||
| [ "$output" = '{"type":"request","event_id":101}' ] | ||
| [ -f "$WORK/nested/handled" ] | ||
| } | ||
|
|
||
| @test "watcher skips ids already handled" { | ||
| printf '%s\n' '{"type":"request","event_id":101}' > "$OUT" | ||
| printf '%s\n' 101 > "$HANDLED" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ -z "$output" ] | ||
| } | ||
|
|
||
| @test "watcher matches handled ids exactly rather than by prefix" { | ||
| printf '%s\n' '{"type":"request","event_id":101}' '{"type":"request","event_id":1011}' > "$OUT" | ||
| printf '%s\n' 101 > "$HANDLED" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ "$output" = '{"type":"request","event_id":1011}' ] | ||
| } | ||
|
|
||
| @test "watcher emits a duplicated request once" { | ||
| printf '%s\n' '{"type":"request","event_id":101}' '{"type":"request","event_id":101}' > "$OUT" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ "$output" = '{"type":"request","event_id":101}' ] | ||
| } | ||
|
|
||
| @test "watcher ignores event and pointer lines" { | ||
| printf '%s\n' '{"type":"event","event_id":101}' '{"event_id":102}' '{"type":"request","event_id":103}' > "$OUT" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ "$output" = '{"type":"request","event_id":103}' ] | ||
| } | ||
|
|
||
| @test "watcher does not deliver a partial line until it is complete" { | ||
| printf '%s\n' '{"type":"event","event_id":100}' > "$OUT" | ||
| printf '%s' '{"type":"request","event_id":101}' >> "$OUT" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ -z "$output" ] | ||
| printf '\n' >> "$OUT" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [ "$output" = '{"type":"request","event_id":101}' ] | ||
| } | ||
|
|
||
| @test "watcher does not mistake an escaped id in content for the request id" { | ||
| printf '%s\n' '{"type":"request","content":"quoted \"event_id\":999","event_id":101}' > "$OUT" | ||
| printf '%s\n' 999 > "$HANDLED" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *'"event_id":101}' ]] | ||
| } | ||
|
|
||
| @test "watcher preserves backslashes in the handled path" { | ||
| printf '%s\n' '{"type":"request","event_id":101}' > "$OUT" | ||
| printf '%s\n' 101 > "$WORK/handled\name" | ||
| run bash "$WATCHER" --once "$OUT" "$WORK/handled\name" | ||
| [ "$status" -eq 0 ] | ||
| [ -z "$output" ] | ||
| } | ||
|
|
||
| @test "watcher refuses an unusable handled path" { | ||
| mkdir "$HANDLED" | ||
| run bash "$WATCHER" --once "$OUT" "$HANDLED" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *"can't use the handled file"* ]] | ||
| } | ||
|
|
||
| @test "watcher fails closed if the handled file vanishes after startup" { | ||
| mkdir "$WORK/bin" | ||
| printf '%s\n' '#!/usr/bin/env bash' 'rm -f "$WATCHER_HANDLED"' 'exec "$REAL_WC" "$@"' > "$WORK/bin/wc" | ||
| chmod +x "$WORK/bin/wc" | ||
| local real_wc | ||
| real_wc="$(command -v wc)" | ||
| printf '%s\n' '{"type":"request","event_id":101}' > "$OUT" | ||
| for mode in once polling; do | ||
| printf '%s\n' 101 > "$HANDLED" | ||
| local args=() | ||
| [ "$mode" != once ] || args+=(--once) | ||
| run env PATH="$WORK/bin:$PATH" WATCHER_HANDLED="$HANDLED" REAL_WC="$real_wc" \ | ||
| bash "$WATCHER" "${args[@]}" "$OUT" "$HANDLED" | ||
| [ "$status" -eq 2 ] | ||
| [[ "$output" == *"cannot read the handled file"* ]] | ||
| [[ "$output" != *'"event_id":101'* ]] | ||
| done | ||
| } | ||
|
|
||
| @test "watcher wakes when a new complete request arrives" { | ||
| bash "$WATCHER" "$OUT" "$HANDLED" > "$WORK/result" 2> "$WORK/errors" & | ||
| WATCH_PID=$! | ||
| printf '%s\n' '{"type":"request","event_id":101}' >> "$OUT" | ||
| for _ in {1..100}; do | ||
| [ ! -s "$WORK/result" ] || break | ||
| sleep 0.05 | ||
| done | ||
| [ -s "$WORK/result" ] | ||
| for _ in {1..100}; do | ||
| kill -0 "$WATCH_PID" 2>/dev/null || break | ||
| sleep 0.05 | ||
| done | ||
| ! kill -0 "$WATCH_PID" 2>/dev/null | ||
| wait "$WATCH_PID" | ||
| WATCH_PID= | ||
| [ "$(< "$WORK/result")" = '{"type":"request","event_id":101}' ] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| #!/usr/bin/env bash | ||
| set -u | ||
|
|
||
| once=false | ||
| if [ "${1:-}" = --once ]; then | ||
| once=true | ||
| shift | ||
| fi | ||
| if [ $# -ne 2 ]; then | ||
| echo "usage: watch-requests.sh [--once] <connector-output-file> <handled-file>" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| out=$1 | ||
| handled=$2 | ||
|
|
||
| if [ ! -f "$out" ]; then | ||
| echo "watch-requests: no connector output at $out" >&2 | ||
| exit 2 | ||
| fi | ||
| if ! { mkdir -p "$(dirname "$handled")" && touch "$handled" && [ -f "$handled" ] && [ -r "$handled" ] && [ -w "$handled" ]; }; then | ||
| echo "watch-requests: can't use the handled file $handled" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| unhandled() { | ||
| local lines | ||
| if [ ! -r "$out" ]; then | ||
| echo "watch-requests: can't read the connector output at $out" >&2 | ||
| exit 2 | ||
| fi | ||
| lines=$(($(wc -l < "$out"))) | ||
| [ "$lines" -gt 0 ] || return 1 | ||
| head -n "$lines" "$out" | HANDLED=$handled awk ' | ||
| BEGIN { | ||
| while ((read = (getline id < ENVIRON["HANDLED"])) > 0) seen[id] = 1 | ||
| if (read < 0) { | ||
| print "watch-requests: cannot read the handled file " ENVIRON["HANDLED"] > "/dev/stderr" | ||
| failed = 1 | ||
| exit 2 | ||
| } | ||
| } | ||
| /"type":"request"/ && match($0, /"event_id":[0-9]+/) { | ||
| id = substr($0, RSTART + 11, RLENGTH - 11) | ||
| if (!(id in seen)) { print; seen[id] = 1; found = 1 } | ||
| } | ||
| END { if (failed) exit 2; exit !found }' | ||
| } | ||
|
|
||
| while :; do | ||
| unhandled | ||
| result=$? | ||
| case "$result" in | ||
| 0) exit 0 ;; | ||
| 1) | ||
| [ "$once" = false ] || exit 0 | ||
| sleep 2 | ||
| ;; | ||
| *) exit "$result" ;; | ||
| esac | ||
| done | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.