fix(ci,train): run fast-integ-tests in CodeBuild and give shallow RLVR cases a reward signal

.github/workflows/pr-checks-master.yml

@@ -246,18 +246,36 @@ jobs:
   # Additive: runs the shallow (submit-then-stop) suite for sagemaker-train
   # alongside the existing integ-tests job above, which is unchanged.
   #
-  # Why a separate job rather than folding this into the CodeBuild suite: this
-  # job's selection is reviewable in the PR that changes it, whereas the
-  # sagemaker-train CodeBuild buildspec is CDK-managed outside this repo. It also
-  # reports as its own check, so a shallow failure is distinguishable at a glance
-  # from a deep-suite failure, and it finishes in minutes -- fast feedback that
-  # does not wait on the 2XLARGE CodeBuild container.
+  # Runs in CodeBuild, not on the runner. It began as a runner job, but
+  # actions/checkout refuses to place a fork's head commit in a
+  # pull_request_target job -- correctly, because the runner also holds the base
+  # repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit
+  # conftest.py and read those credentials out. That is the "pwn request" shape,
+  # and on a public repo it is a live credential-exfiltration path, so the fix is
+  # to move the execution rather than override the refusal with
+  # allow-unsafe-pr-checkout. Nearly every PR here comes from a fork, so a
+  # same-repo guard would have left the suite with almost no gate coverage.
+  # source-version-override is how the three jobs above already run PR code: the
+  # build never sees the runner's token, secrets or default-branch cache.
   #
-  # What runs here: only tests/integ/train/shallow. The client-side tests
-  # (recipe resolution, data utils, dry-run, log streaming) are deliberately NOT
-  # repeated -- the CodeBuild suite already runs the whole tests/integ tree, so
-  # widening this job's scope would duplicate them and double the job creation
-  # the shallow suite performs.
+  # Why its own project rather than folding this into the sagemaker-train
+  # integ-tests project: it reports as its own check, so a shallow failure is
+  # distinguishable at a glance from a deep-suite failure, and it runs
+  # concurrently with the deep suite instead of queueing behind it.
+  #
+  # Tradeoff of moving off the runner: the pytest selection now lives in the
+  # CDK's buildspecs.ts (createCIShallowIntegBuildSpec) instead of this file, so
+  # changing which tests run is no longer reviewable in a PR to this repo. That
+  # is the price of executing fork code safely, and it is the same place the
+  # other three test jobs' selections already live.
+  #
+  # What runs there: only tests/integ/train/shallow, deselecting gpu_intensive
+  # (the CPT and MTRL classes, which need a pre-provisioned HyperPod cluster and
+  # an agent runtime plus an MLflow app) and us_east_1 (Nova cases, which run in
+  # the integ-tests-us-east-1 project against the Nova account). The client-side
+  # tests are deliberately not repeated -- the deep suite already runs the whole
+  # tests/integ tree, so widening scope would duplicate them and double the job
+  # creation the shallow suite performs.
   #
   # Why submit-then-stop is worth gating on: CreateTrainingJob returns a
   # TrainingJobArn only after the request has cleared public-model validation,
@@ -272,88 +290,22 @@ jobs:
   fast-integ-tests:
     runs-on: ubuntu-latest
     needs: [detect-changes]
+    # No same-repo guard: nothing here checks out PR code, so fork PRs are gated
+    # too. The suite only runs when sagemaker-train is in the change set.
     if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
     steps:
-      - uses: actions/checkout@v3
-        with:
-          # pull_request_target checks out the base ref by default; these tests
-          # must run against the PR's code.
-          ref: 'refs/pull/${{ github.event.pull_request.number }}/head'
-
-      - name: Set up Python
-        uses: actions/setup-python@v5
-        with:
-          python-version: '3.12'
-
       - name: Configure AWS Credentials
         uses: aws-actions/configure-aws-credentials@v4
         with:
           role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
           aws-region: us-west-2
           role-duration-seconds: 10800
 
-      - name: Install sagemaker-train and test dependencies
-        run: |
-          python -m pip install --upgrade pip
-          pip install ./sagemaker-core
-          pip install ./sagemaker-train
-          pip install -r requirements/extras/test_requirements.txt
-
       - name: Run shallow sagemaker-train integ tests
