fix: narrow server-entry evidence and pin standardize to the checked app repo (#4137)

.changelog/next/fixed-issue-4137-followup.md

@@ -0,0 +1 @@
+- App detection no longer treats an index.html beside a client-side app.js/main.js as a served app, and PM2 standardization launched by app id now always targets that app's own repo

server/routes/standardize.js

@@ -11,9 +11,16 @@ const router = Router();
  * Whenever an `appId` names a real record, it gets the precondition check —
  * even alongside an explicit `repoPath`, so passing both can't smuggle a
  * refused app past the gate. Standardization rewrites the target repo, so an
- * app PortOS never runs under PM2 (and PortOS itself) is refused here rather
- * than relying on the button being hidden in the UI. A bare `repoPath` has no
- * app record to type-check, so it is taken at face value.
+ * app PortOS never runs under PM2 (or a non-Node repo, or PortOS itself) is
+ * refused here rather than relying on the button being hidden in the UI. A bare
+ * `repoPath` has no app record to type-check, so it is taken at face value.
+ *
+ * The checked record's OWN `repoPath` is what gets returned — a companion
+ * `repoPath` is ignored rather than preferred. Typing an app record and then
+ * rewriting a different directory would make the gate decorative: a permitted
+ * Node `appId` alongside some other repo's path would carry a Python or Docker
+ * project straight past the refusal. No caller sends both (the client wrappers
+ * in `apiSystem.js` send one or the other), so this only closes the hole.
  */
 async function resolveStandardizeTarget({ repoPath, appId }) {
   if (appId) {
@@ -25,7 +32,7 @@ async function resolveStandardizeTarget({ repoPath, appId }) {
     if (refusal) {
       throw new ServerError(refusal, { status: 400, code: 'NOT_STANDARDIZABLE' });
     }
-    return repoPath || app.repoPath;
+    return app.repoPath;
   }
 
   if (repoPath) return repoPath;

server/routes/standardize.test.js

@@ -77,6 +77,20 @@ describe('standardize routes — target resolution and the refusal gate', () =>
     expectNoStandardizerWork();
   });
 
+  it('standardizes the checked app\'s own repo, ignoring a companion repoPath', async () => {
+    // Typing one record and then rewriting a different directory would make the
+    // gate decorative — a permitted Node appId could carry a Python or Docker
+    // repo straight past the refusal.
+    vi.mocked(getAppById).mockResolvedValue({ id: 'app-1', type: 'vite+express', repoPath: '/srv/example-app' });
+
+    const res = await request(makeApp())
+      .post('/api/standardize/analyze')
+      .send({ appId: 'app-1', repoPath: '/srv/some-python-service' });
+
+    expect(res.status).toBe(200);
+    expect(analyzeApp).toHaveBeenCalledWith('/srv/example-app', undefined);
+  });
+
   it('404s an appId with no app record', async () => {
     vi.mocked(getAppById).mockResolvedValue(null);
 

server/services/streamingDetect.js

@@ -28,11 +28,16 @@ const DOCKER_MARKERS = ['docker-compose.yml', 'docker-compose.yaml', 'compose.ym
  * package-less repo shipping `server.mjs` is a Node app someone containerized
  * (or that also serves a page), and classifying it `docker`/`static` would
  * withdraw standardization from an app that genuinely wants it — the exact
- * false negative this predicate exists to avoid. `index` is deliberately absent
- * from the basenames: next to an `index.html` it is far more often a
- * client-side script than a server entry point.
+ * false negative this predicate exists to avoid.
+ *
+ * The basename list is deliberately just `server`, the one name that can only
+ * mean a server. `index`, `app`, and `main` are all at least as common as
+ * browser scripts loaded by an `index.html`, and treating those as evidence
+ * would leave real static sites unclassified (and still offered the Node
+ * standardizer) — trading this rule's false negative for the very false
+ * positive the issue was filed about.
  */
-const SERVER_ENTRY_BASENAMES = ['server', 'app', 'main'];
+const SERVER_ENTRY_BASENAMES = ['server'];
 const SERVER_ENTRY_EXTENSIONS = ['.js', '.mjs', '.cjs', '.ts'];
 
 const hasNodeServerEntry = (present) =>

server/services/streamingDetect.test.js

@@ -1142,7 +1142,6 @@ describe('classifyNonNodeType', () => {
     // — `server.mjs` is as much an entry point as `server.js`.
     for (const server of [
       'server.js', 'server.mjs', 'server.cjs', 'server.ts',
-      'app.js', 'app.mjs', 'main.js', 'main.ts',
       'ecosystem.config.js', 'ecosystem.config.cjs'
     ]) {
       expect(classifyNonNodeType(['index.html', server])).toBeNull();
@@ -1153,19 +1152,22 @@ describe('classifyNonNodeType', () => {
     // A containerized Node service is still a Node service — the entry point
     // outranks the packaging, the same way a language marker does.
     expect(classifyNonNodeType(['server.js', 'Dockerfile'])).toBeNull();
-    expect(classifyNonNodeType(['app.mjs', 'docker-compose.yml'])).toBeNull();
+    expect(classifyNonNodeType(['server.mjs', 'docker-compose.yml'])).toBeNull();
   });
 
-  it('still treats a client-side index.js as static, not a server', () => {
-    // In a static site an `index.js` is a browser script far more often than a
-    // server entry point, so it must not suppress the classification.
-    expect(classifyNonNodeType(['index.html', 'index.js', 'style.css'])).toBe('static');
+  it('treats ambiguous browser-script names as static, not as servers', () => {
+    // `index.js`, `app.js`, and `main.js` are at least as common as scripts a
+    // static page loads. Counting them as server evidence would leave real
+    // static sites unclassified and still offered the Node standardizer.
+    for (const script of ['index.js', 'app.js', 'main.js']) {
+      expect(classifyNonNodeType(['index.html', script, 'style.css'])).toBe('static');
+    }
   });
 
   it('keeps the language markers ahead of a Node entry point', () => {
-    // A Python repo with a stray `app.py`-adjacent `main.js` build script is
-    // still python — the language check runs first.
-    expect(classifyNonNodeType(['requirements.txt', 'main.js'])).toBe('python');
+    // A Python repo that also ships a `server.js` helper is still python —
+    // the language check runs first.
+    expect(classifyNonNodeType(['requirements.txt', 'server.js'])).toBe('python');
   });
 
   it('prefers the language over the packaging when both markers are present', () => {