feat: Add Microsoft Teams notifications via Graph API#118
feat: Add Microsoft Teams notifications via Graph API#118
Conversation
spboyer
requested review from
chlowell,
richardpark-msft and
wbreza
as code owners
There was a problem hiding this comment.
Pull request overview
Adds a Squad-level Microsoft Teams notification integration (via Microsoft Graph using az rest), along with supporting documentation, configuration, and squad logs/decisions so Scribe (and others) can broadcast milestone updates to a Teams channel.
Changes:
- Add
.squad/scripts/teams-notify.sh(Graph API sender) and.squad/scripts/teams-test.sh(manual verification helper). - Add a new skill doc
.squad/skills/teams-notify/SKILL.mdand wire the integration into squad docs/charters. - Add Teams configuration and record the decision/logs in
.squad/decisions.mdand related orchestration/session logs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
.squad/team.md |
Documents Teams as a squad integration (channel/method/config). |
.squad/skills/teams-notify/SKILL.md |
New skill documentation for how Teams notifications work and how to use them. |
.squad/scripts/teams-notify.sh |
Implements the notification sender via az rest to Microsoft Graph. |
.squad/scripts/teams-test.sh |
Adds a convenience script to send a test notification. |
.squad/identity/teams-config.json |
Adds Teams integration configuration (enabled flag, IDs, per-event toggles). |
.squad/decisions.md |
Records design decisions and integration details for Teams notifications. |
.squad/agents/scribe/charter.md |
Instructs Scribe when/how to trigger Teams notifications after logging. |
.squad/agents/livingston/history.md |
Adds an entry documenting the Teams notification skill work. |
.squad/agents/linus/history.md |
Adds an entry documenting the Teams notification scripts work. |
.squad/orchestration-log/2026-03-12T13-45-livingston-teams-skill.md |
Orchestration log for skill documentation work. |
.squad/orchestration-log/2026-03-12T13-45-linus-teams-notify.md |
Orchestration log for script implementation work. |
.squad/log/2026-03-12T13-45-teams-integration.md |
Session log summarizing the Teams integration batch. |
Comments suppressed due to low confidence (1)
.squad/scripts/teams-notify.sh:117
- The grep/sed fallback in
read_notify_flagcan also exit non-zero when the key isn’t found; withpipefailthis will abort the script underset -e, again breaking the “always exit 0” guarantee. Make the fallback pipeline resilient (e.g., ensure the pipeline returns 0 and default tofalsewhen no match).
read_notify_flag() {
local event="$1"
if $USE_JQ; then
jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null
else
# Fallback: look for the key inside notify_on block
grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \
| sed 's/.*:[[:space:]]*//' | head -1
fi
You can also share your feedback on Copilot code review. Take the survey.
| if $USE_JQ; then | ||
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null | ||
| else | ||
| # Fallback: simple grep/sed for flat keys | ||
| # Works for top-level string/boolean values | ||
| local simple_key | ||
| simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g') | ||
| grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" \ | ||
| | sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//' | ||
| fi | ||
| } | ||
|
|
||
| read_notify_flag() { | ||
| local event="$1" | ||
| if $USE_JQ; then | ||
| jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null | ||
| else | ||
| # Fallback: look for the key inside notify_on block | ||
| grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \ | ||
| | sed 's/.*:[[:space:]]*//' | head -1 | ||
| fi |
There was a problem hiding this comment.
With set -euo pipefail, the jq -r "$key" ... call can exit non-zero (e.g., invalid JSON, missing key when using -e, etc.) and terminate the script with a non-zero status, which contradicts the documented “Always exits 0” contract. Consider making config reads non-fatal (e.g., force a 0 exit from read_config_value and return empty/default values on parse errors).
This issue also appears on line 109 of the same file.
| if $USE_JQ; then | |
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null | |
| else | |
| # Fallback: simple grep/sed for flat keys | |
| # Works for top-level string/boolean values | |
| local simple_key | |
| simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g') | |
| grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" \ | |
| | sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//' | |
| fi | |
| } | |
| read_notify_flag() { | |
| local event="$1" | |
| if $USE_JQ; then | |
| jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null | |
| else | |
| # Fallback: look for the key inside notify_on block | |
| grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \ | |
| | sed 's/.*:[[:space:]]*//' | head -1 | |
| fi | |
| local value="" | |
| if $USE_JQ; then | |
| # Use a conditional so jq failures don't trigger set -e | |
| if value=$(jq -r "$key" "$CONFIG_FILE" 2>/dev/null); then | |
| printf '%s\n' "$value" | |
| fi | |
| else | |
| # Fallback: simple grep/sed for flat keys | |
| # Works for top-level string/boolean values | |
| local simple_key | |
| simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g') | |
| # Wrap pipeline in a conditional to avoid pipefail aborts | |
| if value=$(grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" 2>/dev/null \ | |
| | sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//' 2>/dev/null); then | |
| printf '%s\n' "$value" | |
| fi | |
| fi | |
| # Always succeed to honor "Always exits 0" contract | |
| return 0 | |
| } | |
| read_notify_flag() { | |
| local event="$1" | |
| local value="" | |
| if $USE_JQ; then | |
| # Default to false if jq fails or key is missing | |
| if value=$(jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null); then | |
| printf '%s\n' "$value" | |
| else | |
| printf '%s\n' "false" | |
| fi | |
| else | |
| # Fallback: look for the key inside notify_on block | |
| if value=$(grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" 2>/dev/null \ | |
| | sed 's/.*:[[:space:]]*//' 2>/dev/null | head -1); then | |
| # If grep/sed produced a value, print it; otherwise default to false | |
| if [[ -n "$value" ]]; then | |
| printf '%s\n' "$value" | |
| else | |
| printf '%s\n' "false" | |
| fi | |
| else | |
| printf '%s\n' "false" | |
| fi | |
| fi | |
| # Always succeed to honor "Always exits 0" contract | |
| return 0 |
.squad/scripts/teams-notify.sh
Outdated
| VALID_EVENTS="work_complete pr_opened pr_merged issue_closed decisions" | ||
| if ! echo "$VALID_EVENTS" | grep -qw "$EVENT_TYPE"; then | ||
| echo "Warning: Unknown event type '$EVENT_TYPE'. Valid: $VALID_EVENTS" >&2 | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
VALID_EVENTS does not include ralph_status, but teams-config.json and the skill docs list ralph_status as a supported event. Either add ralph_status support here (including header formatting) or remove it from the config/docs to avoid users hitting the “Unknown event type” warning.
| # Usage: teams-test.sh [custom_message] | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| NOTIFY_SCRIPT="$SCRIPT_DIR/teams-notify.sh" | ||
|
|
||
| if [[ ! -x "$NOTIFY_SCRIPT" ]]; then | ||
| echo "Error: teams-notify.sh not found or not executable at $NOTIFY_SCRIPT" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| MESSAGE="${1:-🧪 Waza Squad notifications are working!}" | ||
|
|
||
| echo "Sending test notification to Teams..." | ||
| "$NOTIFY_SCRIPT" work_complete "$MESSAGE" | ||
|
|
||
| echo "Done. Check the Waza Squad channel in Teams." |
There was a problem hiding this comment.
This script is described as a verification tool, but it can’t reliably detect delivery failures because teams-notify.sh always exits 0 (including when the API call fails). Consider adding a strict/verify mode (or a separate direct az rest probe) so teams-test.sh can exit non-zero when the message wasn’t posted.
| # Usage: teams-test.sh [custom_message] | |
| set -euo pipefail | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| NOTIFY_SCRIPT="$SCRIPT_DIR/teams-notify.sh" | |
| if [[ ! -x "$NOTIFY_SCRIPT" ]]; then | |
| echo "Error: teams-notify.sh not found or not executable at $NOTIFY_SCRIPT" >&2 | |
| exit 1 | |
| fi | |
| MESSAGE="${1:-🧪 Waza Squad notifications are working!}" | |
| echo "Sending test notification to Teams..." | |
| "$NOTIFY_SCRIPT" work_complete "$MESSAGE" | |
| echo "Done. Check the Waza Squad channel in Teams." | |
| # Usage: | |
| # teams-test.sh [custom_message] | |
| # teams-test.sh --verify-direct [custom_message] | |
| # | |
| # In --verify-direct mode, this script posts directly to the Teams webhook | |
| # using `az rest` so that failures are reflected in the exit code. | |
| set -euo pipefail | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| NOTIFY_SCRIPT="$SCRIPT_DIR/teams-notify.sh" | |
| VERIFY_DIRECT=0 | |
| if [[ "${1-}" == "--verify-direct" ]]; then | |
| VERIFY_DIRECT=1 | |
| shift | |
| fi | |
| MESSAGE="${1:-🧪 Waza Squad notifications are working!}" | |
| if [[ "$VERIFY_DIRECT" -eq 0 ]]; then | |
| if [[ ! -x "$NOTIFY_SCRIPT" ]]; then | |
| echo "Error: teams-notify.sh not found or not executable at $NOTIFY_SCRIPT" >&2 | |
| exit 1 | |
| fi | |
| echo "Sending test notification to Teams via teams-notify.sh..." | |
| "$NOTIFY_SCRIPT" work_complete "$MESSAGE" | |
| echo "Done. Check the Waza Squad channel in Teams." | |
| else | |
| if [[ -z "${TEAMS_WEBHOOK_URL-}" ]]; then | |
| echo "Error: TEAMS_WEBHOOK_URL environment variable is not set for --verify-direct mode." >&2 | |
| exit 1 | |
| fi | |
| echo "Sending test notification to Teams via az rest (strict verify mode)..." | |
| PAYLOAD=$(printf '{"text": "%s"}' "$MESSAGE") | |
| az rest \ | |
| --method post \ | |
| --uri "$TEAMS_WEBHOOK_URL" \ | |
| --headers 'Content-Type=application/json' \ | |
| --body "$PAYLOAD" >/dev/null | |
| echo "Done. Test message posted via az rest. Check the Waza Squad channel in Teams." | |
| fi |
.squad/team.md
Outdated
|
|
||
| | Service | Channel | Method | Config | | ||
| |---------|---------|--------|--------| | ||
| | Microsoft Teams | [Waza Squad](https://teams.microsoft.com/l/channel/19%3A288df9bbfec84a1da3aec636c7b829a5%40thread.tacv2/Waza%20Squad?groupId=450e4e32-11f8-4436-a9b4-4990ae16fe58&tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47) | Graph API via `az rest` | `.squad/identity/teams-config.json` | |
There was a problem hiding this comment.
This embeds a full Teams channel link including groupId and tenantId. If this repo is public, that’s likely internal metadata that shouldn’t be committed. Suggest replacing the link with a plain channel name (or a placeholder) and keeping the concrete IDs in a local-only config file.
| | Microsoft Teams | [Waza Squad](https://teams.microsoft.com/l/channel/19%3A288df9bbfec84a1da3aec636c7b829a5%40thread.tacv2/Waza%20Squad?groupId=450e4e32-11f8-4436-a9b4-4990ae16fe58&tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47) | Graph API via `az rest` | `.squad/identity/teams-config.json` | | |
| | Microsoft Teams | Waza Squad (see config for IDs) | Graph API via `az rest` | `.squad/identity/teams-config.json` | |
.squad/identity/teams-config.json
Outdated
| "enabled": true, | ||
| "group_id": "450e4e32-11f8-4436-a9b4-4990ae16fe58", | ||
| "channel_id": "19:288df9bbfec84a1da3aec636c7b829a5@thread.tacv2", | ||
| "channel_name": "Waza Squad", |
There was a problem hiding this comment.
This commits real group/channel IDs and sets enabled: true by default. That can lead to unintended posts from any environment where az is logged in, and it also publishes internal identifiers. Consider shipping an example/template config (checked in with enabled: false and placeholder IDs) and gitignoring the real .squad/identity/teams-config.json.
| "enabled": true, | |
| "group_id": "450e4e32-11f8-4436-a9b4-4990ae16fe58", | |
| "channel_id": "19:288df9bbfec84a1da3aec636c7b829a5@thread.tacv2", | |
| "channel_name": "Waza Squad", | |
| "enabled": false, | |
| "group_id": "YOUR-TEAMS-GROUP-ID", | |
| "channel_id": "YOUR-TEAMS-CHANNEL-ID", | |
| "channel_name": "Example Squad Channel", |
.squad/skills/teams-notify/SKILL.md
Outdated
| 3. Test with `az rest` directly: | ||
| ```bash | ||
| az rest --method post \ | ||
| --uri "https://graph.microsoft.com/v1.0/teams/{group_id}/channels/{channel_id}/messages" \ |
There was a problem hiding this comment.
Troubleshooting mixes az rest --uri here while the script uses az rest --url. Even if both flags work, keeping the docs consistent with the script avoids confusion when users copy/paste commands.
| --uri "https://graph.microsoft.com/v1.0/teams/{group_id}/channels/{channel_id}/messages" \ | |
| --url "https://graph.microsoft.com/v1.0/teams/{group_id}/channels/{channel_id}/messages" \ |
.squad/skills/teams-notify/SKILL.md
Outdated
| **Fix:** | ||
| - Ensure the script exists: `ls -la .squad/scripts/teams-notify.sh` | ||
| - Make it executable: `chmod +x .squad/scripts/teams-notify.sh` | ||
| - Call from the repository root: `cd /Users/shboyer/github/waza && .squad/scripts/teams-notify.sh ...` |
There was a problem hiding this comment.
This suggests running from a hard-coded local path (/Users/shboyer/...), which won’t apply for other contributors and leaks a personal filesystem location into repo docs. Suggest using a generic example like cd <repo-root> or referencing $TEAM_ROOT.
| - Call from the repository root: `cd /Users/shboyer/github/waza && .squad/scripts/teams-notify.sh ...` | |
| - Call from the repository root: `cd <repo-root> && .squad/scripts/teams-notify.sh ...` |
.squad/skills/teams-notify/SKILL.md
Outdated
| - Squad workflows **continue normally** — no errors, no retries, no noise | ||
| - The script exits cleanly (`enabled: false` → skip → exit 0) |
There was a problem hiding this comment.
The “Graceful Degradation” section claims “no errors, no noise” when Teams isn’t configured, but teams-notify.sh currently prints a warning when the config file is missing. Either adjust the script to be silent when unconfigured or update this section to match the script’s actual behavior.
| - Squad workflows **continue normally** — no errors, no retries, no noise | |
| - The script exits cleanly (`enabled: false` → skip → exit 0) | |
| - Squad workflows **continue normally** — no failures, no retries | |
| - The script exits cleanly (`enabled: false` or missing config → skip (optionally log a single warning) → exit 0) |
.squad/agents/livingston/history.md
Outdated
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` (not yet created; documented as expected location) | ||
| - Graph API endpoint: `/me/teamwork/sendActivityNotification` (documented in See Also) |
There was a problem hiding this comment.
This history entry says the notify script is “not yet created” and references /me/teamwork/sendActivityNotification, but the PR adds .squad/scripts/teams-notify.sh and it posts to /teams/{groupId}/channels/{channelId}/messages. Please update this entry to reflect what actually shipped in this change set.
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` (not yet created; documented as expected location) | |
| - Graph API endpoint: `/me/teamwork/sendActivityNotification` (documented in See Also) | |
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` | |
| - Graph API endpoint: `/teams/{groupId}/channels/{channelId}/messages` (used by the notification script) |
| ### Files | ||
|
|
||
| - `.squad/scripts/teams-notify.sh` — Main notification script | ||
| - `.squad/scripts/teams-test.sh` — Test/verification companion | ||
| - `.squad/identity/teams-config.json` — Channel config (pre-existing) | ||
|
|
There was a problem hiding this comment.
This says .squad/identity/teams-config.json is “pre-existing”, but it’s introduced in this PR. Please correct the wording so the decision log matches the actual change history (and avoid implying the repo previously contained these IDs).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
.squad/scripts/teams-notify.sh:131
read_config_value/read_notify_flagcan return a non-zero status (e.g.,jqparse errors orgrepfinding no match) which—because ofset -e+pipefailand the surrounding$(...)assignments—will abort the script with a non-zero exit. Make these helpers “non-fatal” (return empty/false and status 0) so missing keys or parse failures degrade gracefully as intended.
read_config_value() {
local key="$1"
if $USE_JQ; then
jq -r "$key" "$CONFIG_FILE" 2>/dev/null
else
# Fallback: simple grep/sed for flat keys
# Works for top-level string/boolean values
local simple_key
simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g')
grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" \
| sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//'
fi
}
read_notify_flag() {
local event="$1"
if $USE_JQ; then
jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null
else
# Fallback: look for the key inside notify_on block
grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \
| sed 's/.*:[[:space:]]*//' | head -1
fi
.squad/scripts/teams-notify.sh:301
- In the Adaptive Card request body,
--arg card_content "$card_json"makes.attachments[].contenta JSON string rather than an object. Microsoft Graph’s attachmentcontentis expected to be an object for Adaptive Cards; sending it as a string can cause cards to render incorrectly or be rejected. Use--argjson(or otherwise ensurecontentis an object) when embedding the card JSON.
jq -n \
--arg card_content "$card_json" \
'{
"body": {
"contentType": "html",
"content": "<attachment id=\"card1\"></attachment>"
},
"attachments": [
{
"id": "card1",
"contentType": "application/vnd.microsoft.card.adaptive",
"contentUrl": null,
"content": $card_content
}
]
.squad/decisions.md:494
- This later list also calls
.squad/identity/teams-config.json“pre-existing”, but the PR adds the file. Please update this wording for accuracy in the decision record.
- **Script:** `.squad/scripts/teams-notify.sh` — Main notification sender
- **Test:** `.squad/scripts/teams-test.sh` — Verification tool
- **Config:** `.squad/identity/teams-config.json` — Channel config (pre-existing)
- **Skill:** `.squad/skills/teams-notify/SKILL.md` — Usage documentation
You can also share your feedback on Copilot code review. Take the survey.
.squad/scripts/teams-notify.sh
Outdated
| # Exit behavior: Always exits 0 to never break the caller's workflow. | ||
| # Errors are logged to stderr as warnings. | ||
|
|
||
| set -euo pipefail |
There was a problem hiding this comment.
The script claims it “always exits 0”, but set -euo pipefail can still cause an immediate non-zero exit on unhandled command failures (including failures inside command substitutions/pipelines). To uphold the contract, either drop -e/pipefail here or guard every potentially-failing command so the script never terminates early.
This issue also appears on line 109 of the same file.
| set -euo pipefail | |
| set -u |
.squad/scripts/teams-notify.sh
Outdated
| local safe_msg | ||
| safe_msg=$(escape_html "$MESSAGE") | ||
| local html="<h3>${CARD_TITLE}</h3><p>${safe_msg}</p><hr/><p style=\"color:gray;font-size:small;\">Waza Squad · ${TIMESTAMP}</p>" | ||
|
|
||
| # Manual JSON — escape embedded quotes | ||
| local escaped="${html//\"/\\\"}" | ||
| echo "{\"body\":{\"contentType\":\"html\",\"content\":\"${escaped}\"}}" |
There was a problem hiding this comment.
build_html_body manually constructs JSON but only escapes quotes; backslashes and control characters (e.g., newlines, tabs) in MESSAGE/CARD_TITLE can produce invalid JSON or altered content (e.g., \t sequences). Prefer using jq -n --arg ... to build the HTML message payload when jq is available, and ensure the non-jq fallback performs full JSON string escaping.
This issue also appears on line 287 of the same file.
| local safe_msg | |
| safe_msg=$(escape_html "$MESSAGE") | |
| local html="<h3>${CARD_TITLE}</h3><p>${safe_msg}</p><hr/><p style=\"color:gray;font-size:small;\">Waza Squad · ${TIMESTAMP}</p>" | |
| # Manual JSON — escape embedded quotes | |
| local escaped="${html//\"/\\\"}" | |
| echo "{\"body\":{\"contentType\":\"html\",\"content\":\"${escaped}\"}}" | |
| # Fallback JSON string escaper for when jq is not available | |
| json_escape() { | |
| local s="$1" | |
| # Order matters: escape backslash before quotes, then control chars | |
| s="${s//\\/\\\\}" | |
| s="${s//\"/\\\"}" | |
| s="${s//$'\n'/\\n}" | |
| s="${s//$'\r'/\\r}" | |
| s="${s//$'\t'/\\t}" | |
| s="${s//$'\f'/\\f}" | |
| s="${s//$'\b'/\\b}" | |
| echo "$s" | |
| } | |
| local safe_msg | |
| safe_msg=$(escape_html "$MESSAGE") | |
| local html="<h3>${CARD_TITLE}</h3><p>${safe_msg}</p><hr/><p style=\"color:gray;font-size:small;\">Waza Squad · ${TIMESTAMP}</p>" | |
| if $USE_JQ; then | |
| # Prefer jq to build JSON safely when available | |
| jq -n --arg html "$html" \ | |
| '{"body":{"contentType":"html","content":$html}}' | |
| else | |
| # Manual JSON — perform full JSON string escaping | |
| local escaped | |
| escaped=$(json_escape "$html") | |
| echo "{\"body\":{\"contentType\":\"html\",\"content\":\"${escaped}\"}}" | |
| fi |
.squad/agents/livingston/history.md
Outdated
| **File paths:** | ||
| - Skill document: `.squad/skills/teams-notify/SKILL.md` | ||
| - Configuration: `.squad/identity/teams-config.json` | ||
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` (not yet created; documented as expected location) |
There was a problem hiding this comment.
This history entry says .squad/scripts/teams-notify.sh is “not yet created”, but the PR adds that script. Please update the history note so it reflects the current state (otherwise it’s confusing for future readers).
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` (not yet created; documented as expected location) | |
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` |
| ```json | ||
| { | ||
| "enabled": true, | ||
| "group_id": "450e4e32-11f8-4436-a9b4-4990ae16fe58", | ||
| "channel_id": "19:288df9bbfec84a1da3aec636c7b829a5@thread.tacv2", | ||
| "channel_name": "Waza Squad", |
There was a problem hiding this comment.
The configuration schema example includes real group_id / channel_id values. If this repo is public, publishing real internal identifiers makes them hard to rotate and encourages copy/paste of production values. Prefer placeholder values (or an explicit “example IDs” note) and point readers to local config for the real IDs.
.squad/agents/livingston/history.md
Outdated
| - Skill document: `.squad/skills/teams-notify/SKILL.md` | ||
| - Configuration: `.squad/identity/teams-config.json` | ||
| - Script location (for user reference): `.squad/scripts/teams-notify.sh` (not yet created; documented as expected location) | ||
| - Graph API endpoint: `/me/teamwork/sendActivityNotification` (documented in See Also) |
There was a problem hiding this comment.
This history note says the Graph API endpoint is /me/teamwork/sendActivityNotification, but the implementation in this PR posts to /v1.0/teams/{groupId}/channels/{channelId}/messages. Please correct the endpoint reference to match the actual integration.
| - Graph API endpoint: `/me/teamwork/sendActivityNotification` (documented in See Also) | |
| - Graph API endpoint: `/v1.0/teams/{groupId}/channels/{channelId}/messages` |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
.squad/scripts/teams-notify.sh:230
- HTML request body is built via string interpolation and only escapes quotes. If MESSAGE (or resulting HTML) contains backslashes or literal newlines, the JSON string will be invalid and the POST will fail. Either (a) always construct the JSON with jq, or (b) implement proper JSON string escaping for \ / \n / \r / \t in the non-jq path.
local safe_msg
safe_msg=$(escape_html "$MESSAGE")
local html="<h3>${CARD_TITLE}</h3><p>${safe_msg}</p><hr/><p style=\"color:gray;font-size:small;\">Waza Squad · ${TIMESTAMP}</p>"
# Manual JSON — escape embedded quotes
local escaped="${html//\"/\\\"}"
echo "{\"body\":{\"contentType\":\"html\",\"content\":\"${escaped}\"}}"
}
.squad/scripts/ralph-watch.sh:251
- Same subshell issue as above: this piped
whileloop meansTOTAL_ISSUE_CLOSEDincrements won’t persist, so the shutdown summary will be wrong. Prefer process substitution / no-pipe loop to keep counter updates in the main shell.
if [[ "$count" -gt 0 ]]; then
echo "$new_issues" | jq -c '.[]' | while read -r issue; do
local num title
num=$(echo "$issue" | jq -r '.number')
title=$(echo "$issue" | jq -r '.title')
echo " ✅ Issue #${num} closed: ${title}"
"$NOTIFY_SCRIPT" issue_closed "Issue #${num} closed: ${title}" 2>/dev/null || true
TOTAL_ISSUE_CLOSED=$((TOTAL_ISSUE_CLOSED + 1))
done
You can also share your feedback on Copilot code review. Take the survey.
.squad/scripts/teams-notify.sh
Outdated
| # Wrap card in Graph API message format | ||
| jq -n \ | ||
| --arg card_content "$card_json" \ | ||
| '{ | ||
| "body": { | ||
| "contentType": "html", | ||
| "content": "<attachment id=\"card1\"></attachment>" | ||
| }, | ||
| "attachments": [ | ||
| { | ||
| "id": "card1", | ||
| "contentType": "application/vnd.microsoft.card.adaptive", | ||
| "contentUrl": null, | ||
| "content": $card_content | ||
| } | ||
| ] | ||
| }' | ||
| } |
There was a problem hiding this comment.
Adaptive Card payload wraps card_json using jq -n --arg card_content "$card_json", which makes attachments[].content a JSON string rather than an object. Graph expects attachments[].content to be a JSON object, so cards may not render. Use --argjson (or build the wrapper in the same jq program) so content is emitted as JSON, not a quoted string.
This issue also appears on line 223 of the same file.
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null | ||
| else | ||
| # Fallback: simple grep/sed for flat keys | ||
| # Works for top-level string/boolean values | ||
| local simple_key | ||
| simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g') | ||
| grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" \ | ||
| | sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//' | ||
| fi | ||
| } | ||
|
|
||
| read_notify_flag() { | ||
| local event="$1" | ||
| if $USE_JQ; then | ||
| jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null | ||
| else | ||
| # Fallback: look for the key inside notify_on block | ||
| grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \ | ||
| | sed 's/.*:[[:space:]]*//' | head -1 |
There was a problem hiding this comment.
The script claims it “always exits 0”, but with set -euo pipefail the grep/sed fallback pipelines in read_config_value/read_notify_flag can terminate the script non-zero when a key is missing or grep finds no match (exit 1 propagates via pipefail). Consider disabling -e for these reads or defensively appending || true / explicit defaulting so missing config just results in a warning + exit 0 as intended.
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null | |
| else | |
| # Fallback: simple grep/sed for flat keys | |
| # Works for top-level string/boolean values | |
| local simple_key | |
| simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g') | |
| grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" \ | |
| | sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//' | |
| fi | |
| } | |
| read_notify_flag() { | |
| local event="$1" | |
| if $USE_JQ; then | |
| jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null | |
| else | |
| # Fallback: look for the key inside notify_on block | |
| grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \ | |
| | sed 's/.*:[[:space:]]*//' | head -1 | |
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null || true | |
| else | |
| # Fallback: simple grep/sed for flat keys | |
| # Works for top-level string/boolean values | |
| local simple_key | |
| simple_key=$(echo "$key" | sed 's/^\.//; s/\./_/g') | |
| grep -o "\"${simple_key}\"[[:space:]]*:[[:space:]]*[^,}]*" "$CONFIG_FILE" \ | |
| | sed 's/.*:[[:space:]]*//; s/"//g; s/[[:space:]]*$//' || true | |
| fi | |
| } | |
| read_notify_flag() { | |
| local event="$1" | |
| if $USE_JQ; then | |
| jq -r ".notify_on.${event} // false" "$CONFIG_FILE" 2>/dev/null || true | |
| else | |
| # Fallback: look for the key inside notify_on block | |
| grep -o "\"${event}\"[[:space:]]*:[[:space:]]*[a-z]*" "$CONFIG_FILE" \ | |
| | sed 's/.*:[[:space:]]*//' | head -1 || true |
.squad/scripts/ralph-watch.sh
Outdated
| echo "$new_prs" | jq -c '.[]' | while read -r pr; do | ||
| local num title author | ||
| num=$(echo "$pr" | jq -r '.number') | ||
| title=$(echo "$pr" | jq -r '.title') | ||
| author=$(echo "$pr" | jq -r '.author.login // .author.name // "unknown"') | ||
|
|
||
| echo " 📦 PR #${num} merged: ${title} (by ${author})" | ||
| "$NOTIFY_SCRIPT" pr_merged "PR #${num} merged: ${title} (by ${author})" 2>/dev/null || true | ||
| TOTAL_PR_MERGED=$((TOTAL_PR_MERGED + 1)) | ||
| done |
There was a problem hiding this comment.
This while ...; do loop runs in a subshell because it’s fed by a pipe. As a result, increments to TOTAL_PR_MERGED won’t persist, and the shutdown summary will always show 0 merged PR notifications even when events were sent. Use process substitution (while ...; do ...; done < <(...)) or avoid the pipe so counters update in the parent shell.
This issue also appears on line 242 of the same file.
| | `ralph_status` | Sensei compliance check completes | "Sensei check: 3 skills updated, 2 need work" | | ||
|
|
||
| Enable or disable any event in `notify_on` configuration. |
There was a problem hiding this comment.
Docs list ralph_status as a supported event type, but teams-notify.sh rejects unknown event types and doesn’t include ralph_status in VALID_EVENTS. Either add ralph_status support end-to-end (config + script + formatting) or remove it from the documentation/config to avoid silent no-op behavior.
| | `ralph_status` | Sensei compliance check completes | "Sensei check: 3 skills updated, 2 need work" | | |
| Enable or disable any event in `notify_on` configuration. | |
| Enable or disable any of these events in the `notify_on` configuration. |
| { | ||
| "enabled": true, | ||
| "group_id": "450e4e32-11f8-4436-a9b4-4990ae16fe58", | ||
| "channel_id": "19:288df9bbfec84a1da3aec636c7b829a5@thread.tacv2", | ||
| "channel_name": "Waza Squad", | ||
| "notify_on": { |
There was a problem hiding this comment.
Repo-default teams-config.json ships with enabled: true and concrete group/channel IDs. That means anyone running teams-test.sh, teams-notify.sh, or ralph-watch.sh from a fresh checkout will post into the real channel by default (potential noise/spam). Consider defaulting enabled to false (and/or shipping an example config) so notifications are opt-in per user/environment.
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| TEAM_ROOT="${TEAM_ROOT:-$(cd "$SCRIPT_DIR/../.." && pwd)}" | ||
| NOTIFY_SCRIPT="$SCRIPT_DIR/teams-notify.sh" | ||
| STATE_FILE="$TEAM_ROOT/.squad/identity/.ralph-watch-state.json" | ||
| INTERVAL_MINUTES=10 |
There was a problem hiding this comment.
ralph-watch.sh writes its dedupe state to .squad/identity/.ralph-watch-state.json, but the repo .gitignore doesn’t currently ignore this file. That makes it easy to accidentally commit personal/local state. Consider moving the state under an ignored directory (e.g., tmp/) or adding an ignore rule, and document the expected behavior.
Merge decisions inbox into team memory and add Teams notification support for milestone updates (work batches, PRs, issues, decisions). Changes: - Merged 6 decision files from inbox/ into .squad/decisions.md - Added teams-notify.sh script for Microsoft Graph API integration - Added teams-test.sh verification companion - Updated Scribe charter with Teams notification protocols - Updated team.md with Integrations section - Updated agent histories (Linus, Livingston) - Added teams-config.json for channel configuration Teams notifications now enabled via .squad/scripts/teams-notify.sh with event filtering, HTML escaping, and graceful error handling (never breaks CI/CD). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rich card format with accent colors, structured fields, and professional layout for each event type. Falls back to plain HTML when jq is not available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adaptive Cards for milestone events (work_complete, pr_merged, decisions). Simple HTML for status events (pr_opened, issue_closed). Falls back to all-HTML when jq is unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Polls GitHub via gh CLI every N minutes for merged PRs, closed issues, and new commits. Posts to Teams channel via teams-notify.sh. State tracked in .squad/identity/.ralph-watch-state.json. Usage: .squad/scripts/ralph-watch.sh --interval 10 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
spboyer
force-pushed
the
squad/teams-notifications
branch
from
d86d2b6 to
cb22ac5
Compare
Security: - Replace real group/channel IDs with placeholders in teams-config.json - Set enabled: false by default, create .example file with placeholders - Remove full Teams channel URL (with tenantId) from team.md - Replace real IDs in SKILL.md schema example Script bugs: - Drop set -euo pipefail in teams-notify.sh (contradicts always-exit-0 contract) - Use jq for proper JSON escaping in build_html_body when available - Fix --arg to --argjson for Adaptive Card content (object, not string) - Fix while-pipe subshell bug in ralph-watch.sh (use process substitution) Consistency: - Add ralph_status to VALID_EVENTS in teams-notify.sh - Fix Graph API endpoint in SKILL.md flow diagram - Fix az rest --uri to --url in SKILL.md troubleshooting - Replace hardcoded /Users/shboyer path with $TEAM_ROOT - Align graceful degradation docs with actual warning behavior Other: - Fix history.md: script exists, correct API endpoint - Fix decisions.md: config is introduced in this PR, not pre-existing - Add .ralph-watch-state.json to .gitignore - Add delivery-failure limitation comment to teams-test.sh Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
.squad/scripts/ralph-watch.sh:237
- The issue polling only fetches the most recent 20 closed issues. If more than 20 issues close between polls (or while the script isn’t running), some closures may never be notified. Consider increasing the limit and/or paging until encountering already-known issue numbers.
local closed
closed=$(gh issue list --state closed --json number,title --limit 20 2>/dev/null || echo '[]')
# Filter to issues not in known list
local new_issues
new_issues=$(echo "$closed" | jq -c --argjson known "$known_issues" \
'[.[] | select(.number as $n | $known | index($n) | not)]')
You can also share your feedback on Copilot code review. Take the survey.
| EVENT TYPES: | ||
| work_complete Squad member finished a task → Adaptive Card (green) | ||
| pr_opened Pull request was opened → Simple HTML | ||
| pr_merged Pull request was merged → Adaptive Card (green) | ||
| issue_closed GitHub issue was closed → Simple HTML | ||
| decisions Team decision was recorded → Adaptive Card (orange) | ||
|
|
There was a problem hiding this comment.
ralph_status is supported (VALID_EVENTS + case), but it’s missing from the top-of-file “Event types” comment and from the --help output. Please add it to the documented event types so callers don’t assume it’s invalid/unsupported.
| read_config_value() { | ||
| local key="$1" | ||
| if $USE_JQ; then | ||
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null |
There was a problem hiding this comment.
When jq is used, jq -r returns the literal string null for missing keys. That means missing group_id/channel_id won’t trip the -z check later and the script will try to call Graph with null in the URL, producing a generic send failure instead of the intended “missing … in config” warning. Consider treating null as empty (e.g., ... // empty) or using jq -e + fallback to empty string.
| jq -r "$key" "$CONFIG_FILE" 2>/dev/null | |
| jq -r "${key} // empty" "$CONFIG_FILE" 2>/dev/null |
| fi | ||
|
|
||
| FORMAT_LABEL=$( [[ "$FORMAT" == "card" ]] && echo "Adaptive Card" || echo "HTML" ) | ||
| echo "Teams notification sent: $EVENT_TYPE ($FORMAT_LABEL)" >&2 |
There was a problem hiding this comment.
This logs successful sends to stderr. Since stderr is typically reserved for warnings/errors, this can create noisy “error-looking” output for callers that capture stderr separately. Consider logging success to stdout, or only when a verbose/debug flag is set.
| echo "Teams notification sent: $EVENT_TYPE ($FORMAT_LABEL)" >&2 | |
| echo "Teams notification sent: $EVENT_TYPE ($FORMAT_LABEL)" |
| echo " State: ${STATE_FILE}" | ||
| echo " Notify: ${NOTIFY_SCRIPT}" | ||
| echo "" | ||
|
|
There was a problem hiding this comment.
gh commands depend on the current working directory (or GH_REPO) to know which repo to query, but this script never cds to TEAM_ROOT before calling gh. If someone runs it from outside the repo, it can initialize state incorrectly and later flood Teams with “new” PR/issue notifications. Consider cd "$TEAM_ROOT" early in main() (or use gh ... --repo microsoft/waza) and validate repo context in preflight().
| # Ensure we run from the team repository root so that any `gh` commands | |
| # operate on the intended repo, regardless of the caller's cwd. | |
| cd "$TEAM_ROOT" || { | |
| echo "❌ Failed to cd into TEAM_ROOT (${TEAM_ROOT}). Exiting." >&2 | |
| exit 1 | |
| } |
| local merged | ||
| merged=$(gh pr list --state merged --json number,title,author --limit 20 2>/dev/null || echo '[]') | ||
|
|
||
| # Filter to PRs not in known list | ||
| local new_prs | ||
| new_prs=$(echo "$merged" | jq -c --argjson known "$known_prs" \ | ||
| '[.[] | select(.number as $n | $known | index($n) | not)]') |
There was a problem hiding this comment.
The PR polling only fetches the most recent 20 merged PRs. If more than 20 PRs merge between polls (or while the script isn’t running), older merges can be missed permanently. Consider increasing this limit substantially and/or paging until you hit already-known PRs.
This issue also appears on line 231 of the same file.
| "group_id": "your-group-id-here", | ||
| "channel_id": "19:your-channel-id-here@thread.tacv2", | ||
| "channel_name": "Waza Squad", | ||
| "notify_on": { | ||
| "work_complete": true, | ||
| "pr_opened": true, | ||
| "pr_merged": true, | ||
| "issue_closed": true, | ||
| "decisions": true, | ||
| "ralph_status": false |
There was a problem hiding this comment.
This repo commits a real teams-config.json alongside the .example. Even with placeholders, it’s easy for someone to paste real group_id/channel_id and accidentally commit internal IDs. Consider gitignoring .squad/identity/teams-config.json and keeping only the .example in-repo (update docs/scripts to reference copying the example), or add a strong warning comment + CI guard to prevent committing non-placeholder values.
| "group_id": "your-group-id-here", | |
| "channel_id": "19:your-channel-id-here@thread.tacv2", | |
| "channel_name": "Waza Squad", | |
| "notify_on": { | |
| "work_complete": true, | |
| "pr_opened": true, | |
| "pr_merged": true, | |
| "issue_closed": true, | |
| "decisions": true, | |
| "ralph_status": false | |
| "group_id": "__DO_NOT_PUT_REAL_GROUP_ID_HERE__", | |
| "channel_id": "__DO_NOT_PUT_REAL_CHANNEL_ID_HERE__", | |
| "channel_name": "DEMO CHANNEL - REPLACE IN LOCAL UNTRACKED COPY ONLY", | |
| "notify_on": { | |
| "work_complete": true, | |
| "pr_opened": true, | |
| "pr_merged": true, | |
| "issue_closed": true, | |
| "decisions": true, | |
| "ralph_status": false |
| ## Configuration | ||
|
|
||
| ### Location | ||
| `.squad/identity/teams-config.json` | ||
|
|
There was a problem hiding this comment.
Docs reference only .squad/identity/teams-config.json, but the repo now also contains teams-config.json.example. Please clarify the intended workflow (e.g., “copy the example to teams-config.json and keep the real file uncommitted”) to avoid contributors editing/committing channel IDs by accident.
…peline feat: Add azd extension release pipeline, publish skill, and version bump
Pull request was closed
Summary
Adds automatic notifications to the Waza Squad Microsoft Teams channel when milestone work completes.
What's New
.squad/scripts/teams-notify.sh— Notification script using Microsoft Graph API viaaz rest. Formats HTML messages, handles graceful degradation (exits 0 ifazisn't logged in or Teams is disabled)..squad/scripts/teams-test.sh— Test companion to verify integration..squad/identity/teams-config.json— Configuration with channel IDs and per-event toggles (work_complete, pr_opened, pr_merged, issue_closed, decisions)..squad/skills/teams-notify/SKILL.md— Comprehensive skill doc covering setup, usage, troubleshooting.Architecture
Design Decisions
az restover Power Automate — direct API call, no middleware, uses existing Azure CLI authazsession tokenTesting
✅ Test notification verified — message landed in the Waza Squad Teams channel.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com