fix: address post-merge review findings (#876, #900, #875)#942
Closed
tamirdresher wants to merge 7 commits intobradygaster:devfrom
Closed
fix: address post-merge review findings (#876, #900, #875)#942tamirdresher wants to merge 7 commits intobradygaster:devfrom
tamirdresher wants to merge 7 commits intobradygaster:devfrom
Conversation
…0.9.4-insider.1 Root cause: CLI depended on SDK via '>=0.9.0' which npm resolves to the latest stable version (0.9.1) — a build that predates FSStorageProvider. npm semver rules prevent >=0.9.0 from matching prerelease versions, so users always got the stale SDK. Fixes: 1. Pin CLI SDK dep to exact '0.9.4-insider.1' 2. Add workflow step to auto-pin SDK version before every publish 3. Add registry verification wait between SDK and CLI publish 4. Bump both packages to 0.9.4-insider.1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…radygaster#875) * feat(ci): add file list with line stats, scope badge, and check subtitles to PR readiness (bradygaster#813) - Add file list table with per-file +additions/-deletions stats - Add PR scope classification (Product/Infrastructure/Mixed) - Rename Architectural Review and Security Review checks with descriptive subtitles Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: scope boundary enforcement for repo-health PRs (bradygaster#826) Add CI check that fails when repo-health PRs include product source code changes under packages/*/src/. Prevents scope creep where infrastructure PRs accidentally touch product code. - Add squad-scope-check.yml workflow - Document PR scope rules in copilot-instructions.md - Fail loudly on git diff errors instead of silently passing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: smart PR nudge for stale PRs (bradygaster#827) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add cross-package export smoke test to catch missing imports Validates every value import squad-cli uses from squad-sdk resolves to a defined export at runtime. Covers 15 SDK subpaths and 50+ named exports including FSStorageProvider, SquadClient, CastingEngine, RalphMonitor, and all resolution/config/platform helpers. Also verifies that every entry in the SDK package.json exports map points to a file that actually exists on disk. Motivation: v0.9.3-insider.1 shipped with FSStorageProvider missing from the SDK barrel — broke users at runtime while all TS-level tests passed (TypeScript resolves from source, not compiled output). Refs: bradygaster#836 --------- Co-authored-by: Dina Berry (MSFT) <diberry@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com>
…adygaster#900) * feat(cli): add deprecation warnings for tunnel, rc, and REPL commands Adds visible deprecation warnings to: - Interactive REPL shell (squad with no args) - squad start (and --tunnel flag) - squad rc / remote-control - squad rc-tunnel Phase 1: warnings only — no behavior changes. Commands still work but now emit a yellow deprecation notice pointing users to the GitHub Copilot CLI as the replacement. Help text updated to show [DEPRECATED] tags on affected commands. Closes bradygaster#899 Related: bradygaster#665 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add changeset for tunnel/rc/REPL deprecation warnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: shorten deprecation hint to fit 80-char UX gate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…bradygaster#876) * Add enforcement wiring step to hiring process + workflow wiring guide (bradygaster#592) Fixes bradygaster#591 - Added step 7 (Wire enforcement) to Adding Team Members in squad.agent.md - Added workflow-wiring-guide.md with configuration surface area, wiring instructions, common mistakes, and verification checklist - Added appendix walkthroughs for code reviewer (gate pattern) and documenter (follow-up trigger pattern) Co-authored-by: Jonathan Ben Ami <jbenami@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(agents): add Challenger / Devil's Advocate agent template + fact-checking skill (bradygaster#603) * feat(skills): add fact-checking skill\n\nAdds challenger/fact-checking review pattern.\nVerified against 200+ issues in production squads.\nCloses bradygaster#598 * feat(agents): add challenger agent charter template\n\nGeneric Devil's Advocate / Challenger template.\nProvides auto-spawn integration pattern for coordinators.\nCloses bradygaster#598 * feat: add APM integration for skill publishing and installation Closes bradygaster#824 ## Changes ### New command: squad skill - squad skill publish [<name>] — exports skill(s) to APM format, generating/updating apm.yml - squad skill install <source> — installs a skill from APM registry - Supports owner/repo, owner/repo/skill-name, and direct URLs - Uses GitHub CLI to fetch from repos that have apm.yml or .squad/skills/ - Writes .apm-source.json metadata to track skill origin - squad skill list — lists installed skills with source provenance ### Updated: squad init - Now generates �pm.yml at project root alongside .squad/ - Follows skipExisting semantics (safe to re-run) - apm.yml includes skills, instructions, and prompts sections ### Updated: squad help - Added skill command to help text with usage examples ## APM format apm.yml is the Agent Package Manager manifest — package.json for AI agent context. See: https://github.com/microsoft/apm The manifest declares skills, instructions, and prompts in a portable format that �pm install can deploy to .github/, .claude/, .cursor/ etc. * chore: add changeset for APM integration * docs: update CHANGELOG.md with APM integration entry * fix(skill): use .copilot/skills/ as primary path per bradygaster#430 The skills unification in bradygaster#430 migrated skills from .squad/skills/ to .copilot/skills/. This updates the APM skill command to: - Check .copilot/skills/ first, fall back to .squad/skills/ (backward compat) - Use resolveSkillsDir() helper matching LocalSkillSource pattern - Update all user-facing messages and apm.yml template paths - Fix installSkillsFromSquadDir candidate order Addresses review feedback from @Meir017. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align CHANGELOG.md with dev branch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add missing fs import in init.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: joniba <joniba@users.noreply.github.com> Co-authored-by: Jonathan Ben Ami <jbenami@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <Copilot@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com>
…#900, bradygaster#875) - Add YAML value escaping helper for skill metadata - Replace catch(err: any) with catch(err: unknown) + narrowing - Add type guards to replace unsafe type assertions - Standardize deprecation messages on `gh copilot` - Fix unsafe exports type cast in cross-package test Closes bradygaster#924, bradygaster#925, bradygaster#926 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tamirdresher
added
the
skip-version-check
Owner
|
Closing in favor of #963 — a clean cherry-pick of the 3 fix commits onto dev. The original branch was based on insider, causing the PR diff to include 29 files when only 4 were actually changed by the fix. #963 has a clean 4-file diff (+15/-4). Thanks @tamirdresher for the contribution! |
bradygaster
pushed a commit
that referenced
this pull request
Apr 12, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bradygaster
added a commit
that referenced
this pull request
Apr 12, 2026
* fix: address post-merge review findings (#876, #900, #875) - Add YAML value escaping helper for skill metadata - Replace catch(err: any) with catch(err: unknown) + narrowing - Add type guards to replace unsafe type assertions - Standardize deprecation messages on `gh copilot` - Fix unsafe exports type cast in cross-package test Closes #924, #925, #926 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add changeset for review findings fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: trigger CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(.squad): EECOM history — PR #942 rebase learnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: update changeset to accurately reflect PR changes (drop YAML escaping reference) Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/54f41407-61bf-4977-85b7-572341c47b62 Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
tamirdresher
pushed a commit
that referenced
this pull request
Apr 21, 2026
* fix: address post-merge review findings (#876, #900, #875) - Add YAML value escaping helper for skill metadata - Replace catch(err: any) with catch(err: unknown) + narrowing - Add type guards to replace unsafe type assertions - Standardize deprecation messages on `gh copilot` - Fix unsafe exports type cast in cross-package test Closes #924, #925, #926 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add changeset for review findings fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: trigger CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(.squad): EECOM history — PR #942 rebase learnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: update changeset to accurately reflect PR changes (drop YAML escaping reference) Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/54f41407-61bf-4977-85b7-572341c47b62 Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes from Dina post-merge reviews (River, Mal, Simon). YAML escaping, catch(err:unknown), type guards, gh copilot standardization, exports test type safety. Closes #924, #925, #926