fix(dev): fix unresolved templates in cmd results (#6850)

* fix(dev): fix warning message during init

* fix(dev): fix unresolved templates in cmd results

We were including the raw graph results in the results of commands
requested over HTTP from the live page. This was always unnecessary (and
the data structures involved were excessively large), but only started
resulting in errors with our recent templating improvements (previously,
we'd just leave the unresolved template strings in there).

We can clean this up further in 0.14 when we get rid of `graphResults`
from `ProcessCommandResult` (which will require fixing a bunch of test
cases which currently use that field).

core/src/actions/base.ts

@@ -54,7 +54,7 @@ import type {
   ResolvedAction,
   ResolvedActionWrapperParams,
 } from "./types.js"
-import { actionKinds, actionStateTypes } from "./types.js"
+import { actionKinds, actionStates } from "./types.js"
 import { baseInternalFieldsSchema, varfileDescription } from "../config/base.js"
 import type { DeployAction } from "./deploy.js"
 import type { TestAction } from "./test.js"
@@ -298,7 +298,7 @@ export const actionStatusSchema = createSchema({
   keys: () => ({
     state: joi
       .string()
-      .allow(...actionStateTypes)
+      .allow(...actionStates)
       .only()
       .required()
       .description("The state of the action."),

core/src/actions/types.ts

@@ -112,8 +112,8 @@ export interface ActionConfigTypes {
  *
  * See https://melvingeorge.me/blog/convert-array-into-string-literal-union-type-typescript
  */
-export const actionStateTypes = ["ready", "not-ready", "processing", "failed", "unknown"] as const
-export type ActionState = (typeof actionStateTypes)[number]
+export const actionStates = ["ready", "not-ready", "processing", "failed", "unknown"] as const
+export type ActionState = (typeof actionStates)[number]
 
 export interface ActionStatus<
   T extends BaseAction = BaseAction,

core/src/commands/base.ts

@@ -35,7 +35,7 @@ import type { DeployState, ForwardablePort, ServiceIngress } from "../types/serv
 import { deployStates, forwardablePortSchema, serviceIngressSchema } from "../types/service.js"
 import type { GraphResultMapWithoutTask, GraphResultWithoutTask, GraphResults } from "../graph/results.js"
 import { splitFirst } from "../util/string.js"
-import type { ActionMode } from "../actions/types.js"
+import { actionStates, type ActionMode, type ActionState } from "../actions/types.js"
 import type { AnalyticsHandler } from "../analytics/analytics.js"
 import { withSessionContext } from "../util/open-telemetry/context.js"
 import { wrapActiveSpan } from "../util/open-telemetry/spans.js"
@@ -699,7 +699,7 @@ ${renderCommands(commands)}
 
 // fixme: These interfaces and schemas are mostly copied from their original locations. This is to ensure that
 // dynamically sized or nested fields don't accidentally get introduced to command results. We should find a neater
-// wat to manage all this.
+// way to manage all this.
 
 interface BuildResultForExport extends ProcessResultMetadata {
   buildLog?: string
@@ -730,6 +730,7 @@ interface DeployResultForExport extends ProcessResultMetadata {
   lastMessage?: string
   lastError?: string
   outputs?: PrimitiveMap
+  // TODO-0.14: Rename to deployState
   state: DeployState
 }
 
@@ -756,6 +757,7 @@ const deployResultForExportSchema = createSchema({
     lastError: joi.string().description("Latest error status message of the service (if any)."),
     outputs: joiVariables().description("A map of values output from the deployment."),
     runningReplicas: joi.number().description("How many replicas of the service are currently running."),
+    // TODO-0.14: Rename to deployState
     state: joi
       .string()
       .valid(...deployStates)
@@ -790,8 +792,8 @@ interface TestResultForExport extends ProcessResultMetadata {
 
 const testResultForExportSchema = createSchema({
   name: "test-result-for-export",
-  keys: () => ({}),
   extend: runResultForExportSchema,
+  keys: () => ({}),
 })
 
 export type ProcessResultMetadata = {
@@ -800,20 +802,21 @@ export type ProcessResultMetadata = {
   success: boolean
   error?: string
   inputVersion: string | null
+  actionState: ActionState
 }
 
 export interface ProcessCommandResult {
   aborted: boolean
   success: boolean
-  graphResults: GraphResultMapWithoutTask // TODO: Remove this.
+  graphResults: GraphResultMapWithoutTask // TODO: Remove this in 0.14.
   build: { [name: string]: BuildResultForExport }
   builds: { [name: string]: BuildResultForExport }
   deploy: { [name: string]: DeployResultForExport }
-  deployments: { [name: string]: DeployResultForExport } // alias for backwards-compatibility
+  deployments: { [name: string]: DeployResultForExport } // alias for backwards-compatibility (remove in 0.14)
   test: { [name: string]: TestResultForExport }
   tests: { [name: string]: TestResultForExport }
   run: { [name: string]: RunResultForExport }
-  tasks: { [name: string]: RunResultForExport } // alias for backwards-compatibility
+  tasks: { [name: string]: RunResultForExport } // alias for backwards-compatibility (remove in 0.14)
 }
 
 export const resultMetadataKeys = () => ({
@@ -831,6 +834,7 @@ export const resultMetadataKeys = () => ({
     .description(
       "Alias for `inputVersion`. The version of the task's inputs, before any resolution or execution happens. For action tasks, this will generally be the unresolved version."
     ),
+  actionState: joi.string().valid(...actionStates),
   outputs: joiVariables().description("A map of values output from the action's execution."),
 })
 
@@ -842,11 +846,11 @@ export const processCommandResultSchema = createSchema({
     // Hide this field from the docs, since we're planning to remove it.
     graphResults: joi.any().meta({ internal: true }),
     build: joiIdentifierMap(buildResultForExportSchema().keys(resultMetadataKeys()))
-      .description("A map of all executed Builds (or Builds scheduled/attempted) and information about the them.")
+      .description("A map of all executed Builds (or Builds scheduled/attempted) and information about them.")
       .meta({ keyPlaceholder: "<Build name>" }),
     builds: joiIdentifierMap(buildResultForExportSchema().keys(resultMetadataKeys()))
       .description(
-        "Alias for `build`. A map of all executed Builds (or Builds scheduled/attempted) and information about the them."
+        "Alias for `build`. A map of all executed Builds (or Builds scheduled/attempted) and information about them."
       )
       .meta({ keyPlaceholder: "<Build name>", deprecated: true }),
     deploy: joiIdentifierMap(deployResultForExportSchema().keys(resultMetadataKeys()))
@@ -929,8 +933,8 @@ function prepareBuildResult(graphResult: GraphResultWithoutTask): BuildResultFor
   if (buildResult) {
     return {
       ...common,
-      buildLog: buildResult && buildResult.buildLog,
-      fresh: buildResult && buildResult.fresh,
+      buildLog: buildResult.buildLog,
+      fresh: buildResult.fresh,
     }
   } else {
     return common
@@ -976,7 +980,9 @@ function prepareDeployResult(graphResult: GraphResultWithoutTask): DeployResultF
 }
 
 function prepareTestResult(graphResult: GraphResultWithoutTask): TestResultForExport & ProcessResultMetadata {
-  const common = commonResultFields(graphResult)
+  const common = {
+    ...commonResultFields(graphResult),
+  }
   const detail = graphResult.result?.detail
   if (detail) {
     return {
@@ -992,7 +998,9 @@ function prepareTestResult(graphResult: GraphResultWithoutTask): TestResultForEx
 }
 
 function prepareRunResult(graphResult: GraphResultWithoutTask): RunResultForExport & ProcessResultMetadata {
-  const common = commonResultFields(graphResult)
+  const common = {
+    ...commonResultFields(graphResult),
+  }
   const detail = graphResult.result?.detail
   if (detail) {
     return {
@@ -1016,6 +1024,7 @@ function commonResultFields(graphResult: GraphResultWithoutTask) {
     inputVersion: graphResult.inputVersion,
     // Here for backwards-compatibility
     version: graphResult.inputVersion,
+    actionState: graphResult.result.state as ActionState,
   }
 }
 
@@ -1051,7 +1060,8 @@ export async function handleProcessResults(
   const result: ProcessCommandResult = {
     aborted: false,
     success,
-    graphResults: graphResultsForExport, // TODO: Remove this.
+    // TODO-0.14: Remove graphResults from this type (will also require refactoring test cases that read from this field)
+    graphResults: graphResultsForExport,
     build: buildResults,
     builds: buildResults, // alias for `build`
     deploy: deployResults,

core/src/commands/get/get-actions.ts

@@ -8,7 +8,7 @@
 
 import { getActionState, getRelativeActionConfigPath } from "../../actions/helpers.js"
 import type { ActionKind, ActionState, ResolvedAction } from "../../actions/types.js"
-import { actionKinds, actionStateTypes } from "../../actions/types.js"
+import { actionKinds, actionStates } from "../../actions/types.js"
 import { BooleanParameter, ChoicesParameter, StringsParameter } from "../../cli/params.js"
 import { createSchema, joi, joiArray } from "../../config/common.js"
 import { printHeader } from "../../logger/util.js"
@@ -50,7 +50,7 @@ export const getActionsCmdOutputSchema = createSchema({
     type: joi.string().required().description(`Action Type (e.g. 'container').`),
     state: joi
       .string()
-      .allow(...actionStateTypes)
+      .allow(...actionStates)
       .only()
       .description("The state of the action."),
     path: joi.string().description("The relative path of the action config file."),

core/src/commands/serve.ts

@@ -186,6 +186,7 @@ export class ServeCommand<
             const distroName = getCloudDistributionName(defaultGarden.cloudDomain)
             const livePageUrl = cloudApi.getLivePageUrl({ shortId: session.shortId }).toString()
             const msg = dedent`\n${printEmoji("🌸", log)}Connected to ${distroName} ${printEmoji("🌸", log)}
+
               Follow the link below to stream logs, run commands, and more from the Garden dashboard ${printEmoji(
                 "👇",
                 log

core/src/events/action-status-events.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { actionStateTypes } from "../actions/types.js"
+import { actionStates } from "../actions/types.js"
 import type { BuildState } from "../plugin/handlers/Build/get-status.js"
 import type { ActionRuntime, RunState } from "../plugin/plugin.js"
 import type { DeployState } from "../types/service.js"
@@ -18,7 +18,7 @@ export type ActionStatusDetailedState = DeployState | BuildState | RunState
  * These are the states emitted in status events. Here, we include additional states to help distinguish status event
  * emitted around status/cache checks VS statuses emitted around the execution after a failed status check.
  */
-const actionStateTypesForEvent = [...actionStateTypes, "getting-status", "cached"] as const
+const actionStateTypesForEvent = [...actionStates, "getting-status", "cached"] as const
 /**
  * This type represents the lifecycle of an individual action execution as emitted to Cloud. Note that the
  * internal semantics are slightly different (e.g. Garden uses "ready" instead of "cached" internally).

core/src/plugin/base.ts

@@ -182,14 +182,15 @@ export const artifactsPathSchema = memoize(() =>
   joi.string().required().description("A directory path where the handler should write any exported artifacts to.")
 )
 
-export type RunState = "outdated" | "unknown" | "running" | "succeeded" | "failed" | "not-implemented"
+export const runStates = ["outdated", "unknown", "running", "succeeded", "failed", "not-implemented"] as const
+export type RunState = (typeof runStates)[number]
 
 export interface RunStatusForEventPayload {
   state: RunState
 }
 
 export const outputSchemaDocs = dedent`
-  The schema must be a single level object, with string keys. Each value must be a primitive (null, boolean, number or string).
+  The schema must be a single level object, with string keys. Each vaue must be a primitive (null, boolean, number or string).
 
   If no schema is provided, an error may be thrown if a plugin handler attempts to return an output key.
 

core/src/plugin/handlers/Build/get-status.ts

@@ -21,7 +21,8 @@ import type { ActionStatus, ActionStatusMap, Resolved } from "../../../actions/t
  * - `built`: The build was completed successfully.
  * - `failed`: An error occurred while fetching or building.
  */
-export type BuildState = "fetching" | "fetched" | "outdated" | "building" | "built" | "failed" | "unknown"
+export const buildStates = ["fetching", "fetched", "outdated", "building", "built", "failed", "unknown"] as const
+export type BuildState = (typeof buildStates)[number]
 
 export interface BuildStatusForEventPayload {
   state: BuildState

core/src/server/server.ts

@@ -233,7 +233,7 @@ export class GardenServer extends EventEmitter {
         }
       } while (!serverStarted)
     }
-    this.log.info(`Garden server has successfully started at port ${styles.highlight(this.port.toString())}\n`)
+    this.log.info(`Garden server has successfully started at port ${styles.highlight(this.port.toString())}`)
 
     const processRecord = await this.globalConfigStore.get("activeProcesses", String(process.pid))
 
@@ -380,7 +380,6 @@ export class GardenServer extends EventEmitter {
           parentSessionId: this.sessionId,
         })
         this.debugLog.debug(`Command '${command.name}' completed successfully`)
-
         ctx.response.body = sanitizeValue(result)
       } catch (error) {
         // Return 200 with errors attached, since commands can legitimately fail (e.g. tests erroring etc.)
@@ -706,7 +705,9 @@ export class GardenServer extends EventEmitter {
           })
           // Here we handle the actual command result.
           .then((commandResult) => {
-            const { result, errors } = commandResult
+            const errors = commandResult.errors
+            // TODO-DODDI-0.14: Remove this line once we've removed graphResults from ProcessCommandResult.
+            const result = omit(commandResult.result, "graphResults")
             send(
               "commandResult",
               sanitizeValue({

core/src/util/ink-divider.tsx

@@ -44,7 +44,15 @@ const getNumberOfCharsPerWidth = (char, width) => width / stringWidth(char)
 const PAD = " "
 
 // Divider
-const Divider = ({ title, width, padding, titlePadding, titleColor, dividerChar, dividerColor }) => {
+const Divider = ({
+  title,
+  width = 50,
+  padding = 1,
+  titlePadding = 1,
+  titleColor = "white",
+  dividerChar = "─",
+  dividerColor = "grey",
+}) => {
   const titleString = title ? `${PAD.repeat(titlePadding) + title + PAD.repeat(titlePadding)}` : ""
   const titleWidth = stringWidth(titleString)
 
@@ -77,14 +85,4 @@ Divider.propTypes = {
   dividerColor: PropTypes.string,
 }
 
-Divider.defaultProps = {
-  title: null,
-  width: 50,
-  padding: 1,
-  titlePadding: 1,
-  titleColor: "white",
-  dividerChar: "─",
-  dividerColor: "grey",
-}
-
 export default Divider

core/src/util/logging.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { isArray, isPlainObject, isString, mapValues } from "lodash-es"
+import { isArray, isPlainObject, isString, mapValues, omit } from "lodash-es"
 import stripAnsi from "strip-ansi"
 import { isPrimitive } from "../config/common.js"
 import { deepFilter } from "./objects.js"
@@ -24,6 +24,11 @@ export function sanitizeValue(value: any, _parents?: WeakSet<any>): any {
     })
   }
 
+  // TODO-DODDI-0.14: Remove this line once we've removed graphResults from ProcessCommandResult.
+  if (isPlainObject(value) && "graphResults" in value) {
+    value = omit(value, "graphResults")
+  }
+
   if (!_parents) {
     _parents = new WeakSet()
   } else if (_parents.has(value)) {

core/test/unit/src/server/server.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { expectError, makeTestGardenA, taskResultOutputs, testPluginReferences } from "../../../helpers.js"
+import { expectError, makeTestGardenA, testPluginReferences } from "../../../helpers.js"
 import type { Server } from "http"
 import { GardenServer, startServer } from "../../../../src/server/server.js"
 import type { Garden } from "../../../../src/garden.js"
@@ -17,7 +17,7 @@ import { gardenEnv } from "../../../../src/constants.js"
 import { deepOmitUndefined } from "../../../../src/util/objects.js"
 import { uuidv4 } from "../../../../src/util/random.js"
 import { GardenInstanceManager } from "../../../../src/server/instance-manager.js"
-import type { CommandParams } from "../../../../src/commands/base.js"
+import type { CommandParams, ProcessCommandResult } from "../../../../src/commands/base.js"
 import { Command } from "../../../../src/commands/base.js"
 import request from "supertest"
 import getPort from "get-port"
@@ -228,9 +228,9 @@ describe("GardenServer", () => {
         .expect(200)
 
       expect(res.body.errors).to.eq(undefined, `error response: ${res.body.errors?.[0]?.stack}`)
-      const result = taskResultOutputs(res.body.result)
-      expect(result["build.module-a"]).to.exist
-      expect(result["build.module-a"].state).to.equal("ready")
+      const result = res.body.result as ProcessCommandResult
+      expect(result.build["module-a"]).to.exist
+      expect(result.build["module-a"].actionState).to.equal("ready")
     })
 
     it("creates a Garden instance as needed", async () => {
@@ -497,14 +497,15 @@ describe("GardenServer", () => {
           if (req.type !== "commandResult") {
             return
           }
-          const taskResult = taskResultOutputs(req.result)
+          const taskResult = req.result
           const result = {
             ...req,
             result: taskResult,
           }
           expect(result.requestId).to.equal(id)
-          expect(result.result["build.module-a"]).to.exist
-          expect(result.result["build.module-a"].state).to.equal("ready")
+          const processRes = result.result as ProcessCommandResult
+          expect(processRes.build["module-a"]).to.exist
+          expect(processRes.build["module-a"].actionState).to.equal("ready")
           done()
         },
         skipType: "logEntry",
@@ -535,14 +536,15 @@ describe("GardenServer", () => {
           if (msg.type !== "commandResult") {
             return
           }
-          const taskResult = taskResultOutputs(msg.result)
+          const taskResult = msg.result
           const result = {
             ...msg,
             result: taskResult,
           }
           expect(result.requestId).to.equal(id)
-          expect(result.result["build.module-a"]).to.exist
-          expect(result.result["build.module-a"].state).to.equal("ready")
+          const processRes = result.result as ProcessCommandResult
+          expect(processRes.build["module-a"]).to.exist
+          expect(processRes.build["module-a"].actionState).to.equal("ready")
           done()
         },
         skipType: "logEntry",