-        working-directory: sagemaker-train
-        env:
-          AWS_DEFAULT_REGION: us-west-2
-          # Role resolution goes through iam:SimulatePrincipalPolicy, which is
-          # low-TPS; adaptive retries keep parallel workers from throttling each
-          # other.
-          AWS_RETRY_MODE: adaptive
-          AWS_MAX_ATTEMPTS: '10'
-          # Cap the training jobs the *service* counts against the concurrency
-          # quota, across all xdist workers, so the suite stays inside the
-          # "concurrent model customization serverless jobs per Region" quota
-          # (20) with room for the deep integ-tests suite running the same
-          # account concurrently. The harness holds each slot until the job is
-          # terminal, not until stop() returns -- the service counts a job for
-          # ~1-3 min after the stop -- so 10 means "at most 10 jobs counted at
-          # once", the batches-of-10 behaviour, not "10 stops in flight".
-          #
-          # Not redundant with -n 8. -n caps worker processes; this caps what
-          # the service counts, and with the slot held to terminal those diverge
-          # sharply (each drain outlives the worker's stop() by minutes). It is
-          # also what keeps the ceiling stable if -n is raised. A serverful job
-          # counts one slot per instance.
-          SHALLOW_MAX_CONCURRENT_JOBS: '10'
-        run: |
-          # Scoped to shallow/ only -- see the comment above this job for why the
-          # rest of tests/integ/train is not repeated here.
-          #
-          # 84 of the suite's 100 tests run; the 16 deselected are:
-          #   us_east_1 (5)     -- Nova cases; this job holds us-west-2
-          #                        credentials only, so they run in the
-          #                        integ-tests-us-east-1 job instead.
-          #   gpu_intensive (11) -- the CPT and MTRL classes. These are written in
-          #                        the shallow style but cannot be made
-          #                        self-contained: CPT submits only via HyperPod
-          #                        (a pre-provisioned cluster, not
-          #                        CreateTrainingJob) and MTRL needs an agent
-          #                        runtime plus an MLflow app. Both become
-          #                        gate-eligible by dropping one marker once those
-          #                        prerequisites exist in the PR account.
-          python -m pytest tests/integ/train/shallow \
-            -m "not gpu_intensive and not us_east_1" \
-            -n 8 \
-            --dist loadfile \
-            -v \
-            --durations=15 \
-            --junitxml=shallow-integ-results.xml
-
-      - name: Upload test results
-        if: always()
-        uses: actions/upload-artifact@v4
+        uses: aws-actions/aws-codebuild-run-build@v1
         with:
-          name: shallow-integ-test-results
-          path: sagemaker-train/shallow-integ-results.xml
-          if-no-files-found: warn
+          project-name: ${{ github.event.repository.name }}-ci-sagemaker-train-fast-integ-tests
+          source-version-override: 'refs/pull/${{ github.event.pull_request.number }}/head^{${{ github.event.pull_request.head.sha }}}'
 
   integ-tests-us-east-1:
     runs-on: ubuntu-latest

sagemaker-train/tests/integ/train/shallow/README.md

@@ -8,6 +8,23 @@ What this suite changes about the gate is not which job runs, but what the exist
 one selects: the `gpu_intensive` marks added here deselect the deep tests that
 submit a job and wait for it, and this suite covers those code paths instead.
 
+> **Where this runs.** The `fast-integ-tests` job in `pr-checks-master.yml` does not
+> execute this suite on the GitHub runner — it starts the CodeBuild project
+> `sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests` via
+> `source-version-override`, the same way the `codestyle-doc-tests`, `unit-tests`
+> and `integ-tests` jobs run PR code. That is deliberate: the runner holds the base
+> repo's `GITHUB_TOKEN` and assumes `CI_AWS_ROLE_ARN`, so `actions/checkout` refuses
+> to place a fork's head commit there, and on a public repo overriding that refusal
+> would be a live credential-exfiltration path. Running in CodeBuild gates fork PRs
+> — which is nearly all of them — without exposing those credentials to PR code.
+>
+> **Consequence for editing this suite:** the marker selection above (`-n 8`,
+> `--dist loadfile`, `-m "not gpu_intensive and not us_east_1"`) lives in
+> `createCIShallowIntegBuildSpec` in the `SageMakerMLFPySDKInfraCDK` package, not in
+> this repo. Adding a file under `shallow/` is picked up automatically, but changing
+> *how* the suite is invoked means a change there, which deploys through a pipeline
+> rather than merging with your PR.
+
 ## What a passing test proves
 
 Each test submits a real `CreateTrainingJob`, asserts the service returned a

sagemaker-train/tests/integ/train/shallow/test_rlvr_trainer.py

@@ -33,12 +33,34 @@
     "arn:aws:sagemaker:us-west-2:729646638167:hub-content/sdktest/JsonDoc/rlvr-test-rf/0.0.1"
 )
 
+# The preset the deep suite pairs with an ordinary training dataset on this same
+# model (test_rlvr_trainer_lora_complete_workflow).
+PRESET_REWARD_FUNCTION = "prime_code"
+
 
 class TestRLVRTrainerSubmission(RecipeTrainerCases):
     """RLVR accepts every shared case, plus recipe customization."""
 
     TRAINER = RLVRTrainer
 
+    def build(self, sagemaker_session, dataset, name, **overrides):
+        """Add a reward signal, which RLVR requires before it will submit.
+
+        ``RLVRTrainer.train()`` raises ``ValueError`` unless
+        ``custom_reward_function`` was passed or
+        ``hyperparameters.preset_reward_function`` is set. The cases inherited from
+        ``RecipeTrainerCases`` pass neither -- they are about recipe rendering and
+        dataset handling, not reward configuration -- so the preset is applied once
+        here rather than repeated in each test.
+
+        Skipped when the test supplies its own ``custom_reward_function``, so the
+        reward-function variants below still exercise exactly what they name.
+        """
+        trainer = super().build(sagemaker_session, dataset, name, **overrides)
+        if not overrides.get("custom_reward_function"):
+            trainer.hyperparameters.preset_reward_function = PRESET_REWARD_FUNCTION
+        return trainer
+
     def test_direct_hyperparameter_mutation(self, sagemaker_session, train_data_uri):
         """trainer.hyperparameters.<field> = ... is a documented pattern (used
         by the existing RLVR tests) and must reach the payload intact."""